Bug | ISPP processing code after elif that it shouldn't but should work anyway

[Bredahl Joseph]

Example 1: Should work on its own.

 

   #ifndef BAZ

      #define BAZ = NULL

   #elif BAZ == "foo"

      ; do nothing

   #endif

 

Example 2: With Example 1 nested in elif

 

   #define FOO = "foo"

   #if FOO == "foo"

      FOO = "bar"

   #elif FOO == "baz"

      #ifndef BAZ

         #define BAZ = NULL

      #elif BAZ == "foo"

         ; do nothing

      #endif

   #endif

 

End examples

 

The #elif FOO == “baz” is unreachable and shouldn’t be processed at all. However, if it were it should still work.

Public

[Joseph Bredahl]

I forgot to mention the unexpected error while running example 2 via Build>Compile in Inno Setup Compiler 6.1.2

Line 7:

Column 11:

Undeclared identifier: BAZ

[Gavin Lambert]

On 6/05/2022 13:13, Joseph Bredahl wrote:
> I forgot to mention the unexpected error while running example 2 via
> Build>Compile in Inno Setup Compiler 6.1.2
>
> Line 7:
> Column 11:
> Undeclared identifier: BAZ

The following compiles as expected without errors for me; note that I've
corrected a #define that you were missing:

```
#ifndef BAZ
# define BAZ "foo"
#elif BAZ == "bar"
#endif

#define FOO "foo"
#if FOO == "foo"
# define FOO "bar"

#elif FOO == "baz"
# ifndef BAZ

# define BAZ "foo"
# elif BAZ == "bar"
# endif
#endif
```

[Joseph Bredahl]

Thanks for the response Gavin.

>  corrected a #define that you were missing

The define for BAZ isn't missing. Its nested inside the elif logic where FOO == "baz". This logic should be unreachable and thrown out by the preprocessor anyway. It is correct and should work but if it wasn't correct shouldn't it have been thrown away anyway?

I might be confusing myself or misunderstanding how the ISPP should work. If so I apologize. Below I will try to provide more examples that should all work. They should show how the ISPP is behaving inside unreachable elif logic. I've also added line numbers to make it easier to talk about specific lines. It might not be so fun to copy/paste so I'll zip up and attached these examples.

examples-set-2-ex-1

This on its own should work but it will fail.

Error: line 7: column 11: Undeclared identifier: BAZ

 - Note 1: Lines 5-8 inclusive should not fail and won't on their own while not nested in an elif.

 - Note 2: Lines 5-8 inclusive should have been thrown out by ISPP and never processed. Even if they were incorrect it shouldn't care.

```

1| #define FOO "foo"
2| #if FOO == "foo"
3|    #define FOO "bar"
4| #elif FOO == "baz"
5|    #ifndef BAZ
6|       #define BAZ "foo"
7|    #elif BAZ == "bar"
8|    #endif
9| #endif

```

examples-set-2-ex-2

If lines 5 and 6 are removed, from examples-set-2-ex-1, and the elif is corrected to be if, it will work. 

(note I renumbered the lines in the below example after removing line 5&6 from examples-set-2-ex-1)

- Note 1: Lines 5-6 inclusive should fail on their own when not nested in an elif. (see examples-set-2-ex-3)

- Note 2: I believe this isn't failing because the ISPP is correctly throwing those lines away since they are unreachable.

```

1| #define FOO "foo"
2| #if FOO == "foo"
3|    #define FOO "bar"
4| #elif FOO == "baz"
5|    #if BAZ == "bar"
6|    #endif
7| #endif

```

examples-set-2-ex-3

If lines 5 and 6 from examples-set-2-ex-2 were on their own they will fail.

(note: I renumbered 5 & 6 from examples-set-2-ex-2 to 1 & 2 respectfully). 

Error: Line 1: Column 9: Undeclared Identifier: BAZ

```

1| #if BAZ == "bar"
2| #endif

```

It seems like the ISPP will jump into the unreachable portion of an elif if there is an ifndef in it. Then it will throw that ifndef away but still process the remainder of what should be unreachable.

examples-set-2-ex-4

If the ifndef->elif->endif were broken into two stand alone if blocks, ifndef->endif and if->endif, it will work.

Something is breaking when ifndef->elif->endif is contained in an unreachable elif. 

Key word being "unreachable". If the logic were in reachable code the ISPP would process it correctly. (see examples-set-2-ex-5)

