Skip to content

Commit 5f28b21

Browse files
tglsfdctglsfdc
committed
Fix behavior of ecpg's "EXEC SQL elif name".
This ought to work much like C's "#elif defined(name)"; but the code implemented it in a way equivalent to endif followed by ifdef, so that it didn't matter whether any previous branch of the IF construct had succeeded. Fix that; add some test cases covering elif and nested IFs; and improve the documentation, which also seemed a bit confused. AFAICS the code has been like this since the feature was added in 1999 (commit b57b0e0). So while it's surely wrong, there might be code out there relying on the current behavior. Hence, don't back-patch into stable branches. It seems all right to fix it in v13 though. Per report from Ashutosh Sharma. Reviewed by Ashutosh Sharma and Michael Meskes. Discussion: https://postgr.es/m/CAE9k0P=dQk9X0cU2tN49S7a9tv733-e1pVdpB1P-pWJ5PdTktg@mail.gmail.com
1 parent b8fdee7 commit 5f28b21

File tree

5 files changed

+211
-111
lines changed

5 files changed

+211
-111
lines changed

‎doc/src/sgml/ecpg.sgml‎

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5695,7 +5695,7 @@ EXEC SQL UPDATE Tbl SET col = MYNUMBER;
56955695
</sect2>
56965696

56975697
<sect2 id="ecpg-ifdef">
5698-
<title>ifdef, ifndef, else, elif, and endif Directives</title>
5698+
<title>ifdef, ifndef, elif, else, and endif Directives</title>
56995699
<para>
57005700
You can use the following directives to compile code sections conditionally:
57015701

@@ -5705,7 +5705,7 @@ EXEC SQL UPDATE Tbl SET col = MYNUMBER;
57055705
<listitem>
57065706
<para>
57075707
Checks a <replaceable>name</replaceable> and processes subsequent lines if
5708-
<replaceable>name</replaceable> has been created with <literal>EXEC SQL define
5708+
<replaceable>name</replaceable> has been defined via <literal>EXEC SQL define
57095709
<replaceable>name</replaceable></literal>.
57105710
</para>
57115711
</listitem>
@@ -5716,30 +5716,40 @@ EXEC SQL UPDATE Tbl SET col = MYNUMBER;
57165716
<listitem>
57175717
<para>
57185718
Checks a <replaceable>name</replaceable> and processes subsequent lines if
5719-
<replaceable>name</replaceable> has <emphasis>not</emphasis> been created with
5719+
<replaceable>name</replaceable> has <emphasis>not</emphasis> been defined via
57205720
<literal>EXEC SQL define <replaceable>name</replaceable></literal>.
57215721
</para>
57225722
</listitem>
57235723
</varlistentry>
57245724

57255725
<varlistentry>
5726-
<term><literal>EXEC SQL else;</literal></term>
5726+
<term><literal>EXEC SQL elif <replaceable>name</replaceable>;</literal></term>
57275727
<listitem>
57285728
<para>
5729-
Starts processing an alternative section to a section introduced by
5730-
either <literal>EXEC SQL ifdef <replaceable>name</replaceable></literal> or
5731-
<literal>EXEC SQL ifndef <replaceable>name</replaceable></literal>.
5729+
Begins an optional alternative section after an
5730+
<literal>EXEC SQL ifdef <replaceable>name</replaceable></literal> or
5731+
<literal>EXEC SQL ifndef <replaceable>name</replaceable></literal>
5732+
directive. Any number of <literal>elif</literal> sections can appear.
5733+
Lines following an <literal>elif</literal> will be processed
5734+
if <replaceable>name</replaceable> has been
5735+
defined <emphasis>and</emphasis> no previous section of the same
5736+
<literal>ifdef</literal>/<literal>ifndef</literal>...<literal>endif</literal>
5737+
construct has been processed.
57325738
</para>
57335739
</listitem>
57345740
</varlistentry>
57355741

