[bug #64185] *** only one 'else' per conditional. Stop. due to else in recipe

[Dmitry Goncharov]

Follow-up Comment #11, bug #64185 (project make):


> If someone has a thought of something this might break (other than people
indenting their conditionals with the recipe prefix) please let me know 

See https://savannah.gnu.org/bugs/index.php?64259


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #64185] *** only one 'else' per conditional. Stop. due to else in recipe

[Paul D. Smith]

Update of bug #64185 (project make):

  Status:None => Fixed  
 Assigned to:None => psmith 
 Open/Closed:Open => Closed 
Operating System:None => Any
   Fixed Release:None => SCM
   Triage Status:None => Small Effort   

___

Follow-up Comment #10:

Fixed but I put the wrong bug number in the commit *sigh*


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #64185] *** only one 'else' per conditional. Stop. due to else in recipe

[Paul D. Smith]

Follow-up Comment #9, bug #64185 (project make):

I agree that this is simply a bug.  There are definitely places where it's
impossible to "do the right thing" with relation to GNU Make's conditionals,
because there is no easy way to tell them apart from non-conditional
statements (this is the difference between GNU Make's conditionals and the C
preprocessor's conditionals, which use an invalid syntax).

However, this example plainly contravenes the explicit text of the
documentation AND I don't see any reason for it to do so, so I think it's a
bug.  It seems simple to fix as well:

diff --git a/src/read.c b/src/read.c
index 4e8432a0..6748486e 100644
--- a/src/read.c
+++ b/src/read.c
@@ -666,16 +666,16 @@ eval (struct ebuffer *ebuf, int set_default)
 /* Ignore the commands in a rule with no targets.  */
 continue;

+  if (ignoring)
+/* Yep, this is a shell command, and we don't care.  */
+continue;
+
   /* If there is no preceding rule line, don't treat this line
  as a command, even though it begins with a recipe prefix.
  SunOS 4 make appears to behave this way.  */

   if (filenames != 0)
 {
-  if (ignoring)
-/* Yep, this is a shell command, and we don't care.  */
-continue;
-
   if (commands_idx == 0)
 cmds_started = ebuf->floc.lineno;



Plus some tests need to be written of course.

If someone has a thought of something this might break (other than people
indenting their conditionals with the recipe prefix) please let me know
because I can't think of one.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #64185] *** only one 'else' per conditional. Stop. due to else in recipe

[Harry Clauson]

Follow-up Comment #8, bug #64185 (project make):

Sorry, the first excerpt should read as follows:

all:
ifdef blah
junk:
else
else
endif


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #64185] *** only one 'else' per conditional. Stop. due to else in recipe

[Harry Clauson]

Follow-up Comment #7, bug #64185 (project make):

I often remind developers that “the tail does not wag the dog”, in the
same way that the current behavior of the code does not define correctness.

While you keep explaining what the parser is currently doing, what I am
reporting is that this is not the correct behavior as described by the GNU
Make documentation.  While the documented behavior makes perfect sense, the
parser behavior you are describing contradicts the documentation as well as
common sense.

Specifically, the documentation clearly defines two fundamental behaviors for
Make Conditionals as follows:

“Extra spaces are allowed and ignored at the beginning of the conditional
directive line, but a tab is not allowed.  (If the line begins with a tab, it
will be considered part of a recipe for a rule.)”

“The other two directives that play a part in a conditional are else and
endif.  Each of these directives is written as one word, with no arguments. 
Extra spaces are allowed and ignored at the beginning of the line, and spaces
or tabs at the end.”

“Conditionals affect which lines of the makefile make uses.  If the
condition is true, make reads the lines of the test-if-true as part of the
makefile; if the condition is false, make ignores those lines completely.”

The two behaviors are:

1. Any lines of code within an inactive conditional branch are entirely
ignored.
2. Conditional directives cannot be confused with recipe lines because they
cannot begin with a tab character. The documentation should be updated to
refer to the .RECIPEPREFIX character, which did not exist prior to Make 3.82.

Lets apply the above statements, in detail, to the behavior you have explained
regarding the excerpt:

all:
ifdef blah
junk:
else
else
endif

> Here, the parser sees rule 'all:' and begins its search for recipe lines of
'all:'. Then the parser sees 'ifdef blah'. 'blah' is not defined and the
parser
ignores the following line, which is 'junk:'. 

> Then parser reads '\telse' and takes
it as a recipe of 'all:'. 

This is incorrect behavior, because the condition was false so it is in an
inactive branch:

"if the conditon is false, make ignores those lines completely.”

> Because the parser took one of the two elses as a recipe,
there is no complain about extra elses in a conditional.

This is incorrect behavior because the recipe else begins with a tab so it is
not a conditional directive:

“(If the line begins with a tab, it will be considered part of a recipe for
a rule.)”

Since the conditional is false, all lines of code are ignored until the next
conditional (else) preceded only by optional spaces:

"Extra spaces are allowed and ignored at the beginning of the line, and spaces
or tabs at the end.”

The else begins the active branch of the conditional, so any lines of code (in
this case none) until the following conditional endif are passed to the Make
parser.  The endif ends the active branch and the scope of the conditional.

The correct result is that only the following lines of code are "seen" by the
Make parser:

all:

Similarly, for the second excerpt:

all:

$(info hello)
ifdef blah
junk:
else
else
endif

> Here, $(info hello) ends the search for recipe lines of 'all'.
By the time the parser reads 'ifdef blah' it is no longer looking for recipe
lines.

> The parser then ignores 'junk:', because 'blah' is not defined and
treats '\telse' as a conditional directive.

This is incorrect behavior because conditional directives cannot begin with a
tab character:

“(If the line begins with a tab, it will be considered part of a recipe for
a rule.)”

As above, since the conditional was false, all lines of code will be ignored
until it reaches the next conditional directive:

"if the conditon is false, make ignores those lines completely.”

As above, the conditional directive else terminates the inactive branch and
begins the active branch until the conditional directive endif terminates the
active branch and the scope of the conditional.

The correct result is that the following lines of code are "seen" by the Make
parser:

all:

$(info hello)

It may be helpful to understand that Make conditionals are defined to behave
similarly to "C" pre-processor conditionals, in that each line of code is
first examined and handled as follows:

1. If the line is a conditional directive, it is processed as such and the
"state" of any conditional is adjusted accordingly (none, active conditional
branch, inactive conditional branch).
2. Otherwise, if the "state" is currently an inactive conditional branch, the
line is ignored.
3. Otherwise, the line of code is passed to the code parser.



___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #64185] *** only one 'else' per conditional. Stop. due to else in recipe

[Harry Clauson]

Follow-up Comment #5, bug #64185 (project make):

In addition, section 7.2 "Syntax of Conditionals" states:

"Extra spaces are allowed and ignored at the beginning of the conditional
directive line, but a tab is not allowed. (If the line begins with a tab, it
will be considered part of a recipe for a rule.)"

This explains why it should, and does, work correctly without the $(warning
...) statement. But it does not appear to work correctly with the addition of
that statement.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #64185] *** only one 'else' per conditional. Stop. due to else in recipe

[Harry Clauson]

Follow-up Comment #4, bug #64185 (project make):

Maybe I'm missing something here but, as I reported this issue, the recipe
else is handled correctly (that is, it is not recognized as a make else within
the recipe).

The error is caused not by the else in the recipe, but when adding the
$(warning ...) statement immediately preceding the make else. That is what
appears to cause the parser to behave incorrectly, and I don't see any reason
for such a statement to affect parsing behavior at all.

It sounds like you're saying the parser will always be confused by an indented
recipe else statement, but this was not the case.

Thank you.




___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #64185] *** only one 'else' per conditional. Stop. due to else in recipe

[Paul D. Smith]

Update of bug #64185 (project make):

  Item Group: Bug => Enhancement

___

Follow-up Comment #3:

I agree that due to parsing limitations it's very difficult to manage this as
make's keywords are too common and its structure is not regular enough.

We could modify the parser so that "else" lines which start with the recipe
prefix are ignored, so "else" that appears inside a recipe wouldn't be in
effect.  The problem with this is that the recipe prefix could be changed and
since we're not actually parsing the code we would not notice:


if FOO
all:
else
.RECIPEPREFIX = >
else
X = y
endif


How do we understand that once the .RECIPEPREFIX is modified, the "else" that
starts with a TAB should not be assumed to be a recipe prefix anymore?

I guess we can just say that it's never OK to indent make processor statements
with a TAB.  If we assume that any "else" (for example) indented with a TAB is
never a make "else" then we shouldn't have this problem.

I think.  I'm not sure how much havoc this would cause for backward
compatibility.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #64185] *** only one 'else' per conditional. Stop. due to else in recipe

[Dmitry Goncharov]

Follow-up Comment #2, bug #64185 (project make):

It is also necessary to ignore rule definitions in the branches which are not
taken from the point of view of correctness.


all: hello.tsk
hello=1
ifdef hello
hello.tsk:; echo true
else
hello.tsk:; echo false
endif


Here, we need the 'echo true' rule to be defined. If make honored the else
branch, then make would've redefined 'echo true' with 'echo false'.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #64185] *** only one 'else' per conditional. Stop. due to else in recipe

[Dmitry Goncharov]

Follow-up Comment #1, bug #64185 (project make):

The same behavior can be demonstrated with the following makefile


ifdef blah
junk:
else
else
endif


The parser does not perform variable expansion and ignores rule definitions
in
conditional branches which are not taken to avoid redundant work.
In this case, the ifdef branch is not taken and the parser ignores 'junk:'
rule
definition . The parser keeps looking for conditional keywords and finds
'else'
in the junk recipe. Because the parser ignored the definition of rule 'junk:'
earlier, the parser does not know that this else is a part of a rule recipe.

The error check should stay, because it is useful, it catches a missing
ifdef.
The parser should not waste efforts of processing branches which are not
taken.
It is unfortunate that make conditional keyword 'else' is also used by shell.
bsd make is more fortunate in this regards, their keyword is '.else'.
The only thing that comes to mind is to introduce a set of synonym keywords
like '.ifdef', '.else', etc.

To work around you can move the definition of junk recipe to a variable.


define junk_recipe
else
endef

ifdef blah
junk:
$(junk_recipe)
else
endif




___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #64185] *** only one 'else' per conditional. Stop. due to else in recipe

[Harry Clauson]

URL:
  

 Summary: *** only one 'else' per conditional.  Stop. due to
else in recipe
   Group: make
   Submitter: harryc
   Submitted: Fri 12 May 2023 02:04:16 AM UTC
Severity: 3 - Normal
  Item Group: Bug
  Status: None
 Privacy: Public
 Assigned to: None
 Open/Closed: Open
 Discussion Lock: Any
   Component Version: 4.4.1
Operating System: None
   Fixed Release: None
   Triage Status: None


___

Follow-up Comments:


---
Date: Fri 12 May 2023 02:04:16 AM UTC By: Harry Clauson 
Please see attached makefiles. As provided, they should work correctly as
follows:

cmd: make -fmakefile.start
makefile.start:1: including makefile.start ver 4.4.1 goals
makefile.blah:2: blah = 1 goals:
makefile.blah:6: begin blah pass
makefile.blah:30: end of blah
makefile.start:10: end of start
echo "target help"
target help

However, if you uncomment makefile.blah, line 15, make sees the else statement
in the 'junk' recipe as part of the conditional. This is odd as that line is
simply a $(warning ...) logging call.

These were complex makefile which worked find until the $(warning ...) call
was inserted ahead of the else statement. I simplified them as much as
possible and still reproduce the error.

This happens with make-4.4.1 build on aix:

cmd: make --version
GNU Make 4.4.1
Built for powerpc-ibm-aix7.2.5.0
Copyright (C) 1988-2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later

This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

My workaround is to comment out the particular $(warning ...) call.

Thank you for any assistance!






___
File Attachments:


---
Date: Fri 12 May 2023 02:04:16 AM UTC  Name: makefile.start  Size: 173B   By:
harryc


---
Date: Fri 12 May 2023 02:04:16 AM UTC  Name: makefile.blah  Size: 404B   By:
harryc



___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/