```

1| #define FOO "foo"
2| #if FOO == "foo"
3|    #define FOO "bar"
4| #elif FOO == "baz"
5|    #ifndef BAZ
6|       #define BAZ "foo"

7|    #endif
8|    #if BAZ == "bar"
9|    #endif
10| #endif

```

examples-set-2-ex-5

Code in question is now in reachable elif. The only thing I changed from examples-set-2-exe-1 was #define FOO as "baz" instead of "foo".

```

1| #define FOO "baz"
2| #if FOO == "foo"
3|    #define FOO "bar"
4| #elif FOO == "baz"
5|    #ifndef BAZ
6|       #define BAZ "foo"
7|    #elif BAZ == "bar"
8|    #endif
9| #endif

```

[Wilenty org]

Hello,
I corrected your examples by adding '#pragma message (...)', so, you will see how it's parsed.

I won't post source of them in this message that this message would not be too long...

Please look at the "compiler output", you will see line numbers (in parentheses) which was parsed.

[Joseph Bredahl]

Wilenty,

Thanks for suggesting the use of `#pragma message` I've tossed it into the original examples and noticed that the places that the messages I expect the ISPP to throw away are thrown away.

Also, thank you for the suggested changes to work around the bug. I agree there is a different way to write the logic that I think will be nearly functionally equivalent.

However, the corrections you've made actually change the test case to represent something other then what is being attempted to represent.

Logic Set A: Successful

```

#ifndef BAZ
   #define BAZ "foo"

#elif BAZ == "bar"
#endif

```

Logic Set B: Successful

```

#define FOO "foo"
#if FOO == "foo"

   #define FOO "bar"
#elif FOO == "baz"

#endif

```

Logic Set C: Fails - not because its syntactically incorrect.

```

#define FOO "foo"
#if FOO != "foo"
  #define FOO "bar"
#elif FOO == "FOO"
  #ifndef BAZ
    #define BAZ "foo"
  #endif

#elif BAZ == "bar"
#endif

```

[Joseph Bredahl]

> Logic Set C: Fails - not because its syntactically incorrect.

I actually pasted the wrong set of logic here. I intended it to be...

```

#define FOO "foo"
#if FOO == "foo"
   #define FOO "bar"
#elif FOO == "baz"

   #ifndef BAZ
      #define BAZ "foo"
   #elif BAZ == "bar"
   #endif

#endif

```

[Wilenty org]

In my opinion you doing it in a wrong way...

Treat the #if(...)#endif as a block of work to do. Before the start of the block there is only one #define FOO before it. But in the second part of first block you are trying to define second #define BAZ if it's not defined and immediately you want to check it in second #if(...)#endif in the second part of first block.

Look that it works if you move the problematic block outside the first block and/or you will divide the second block in the first block into two parts.

```

#define FOO "foo"
#if FOO == "foo"
  #define FOO "bar"
#elif FOO == "baz"
  #ifndef BAZ
    #define BAZ "foo"

  #endif
  #if BAZ == "bar"
  #endif
#endif

#ifndef BAZ
  #define BAZ "bar"
#elif BAZ == "foo"
#endif

#pragma error "BAZ -> " + BAZ
```

[Joseph Bredahl]

Wilenty,

I agree, I think breaking up the ifndef->elif->endif block into two different blocks ifndef->endif and then a following if->endif is an appropriate work around for the bug. I plan on implementing things this way as a work around.

I still believe there is a bug and that's what I'm reporting. The single ifndef->elif->endif block will work all by itself but not if its nested in a block that the ISPP skips.

If the single block is nested inside of another block that gets skipped by the ISPP, the ISPP is calling it invalid when its not invalid. Possibly the most important point is the single block should be allowed to be invalid because the ISPP should be skipping it. Why validate skipped code and why is valid code being called invalid?

 

If this works by itself, why doesn't it work when nested in an elif that the ISPP should skip.

```

#ifndef BAZ
  #define BAZ "bar"
#elif BAZ == "foo"
#endif

```

-Joe

[Joseph Bredahl]

Maybe this will help.

Try this...

```

#define FOO "baz"

#if FOO == "foo"
   #define FOO "bar"
#elif FOO == "baz"
   #ifndef BAZ
      #define BAZ "foo"
   #elif BAZ == "bar"
   #endif
#endif

```

vs this...

```

#define FOO "foo"
#if FOO == "foo"
   #define FOO "bar"
#elif FOO == "baz"
   #ifndef BAZ
      #define BAZ "foo"
   #elif BAZ == "bar"
   #endif
#endif

```

[Kevin Puetz]

I don't think the example

#ifndef BAZ
  #define BAZ "bar"

#elif BAZ == "foo"

  ...