57365742
<varlistentry>
5737-
<term><literal>EXEC SQL elif <replaceable>name</replaceable>;</literal></term>
5743+
<term><literal>EXEC SQL else;</literal></term>
57385744
<listitem>
57395745
<para>
5740-
Checks <replaceable>name</replaceable> and starts an alternative section if
5741-
<replaceable>name</replaceable> has been created with <literal>EXEC SQL define
5742-
<replaceable>name</replaceable></literal>.
5746+
Begins an optional, final alternative section after an
5747+
<literal>EXEC SQL ifdef <replaceable>name</replaceable></literal> or
5748+
<literal>EXEC SQL ifndef <replaceable>name</replaceable></literal>
5749+
directive. Subsequent lines will be processed if no previous section
5750+
of the same
5751+
<literal>ifdef</literal>/<literal>ifndef</literal>...<literal>endif</literal>
5752+
construct has been processed.
57435753
</para>
57445754
</listitem>
57455755
</varlistentry>
@@ -5748,22 +5758,30 @@ EXEC SQL UPDATE Tbl SET col = MYNUMBER;
57485758
<term><literal>EXEC SQL endif;</literal></term>
57495759
<listitem>
57505760
<para>
5751-
Ends an alternative section.
5761+
Ends an
5762+
<literal>ifdef</literal>/<literal>ifndef</literal>...<literal>endif</literal>
5763+
construct. Subsequent lines are processed normally.
57525764
</para>
57535765
</listitem>
57545766
</varlistentry>
57555767
</variablelist>
57565768
</para>
57575769

57585770
<para>
5759-
Example:
5771+
<literal>ifdef</literal>/<literal>ifndef</literal>...<literal>endif</literal>
5772+
constructs can be nested, up to 127 levels deep.
5773+
</para>
5774+
5775+
<para>
5776+
This example will compile exactly one of the three <literal>SET
5777+
TIMEZONE</literal> commands:
57605778
<programlisting>
5761-
EXEC SQL ifndef TZVAR;
5762-
EXEC SQL SET TIMEZONE TO 'GMT';
5779+
EXEC SQL ifdef TZVAR;
5780+
EXEC SQL SET TIMEZONE TO TZVAR;
57635781
EXEC SQL elif TZNAME;
57645782
EXEC SQL SET TIMEZONE TO TZNAME;
57655783
EXEC SQL else;
5766-
EXEC SQL SET TIMEZONE TO TZVAR;
5784+
EXEC SQL SET TIMEZONE TO 'GMT';
57675785
EXEC SQL endif;
57685786
</programlisting>
57695787
</para>

‎src/interfaces/ecpg/preproc/pgc.l‎

Lines changed: 88 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,29 @@ struct _yy_buffer
7979

8080
static char *old;
8181

82+
/*
83+
* Vars for handling ifdef/elif/endif constructs. preproc_tos is the current
84+
* nesting depth of such constructs, and stacked_if_value[preproc_tos] is the
85+
* state for the innermost level. (For convenience, stacked_if_value[0] is
86+
* initialized as though we are in the active branch of some outermost IF.)
87+
* The active field is true if the current branch is active (being expanded).
88+
* The saw_active field is true if we have found any successful branch,
89+
* so that all subsequent branches of this level should be skipped.
90+
* The else_branch field is true if we've found an 'else' (so that another
91+
* 'else' or 'elif' at this level is an error.)
92+
* For IFs nested within an inactive branch, all branches always have active
93+
* set to false, but saw_active and else_branch are maintained normally.
94+
* ifcond is valid only while evaluating an if-condition; it's true if we
95+
* are doing ifdef, false if ifndef.
96+
*/
8297
#define MAX_NESTED_IF 128
8398
static short preproc_tos;
84-
static short ifcond;
99+
static bool ifcond;
85100
static struct _if_value
86101
{
87-
short condition;
88-
short else_branch;
102+
bool active;
103+
bool saw_active;
104+
bool else_branch;
89105
} stacked_if_value[MAX_NESTED_IF];
90106