#endif

is trying to test the value that was just set with #define BAZ "bar". It's either providing a fallback definition (if BAZ is not defined at all yet), or checking the existing definition (if there is one already in scope). And (because of the #elif) it's specifically not checking the value that it itself just set. That seems like it ought to be legal to do, whether or not BAZ is currently defined, and it in fact works (at top-level). Now, in Joe's specific case, the value it's checking for is different than the default it sets for #ifndef, there's no harm in translating it to

#ifndef BAZ
  #define BAZ "bar"

#endif

#if BAZ == "foo"

  ...

#endif

If we just defined BAZ = "bar" in the #ifndef case, then it just won't match the BAZ  == "foo". But I think the real bug Joe is getting at is that this code (which is valid as-is at top level whether or not BAZ is defined) starts sthrowing "Undeclared identifier" if the whole thing is nested inside  another #if

In his initial example this was #if FOO=="baz", but it really doesn't matter - if it's somewhere that is reachable it's checking for and handling the !defined case, and if it's somewhere that is not unreachable it shouldn't matter whether the expression is valid or not.

I.e. Since

#ifndef BAZ
  #define BAZ "bar"

#elif BAZ == "foo"

#endif

will preprocess successfully, then one would think that

#if 0

#ifndef BAZ
  #define BAZ "bar"

#elif BAZ == "foo"

#endif

#endif

certainly should - it's the same thing, except skipped!

[Kevin Puetz]

Experimenting/simplifying even more, I hit something even weirder (and maybe more telling)

#if 1

#elif NOEXIST
#endif

#if 0
#if NOEXIST
#endif

#endif

both preprocess successfully (despite using an undefined  NOEXIST). But

#if 0

#if 1

#elif NOEXIST
#endif

#endif

fails with "Undeclared identifier: NOEXIST", despite the fact this is doubly-unreachable (#if 0 is always false, so the whole inner #if/#elif case ought to be skipped, and also an else branch following a #if 1 is also unreachable). So It seems to only be a problem specifically with #elif. That sure seems like there's some kind of ISPP scoping error here... I don't really know how ISPP's parser/evaluation works internally, but the behavior makes me suspect some kind of flattening of nested #if expressons by combining the parent expression with a logical AND.That would yield something like

#if 0 && 1

#elif 0 && NOEXIST

#endif

And then the first case is false, so the #elif would get evaluated, and while it's already false too (0 && anything == 0) if there's no short-circuiting you might diagnose the "Undeclared identifier" before realizing it.

[Wilenty org]

So, look at this:
```
#pragma parseroption -u+

#if BredahlJoseph == "user"
#elif JosephBredahl == "user"
#elif GavinLambert == "user"
#elif Wilenty == "user"
#elif KevinPuetz == "user"
#elif MaybeWeCall_MartijnLaan == "YES!"
#endif
```

[Kevin Puetz]

Ok, attempting to read the code I think I may have found the right place:

https://github.com/jrsoftware/issrc/blob/bd96b87f5edc0317134e3d5c0b9ed321e3f01015/Projects/ISPP/IsppPreprocessor.pas#L361-L366

If I remember my pascal from (from the 90s so no great guarantees) pascal `and` does short-circuit. So #if, .i.e.

pcIf..pcIfNExist:

FStack.IfInstruction(FStack.Include and ProcessPreprocCommand(Command, S, DirectiveOffset));

will not call ProcessPreprocCommand when  FStack.Include is already false, and thus will pass Eval := False, and thus set BlockState := False

But #elif:

pcElseIf:
FStack.ElseIfInstruction(FStack.Last.Fired or
(FStack.Include or not FStack.Last.BlockState) and ProcessPreprocCommand(Command, S, DirectiveOffset));

will see that FStack.Last.BlockState = False, and thus (FStack.Include or not FStack.Last.BlockState) = True, and thus it will call ProcessPreprocCommand(Command, S, DirectiveOffset))

Which in this case will error out.

I would guess that the `or not FStack.Last.BlockState` part is because we want to evaluate #elif after the #if was false, but TConditionalTranslationStack.Include does not distinguish whether it was the top-most #if that was false or a higher level. What we really want to ask here is whether FStack.Include is all true, but stopping the check one element early (ignoring FStack.Last.BlockState).  Which is not quite the same thing as anding all of them, and then or-ing in not FStack.Last.BlockState

[Kevin Puetz]

Posted https://github.com/jrsoftware/issrc/pull/419 with what I think might be the right fix (based on reading through the code), though I do not have access to Embarcadero Delphi to test it.