91107
%}
@@ -1165,11 +1181,26 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
11651181
return S_ANYTHING;
11661182
}
11671183
}
1168-
<C,xskip>{exec_sql}{ifdef}{space}* { ifcond = true; BEGIN(xcond); }
1184+
<C,xskip>{exec_sql}{ifdef}{space}* {
1185+
if (preproc_tos >= MAX_NESTED_IF-1)
1186+
mmfatal(PARSE_ERROR, "too many nested EXEC SQL IFDEF conditions");
1187+
preproc_tos++;
1188+
stacked_if_value[preproc_tos].active = false;
1189+
stacked_if_value[preproc_tos].saw_active = false;
1190+
stacked_if_value[preproc_tos].else_branch = false;
1191+
ifcond = true;
1192+
BEGIN(xcond);
1193+
}
11691194
<C,xskip>{informix_special}{ifdef}{space}* {
11701195
/* are we simulating Informix? */
11711196
if (INFORMIX_MODE)
11721197
{
1198+
if (preproc_tos >= MAX_NESTED_IF-1)
1199+
mmfatal(PARSE_ERROR, "too many nested EXEC SQL IFDEF conditions");
1200+
preproc_tos++;
1201+
stacked_if_value[preproc_tos].active = false;
1202+
stacked_if_value[preproc_tos].saw_active = false;
1203+
stacked_if_value[preproc_tos].else_branch = false;
11731204
ifcond = true;
11741205
BEGIN(xcond);
11751206
}
@@ -1179,11 +1210,26 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
11791210
return S_ANYTHING;
11801211
}
11811212
}
1182-
<C,xskip>{exec_sql}{ifndef}{space}* { ifcond = false; BEGIN(xcond); }
1213+
<C,xskip>{exec_sql}{ifndef}{space}* {
1214+
if (preproc_tos >= MAX_NESTED_IF-1)
1215+
mmfatal(PARSE_ERROR, "too many nested EXEC SQL IFDEF conditions");
1216+
preproc_tos++;
1217+
stacked_if_value[preproc_tos].active = false;
1218+
stacked_if_value[preproc_tos].saw_active = false;
1219+
stacked_if_value[preproc_tos].else_branch = false;
1220+
ifcond = false;
1221+
BEGIN(xcond);
1222+
}
11831223
<C,xskip>{informix_special}{ifndef}{space}* {
11841224
/* are we simulating Informix? */
11851225
if (INFORMIX_MODE)
11861226
{
1227+
if (preproc_tos >= MAX_NESTED_IF-1)
1228+
mmfatal(PARSE_ERROR, "too many nested EXEC SQL IFDEF conditions");
1229+
preproc_tos++;
1230+
stacked_if_value[preproc_tos].active = false;
1231+
stacked_if_value[preproc_tos].saw_active = false;
1232+
stacked_if_value[preproc_tos].else_branch = false;
11871233
ifcond = false;
11881234
BEGIN(xcond);
11891235
}
@@ -1193,28 +1239,22 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
11931239
return S_ANYTHING;
11941240
}
11951241
}
1196-
<C,xskip>{exec_sql}{elif}{space}* { /* pop stack */
1197-
if ( preproc_tos == 0 ) {
1242+
<C,xskip>{exec_sql}{elif}{space}* {
1243+
if (preproc_tos == 0)
11981244
mmfatal(PARSE_ERROR, "missing matching \"EXEC SQL IFDEF\" / \"EXEC SQL IFNDEF\"");
1199-
}
1200-
else if ( stacked_if_value[preproc_tos].else_branch )
1245+
if (stacked_if_value[preproc_tos].else_branch)
12011246
mmfatal(PARSE_ERROR, "missing \"EXEC SQL ENDIF;\"");
1202-
else
1203-
preproc_tos--;
1204-
1205-
ifcond = true; BEGIN(xcond);
1247+
ifcond = true;
1248+
BEGIN(xcond);
12061249
}
12071250
<C,xskip>{informix_special}{elif}{space}* {
12081251
/* are we simulating Informix? */
12091252
if (INFORMIX_MODE)
12101253
{
12111254
if (preproc_tos == 0)
12121255
mmfatal(PARSE_ERROR, "missing matching \"EXEC SQL IFDEF\" / \"EXEC SQL IFNDEF\"");
1213-
else if (stacked_if_value[preproc_tos].else_branch)
1256+
if (stacked_if_value[preproc_tos].else_branch)
12141257
mmfatal(PARSE_ERROR, "missing \"EXEC SQL ENDIF;\"");
1215-
else
1216-
preproc_tos--;
1217-
12181258
ifcond = true;
12191259
BEGIN(xcond);
12201260
}
@@ -1226,16 +1266,19 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
12261266
}
12271267

12281268
<C,xskip>{exec_sql}{else}{space}*";" { /* only exec sql endif pops the stack, so take care of duplicated 'else' */
1229-
if (stacked_if_value[preproc_tos].else_branch)
1269+
if ( preproc_tos == 0 )
1270+
mmfatal(PARSE_ERROR, "missing matching \"EXEC SQL IFDEF\" / \"EXEC SQL IFNDEF\"");
1271+
else if (stacked_if_value[preproc_tos].else_branch)
12301272
mmfatal(PARSE_ERROR, "more than one EXEC SQL ELSE");
12311273
else
12321274
{
12331275
stacked_if_value[preproc_tos].else_branch = true;
1234-
stacked_if_value[preproc_tos].condition =
1235-
(stacked_if_value[preproc_tos-1].condition &&
1236-
!stacked_if_value[preproc_tos].condition);
1276+
stacked_if_value[preproc_tos].active =
1277+
(stacked_if_value[preproc_tos-1].active &&
1278+
!stacked_if_value[preproc_tos].saw_active);
1279+
stacked_if_value[preproc_tos].saw_active = true;
12371280

1238-
if (stacked_if_value[preproc_tos].condition)
1281+
if (stacked_if_value[preproc_tos].active)
12391282
BEGIN(C);
12401283
else
12411284
BEGIN(xskip);
@@ -1245,16 +1288,19 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
12451288
/* are we simulating Informix? */
12461289
if (INFORMIX_MODE)
12471290
{
1248-
if (stacked_if_value[preproc_tos].else_branch)
1291+
if ( preproc_tos == 0 )
1292+
mmfatal(PARSE_ERROR, "missing matching \"EXEC SQL IFDEF\" / \"EXEC SQL IFNDEF\"");
1293+
else if (stacked_if_value[preproc_tos].else_branch)
12491294
mmfatal(PARSE_ERROR, "more than one EXEC SQL ELSE");
12501295
else
12511296
{
12521297
stacked_if_value[preproc_tos].else_branch = true;
1253-
stacked_if_value[preproc_tos].condition =
1254-
(stacked_if_value[preproc_tos-1].condition &&
1255-
!stacked_if_value[preproc_tos].condition);
1298+
stacked_if_value[preproc_tos].active =
1299+
(stacked_if_value[preproc_tos-1].active &&
1300+
!stacked_if_value[preproc_tos].saw_active);
1301+
stacked_if_value[preproc_tos].saw_active = true;
12561302

1257-
if (stacked_if_value[preproc_tos].condition)
1303+
if (stacked_if_value[preproc_tos].active)
12581304
BEGIN(C);
12591305
else
12601306
BEGIN(xskip);
@@ -1272,7 +1318,7 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
12721318
else
12731319
preproc_tos--;
12741320

1275-
if (stacked_if_value[preproc_tos].condition)
1321+
if (stacked_if_value[preproc_tos].active)
12761322
BEGIN(C);
12771323
else
12781324
BEGIN(xskip);
@@ -1286,7 +1332,7 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
12861332
else
12871333
preproc_tos--;
12881334

1289-
if (stacked_if_value[preproc_tos].condition)
1335+
if (stacked_if_value[preproc_tos].active)
12901336
BEGIN(C);
12911337
else
12921338
BEGIN(xskip);
@@ -1301,12 +1347,10 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
13011347
<xskip>{other} { /* ignore */ }
13021348

13031349
<xcond>{identifier}{space}*";" {
1304-
if (preproc_tos >= MAX_NESTED_IF-1)
1305-
mmfatal(PARSE_ERROR, "too many nested EXEC SQL IFDEF conditions");
1306-
else
13071350
{
13081351
struct _defines *defptr;
13091352
unsigned int i;
1353+
bool this_active;
13101354

13111355
/*
13121356
* Skip the ";" and trailing whitespace. Note that yytext
@@ -1324,13 +1368,15 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
13241368
defptr = defptr->next)
13251369
/* skip */ ;
13261370

1327-
preproc_tos++;
1328-
stacked_if_value[preproc_tos].else_branch = false;
1329-
stacked_if_value[preproc_tos].condition =
1330-
(defptr ? ifcond : !ifcond) && stacked_if_value[preproc_tos-1].condition;
1371+
this_active = (defptr ? ifcond : !ifcond);
1372+
stacked_if_value[preproc_tos].active =
1373+
(stacked_if_value[preproc_tos-1].active &&
1374+
!stacked_if_value[preproc_tos].saw_active &&
1375+
this_active);
1376+
stacked_if_value[preproc_tos].saw_active |= this_active;
13311377
}
13321378

1333-
if (stacked_if_value[preproc_tos].condition)
1379+
if (stacked_if_value[preproc_tos].active)
13341380
BEGIN(C);
13351381
else
13361382
BEGIN(xskip);
@@ -1442,10 +1488,12 @@ lex_init(void)
14421488
parenths_open = 0;
14431489
current_function = NULL;
14441490

1445-
preproc_tos = 0;
14461491
yylineno = 1;
1447-
ifcond = true;
1448-
stacked_if_value[preproc_tos].condition = ifcond;
1492+
1493+
/* initialize state for if/else/endif */
1494+
preproc_tos = 0;
1495+
stacked_if_value[preproc_tos].active = true;
1496+
stacked_if_value[preproc_tos].saw_active = true;
14491497
stacked_if_value[preproc_tos].else_branch = false;
14501498

14511499
/* initialize literal buffer to a reasonable but expansible size */

0 commit comments

Comments
 (0)