[bug #65596] Test for let function not available in .FEATURES

[Jouke Witteveen]

Follow-up Comment #1, bug #65596 (group make):

You're right. I guess it's too late to fix that now, since it will not be
fixed in all releases that have the "let" function.

You can test for its presence with the following construct:

$(filter builtin,$(foreach let,unavailable,$(call let,,,builtin)))

This will expand to "builtin" if there is a built-in "let" function and to the
empty string otherwise. Without the outermost "filter", it would expand to
"unavailable" when there is no built-in "let" function.


___

Reply to this item at:

  

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

[bug #65448] $(intcmp) problems with negative integers

[Jouke Witteveen]

Follow-up Comment #1, bug #65448 (group make):

Indeed, it seems we currently compare magnitudes instead of values. I've
attached a fix and regression tests.

(file #55826)

___

Additional Item Attachment:

File name: 0001-SV-65448-Compare-values-instead-of-magnitudes-in-int.patch
Size:2 KB
   



AGPL NOTICE

These attachments are served by Savane. You can download the corresponding
source code of Savane at
https://git.savannah.nongnu.org/cgit/administration/savane.git/snapshot/savane-f95530753e974ac1d60f8c0c697f148099475f1a.tar.gz


___

Reply to this item at:

  

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

Re: Conditional append operator (was: Re: New conditional assignment facility)

[Jouke Witteveen]

On Sun, Feb 4, 2024 at 12:05 AM Paul Smith  wrote:
>
> On Sat, 2024-02-03 at 17:45 -0500, Paul Smith wrote:
> > Here's how I think the "append changes the type of the variable"
> > option works:
>
...
>
> Sorry for the possible confusion.
>

I feel these proposals are
1) not obvious in how they work, regardless of how they work, and
2) largely possible without new syntax.

The first of these is proven by the amount of communication so far.
The second is true except for maybe leaking some helper variables.

For the original ?= these two points did not hold. When you understand
what kind of problem it solves, it is fairly clear that it does so in
a very reasonable way.

What about adding only ?:= and !:= and forgetting about appending? The
semantics of these two operators should be so obvious that I don't
even need to explain them. They are also not really possible with the
current operators (the $(shell) function is not an operator).

Regards,
- Jouke

Re: New conditional assignment facility

[Jouke Witteveen]

On Mon, Jan 22, 2024 at 4:59 PM Martin Dorey
 wrote:
>
> Why is that?
>
>
> I imagine because that's how, to my surprise, it is today:
>
> martind@stormy:~/tmp/make-conditional-assignment-2024-01-22$ cat Makefile
> A := 42
> A += $(shell hello)
> martind@stormy:~/tmp/make-conditional-assignment-2024-01-22$ make
> make: hello: No such file or directory
> make: *** No targets.  Stop.
> martind@stormy:~/tmp/make-conditional-assignment-2024-01-22$
>
> ... and changing something so foundational sounds likely to cause widespread 
> issues.
>

As said before, += is going to be the exception (by respecting any
pre-existing variable type) because of legacy. That's not all that bad
though, it just exemplifies that variables can go from recursively
expanded to simply expanded, but not back.

Re: New conditional assignment facility

[Jouke Witteveen]

On Mon, Jan 22, 2024 at 2:16 PM Paul Smith  wrote:
> > If the only goal was to allow +:= create a simple variable, then can
> > we do the following?
> > "If no variable with this name exists, then +:= creates a simple
> > variable, and +:::= creates an immediately-expanded-and-escaped
> > variable. Otherwise, +:= and +:::= behave the same as +=".
> > If we go with this, then there is no need for +!=, because +!= would
> > behave as +=. Similarly, there is no need for +::=, because +::=
> > would behave as +:=.
>
> I agree that we don't need to treat +:= and +::= differently.  They are
> just different names for the same operator.
>
...
>
> The variable assignment operators describe two things: how to handle
> the RHS value and what the resulting flavor of the variable will be.
> Although we have four assignment operators, once the assignment is
> complete there are only two flavors of variables: either recursive or
> simple:
>
>   =- Don't expand the RHS, --> recursive variable
>   :=   - Expand the RHS, --> simple variable
>   ::=  - Expand the RHS, --> simple variable
>   :::= - Expand and escape the RHS, --> recursive variable
>   !=   - Run the RHS as a shell script, --> recursive variable.
>
> If an appending operator is applied to a variable that doesn't exist
> already, then the behavior is the same as if we used the non-appending
> operator in all cases.  So if the variable doesn't exist, "+=" behaves
> like "=", "+:=" behaves like ":=", "+::=" behaves like "::=", "+:::="
> behaves like ":::=", and "+!=" behaves like "!=".  That's simple.
>
> What about a variable that does exist already?
>
> I think that we can agree that the append operator cannot change the
> type of a variable that already exists.  If the variable was recursive
> before it must stay recursive afterward; the same for simple variables.

Why is that? Why not say
X +:= Y
behaves similar to
X := $(X) $(Y)
(the difference being that the space is added only if X is nonempty)?
That would make documentation simpler.

I was impressed by the realization that + can be treated as an
assignment modifier just like ?. I expect that someone will inevitably
ask for !:= as well, even though ! cannot be considered an assignment
modifier. How far do we go?

"+!:=" could mean append simply expanded shell output to a variable
that retains its type.

"+:!:=" could mean append simply expanded shell output to a simply
expanded variable.

ugh...

Regards,
- Jouke

[bug #65019] Let function segfaults when foreach return empty list

[Jouke Witteveen]

Follow-up Comment #4, bug#65019 (group make):

Thanks for applying! If I'm not mistaken, I had put a reference in the subject
and _git am_ would have included it in the commit message.


___

Reply to this item at:

  

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

Re: Bug with $(info xxx) in 4.2.1

[Jouke Witteveen]

On Fri, Dec 8, 2023 at 7:38 PM Paul Smith  wrote:
>
> In make it's fine to have a make line indented with a TAB, that is not
> part of a recipe.  If a line appears outside of the context of a rule,
> then leading whitespace is ignored.

There is a section in the documentation called "How Makefiles Are
Parsed" which contains:

3. If the line begins with the recipe prefix character and we are in a
rule context, add the line to the current recipe and read the next
line.

Maybe that should be tweaked to indicate that it only applies "in a
recipe context".

> This can be seen by the example given, where the variable assignment
> line did not generate an error.  You can do this simpler experiment
> without the ifeq, which is irrelevant for this issue:
>
>   FOO = bar
>   $(info baz)
>   all:;
>
> (where the first two lines are indented with TAB).  Here you will see
> that make accepts the first line but fails on the second line:
>
>   $ make
>   Makefile:2: *** recipe commences before first target.  Stop.
>
> The reason for this is that make tries to understand what kind of line
> it has, BEFORE it expands the line.  You can tell it is not expanded
> because the info function's output is not shown.  So in the first line,
> make sees that it's a variable assignment and it accepts it.
>
> However make doesn't know what kind of line the second line is, and
> before it expands the content and realizes that the result would be the
> empty string it generates this error.

The documentation continues with:

4. Expand elements of the line which appear in an immediate expansion
context (see How make Reads a Makefile).
5. Scan the line for a separator character, such as ‘:’ or ‘=’, to
determine whether the line is a macro assignment or a rule (see Recipe
Syntax).
6. Internalize the resulting operation and read the next line.

The last statement suggests that a line containing only a variable
reference or macro expansion, such as $(aaa) or $(info xxx) presents
an immediate expansion context, although it does not meet any of the
stated forms of an immediate expansion context as defined in section
"How make Reads a Makefile".

It is not entirely clear to me whether implemented behavior, the
documented behavior, or neither is best. The documentation was updated
a few years ago. Back then, I already observed these inconsistencies
[SV 51974].

Probably these observations are only tangentially related to the issue
at hand, but I thought it would be worth highlighting for some more
context.

> However, this behavior should probably be considered a bug.  Aaron, it
> would be helpful if you could file this as a bug in Savannah:
>
> https://savannah.gnu.org/bugs/?group=make&func=additem

Cheers,
- Jouke

[bug #65019] Let function segfaults when foreach return empty list

[Jouke Witteveen]

Follow-up Comment #2, bug#65019 (group make):

The attached patch should fix the issue and prevent regressions.

(file #55442)

___

Additional Item Attachment:

File name: SV-65019-Fix-let-segfault-on-value-with-trailing-whi.patch Size:1
KB
   



AGPL NOTICE

These attachments are served by Savane. You can download the corresponding
source code of Savane at
https://git.savannah.nongnu.org/cgit/administration/savane.git/snapshot/savane-6c5908e04e1ed307ffbde38f75d0d2a0bd284b17.tar.gz


___

Reply to this item at:

  

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

[bug #65019] Let function segfaults when foreach return empty list

[Jouke Witteveen]

Follow-up Comment #1, bug#65019 (group make):

You are right! An even shorter way to reproduce the segfault is with

$(let a b, ,)

Having more than one variable is needed to trigger the bug. Without the
whitespace in the value list, the bug also does not trigger.

I believe the following will fix it. I hope to prepare a patch with some tests
later.

diff --git a/src/function.c b/src/function.c
index a705c8a0..4596bd90 100644
--- a/src/function.c
+++ b/src/function.c
@@ -927,7 +927,7 @@ func_let (char *o, char **argv, const char *funcname
UNUSED)
   while (*vp_next != '\0')
 {
   p = find_next_token (&list_iterator, &len);
-  if (*list_iterator != '\0')
+  if (p && *list_iterator != '\0')
 {
   ++list_iterator;
   p[len] = '\0';



___

Reply to this item at:

  

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

[bug #45799] handle local variables in $(call )

[Jouke Witteveen]

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

The recently released GNU Make 4.4 has the $(let ...) function, which provides
local variables. The current feature request is thus taken care of.


___

Reply to this item at:

  

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

[bug #3656] .MUTEX directive

[Jouke Witteveen]

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

In the recently released GNU Make 4.4, .NOTPARALLEL accepts prerequisites,
thus this feature request is effectively satisfied!


___

Reply to this item at:

  

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

Re: [PATCH] Fix nonzero detection in integer parsing

[Jouke Witteveen]

On Mon, Dec 20, 2021 at 8:24 PM Jouke Witteveen  wrote:
>
> * src/function.c (parse_textint): Fix nonzero detection.
> * tests/scripts/functions/intcmp: Extend test coverage.
> ---
>
> The `*p++ == '0'` test should have been `*p++ != '0'` and did not need
> to be repeated for every digit anyway.  By pure luck, all numbers used
> in tests included a digit '0', so this mistake went unnoticed.

I would like to draw attention to this patch again, since without it
the intcmp function is misbehaving.

Thanks,
- Jouke

>
>  src/function.c |  6 +++---
>  tests/scripts/functions/intcmp | 11 +++
>  2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/src/function.c b/src/function.c
> index c107a387..28594942 100644
> --- a/src/function.c
> +++ b/src/function.c
> @@ -1293,7 +1293,7 @@ parse_textint (const char *number, const char *msg,
>const char *after_sign, *after_number;
>const char *p = next_token (number);
>int negative = *p == '-';
> -  int nonzero = 0;
> +  int nonzero;
>
>if (*p == '\0')
>  OS (fatal, *expanding_var, _("%s: empty value"), msg);
> @@ -1306,9 +1306,9 @@ parse_textint (const char *number, const char *msg,
>*numstart = p;
>
>while (ISDIGIT (*p))
> -if (*p++ == '0')
> -  nonzero = 1;
> +p++;
>after_number = p;
> +  nonzero = *numstart != after_number;
>*sign = negative ? -nonzero : nonzero;
>
>/* Check for extra non-whitespace stuff after the value.  */
> diff --git a/tests/scripts/functions/intcmp b/tests/scripts/functions/intcmp
> index 7466ff97..24e25b22 100644
> --- a/tests/scripts/functions/intcmp
> +++ b/tests/scripts/functions/intcmp
> @@ -9,15 +9,18 @@ n = -10
>  # Zero
>  z = 0
>  # Positive
> -p = 10
> +p = 888
>  min = -9223372036854775808
>  max = 9223372036854775807
>  huge = 8857889956778499040639527525992734031025567913257255490371761260681427
>  .RECIPEPREFIX = >
>  all:
>  > @echo 0_1 $(intcmp $n,$n)
> -> @echo 0_2 $(intcmp $n,$z)
> -> @echo 0_3 $(intcmp $z,$n)
> +> @echo 0_2 $(intcmp $z,$z)
> +> @echo 0_3 $(intcmp -$z,$z)
> +> @echo 0_4 $(intcmp $p,$p)
> +> @echo 0_5 $(intcmp $n,$z)
> +> @echo 0_6 $(intcmp $z,$n)
>  > @echo 1_1 $(intcmp $n,$n,$(shell echo lt))
>  > @echo 1_2 $(intcmp $n,$z,$(shell echo lt))
>  > @echo 1_3 $(intcmp $z,$n,$(shell echo lt))
> @@ -36,7 +39,7 @@ all:
>  > @echo 5_1 $(intcmp $(huge),-$(huge),lt,eq,gt)
>  > @echo 5_2 $(intcmp -$(huge),-$(huge),lt,eq,gt)
>  > @echo 5_3 $(intcmp +$(huge),$(huge),lt,eq,gt)
> -', '', "0_1 -10\n0_2\n0_3\n1_1\n1_2 lt\n1_3\n2_1 lt\n2_2 ge\n2_3 
> ge\n3_0\n3_1 lt\n3_2 gt\n3_3 eq\n4_0 lt\n4_1 gt\n4_2 eq\n4_3 eq\n5_0 lt\n5_1 
> gt\n5_2 eq\n5_3 eq\n");
> +', '', "0_1 -10\n0_2 0\n0_3 0\n0_4 888\n0_5\n0_6\n1_1\n1_2 lt\n1_3\n2_1 
> lt\n2_2 ge\n2_3 ge\n3_0\n3_1 lt\n3_2 gt\n3_3 eq\n4_0 lt\n4_1 gt\n4_2 eq\n4_3 
> eq\n5_0 lt\n5_1 gt\n5_2 eq\n5_3 eq\n");
>
>  # Test error conditions
>
> --
> 2.34.1
>

[PATCH] Fix nonzero detection in integer parsing

[Jouke Witteveen]

* src/function.c (parse_textint): Fix nonzero detection.
* tests/scripts/functions/intcmp: Extend test coverage.
---

The `*p++ == '0'` test should have been `*p++ != '0'` and did not need
to be repeated for every digit anyway.  By pure luck, all numbers used
in tests included a digit '0', so this mistake went unnoticed.

 src/function.c |  6 +++---
 tests/scripts/functions/intcmp | 11 +++
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/function.c b/src/function.c
index c107a387..28594942 100644
--- a/src/function.c
+++ b/src/function.c
@@ -1293,7 +1293,7 @@ parse_textint (const char *number, const char *msg,
   const char *after_sign, *after_number;
   const char *p = next_token (number);
   int negative = *p == '-';
-  int nonzero = 0;
+  int nonzero;
 
   if (*p == '\0')
 OS (fatal, *expanding_var, _("%s: empty value"), msg);
@@ -1306,9 +1306,9 @@ parse_textint (const char *number, const char *msg,
   *numstart = p;
 
   while (ISDIGIT (*p))
-if (*p++ == '0')
-  nonzero = 1;
+p++;
   after_number = p;
+  nonzero = *numstart != after_number;
   *sign = negative ? -nonzero : nonzero;
 
   /* Check for extra non-whitespace stuff after the value.  */
diff --git a/tests/scripts/functions/intcmp b/tests/scripts/functions/intcmp
index 7466ff97..24e25b22 100644
--- a/tests/scripts/functions/intcmp
+++ b/tests/scripts/functions/intcmp
@@ -9,15 +9,18 @@ n = -10
 # Zero
 z = 0
 # Positive
-p = 10
+p = 888
 min = -9223372036854775808
 max = 9223372036854775807
 huge = 8857889956778499040639527525992734031025567913257255490371761260681427
 .RECIPEPREFIX = >
 all:
 > @echo 0_1 $(intcmp $n,$n)
-> @echo 0_2 $(intcmp $n,$z)
-> @echo 0_3 $(intcmp $z,$n)
+> @echo 0_2 $(intcmp $z,$z)
+> @echo 0_3 $(intcmp -$z,$z)
+> @echo 0_4 $(intcmp $p,$p)
+> @echo 0_5 $(intcmp $n,$z)
+> @echo 0_6 $(intcmp $z,$n)
 > @echo 1_1 $(intcmp $n,$n,$(shell echo lt))
 > @echo 1_2 $(intcmp $n,$z,$(shell echo lt))
 > @echo 1_3 $(intcmp $z,$n,$(shell echo lt))
@@ -36,7 +39,7 @@ all:
 > @echo 5_1 $(intcmp $(huge),-$(huge),lt,eq,gt)
 > @echo 5_2 $(intcmp -$(huge),-$(huge),lt,eq,gt)
 > @echo 5_3 $(intcmp +$(huge),$(huge),lt,eq,gt)
-', '', "0_1 -10\n0_2\n0_3\n1_1\n1_2 lt\n1_3\n2_1 lt\n2_2 ge\n2_3 ge\n3_0\n3_1 
lt\n3_2 gt\n3_3 eq\n4_0 lt\n4_1 gt\n4_2 eq\n4_3 eq\n5_0 lt\n5_1 gt\n5_2 eq\n5_3 
eq\n");
+', '', "0_1 -10\n0_2 0\n0_3 0\n0_4 888\n0_5\n0_6\n1_1\n1_2 lt\n1_3\n2_1 
lt\n2_2 ge\n2_3 ge\n3_0\n3_1 lt\n3_2 gt\n3_3 eq\n4_0 lt\n4_1 gt\n4_2 eq\n4_3 
eq\n5_0 lt\n5_1 gt\n5_2 eq\n5_3 eq\n");
 
 # Test error conditions
 
-- 
2.34.1

Re: Compilation error with GCC

[Jouke Witteveen]

I believe what you are seeing is reported here:
https://savannah.gnu.org/bugs/?60798

The patches "file #52422" and "file #52428" should resolve your issues.

Regards,
- Jouke

On Sun, Dec 19, 2021 at 7:55 PM Mohammad Akhlaghi  wrote:
>
> So sorry, there was a typo in my last mail:
>
> As the previously attached file shows, I was built Make over commit
> 'e62f4cf9a2eaf7' (Sun Nov 28 14:17:55 2021 -0500).
>
> Cheers,
> Mohammad
>
> On 12/19/21 19:49, Mohammad Akhlaghi wrote:
> > Dear GNU Make maintainers,
> >
> > I just cloned Make (with the 'master' branch on commit c5d4b7b2f260c)
> > and bootstrapped it with Gnulib (on commit 64f5221ef20ba) on Arch
> > GNU/Linux and with GCC 11.1.0 and Glibc 2.33.
> >
> > The configuration went nicely, however, during the compilation it
> > crashed with the attached error.
> >
> > Please let me know if any further information is necessary.
> >
> > Thank you,
> > Mohammad
> >
>

[bug #60798] Make does not compile with GCC 11.1.0

[Jouke Witteveen]

Additional Item Attachment, bug #60798 (project make):

File name: SV-60798-Rework-to-silence-GCC11-warning.patch Size:3 KB
   




___

Reply to this item at:

  

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

[bug #60798] Make does not compile with GCC 11.1.0

[Jouke Witteveen]

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

As a sort of exercise, I got rid of the final warning too. This required some
moving code around and I duplicated the `strchr` call and the `p[-1] != '\\'`
test. Let me know if these changes make any sense.

While at it, I also added an update to the string length variable which was
missing from the old code.

With the last two patches, Make compiles with no warnings on my Arch Linux
system with GCC 11.1.0.

(file #52425)
___

Additional Item Attachment:

File name: SV-60798-Rework-to-silence-GCC11-warning.patch Size:3 KB
   




___

Reply to this item at:

  

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

[bug #60798] Make does not compile with GCC 11.1.0

[Jouke Witteveen]

Follow-up Comment #3, bug #60798 (project make):

I expanded the patch a bit to also silence a _maybe-uninitialized_ error.

With GCC11, there is now only one warning remaining, which is the
_return-local-addr_ warning in src/read.c:2534 (find_percent_cached). The code
is correct, but GCC fails to prove that the final stack-bound allocation (if
applicable) will be copied to the string cache.

(file #52422)
___

Additional Item Attachment:

File name: SV-60798-Silence-bogus-GCC10-and-GCC11-warnings.patch Size:3 KB
   




___

Reply to this item at:

  

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

Re: [PATCH] doc: note that $(wildcard) is implemented in terms of glob(3)

[Jouke Witteveen]

On Fri, Dec 3, 2021 at 3:19 PM Ævar Arnfjörð Bjarmason  wrote:
>
> The motivation for this patch is that I've seen code like this in the
> wild:
>
> FILES = $(sort $(wildcard t*.sh))
>
> I thought that it would be documented that that sort of thing was
> redundant, but I didn't find any mention of it in the manual.

Just for reference, the output of $(wildcard) wasn't sorted before
make 4.3. As stated in the relevant bug report:
https://savannah.gnu.org/bugs/index.php?52076
the code you spotted in the wild is in fact the most portable use of
wildcard when the order matters.

Regards,
- Jouke

Re: [bug #60798] Make does not compile with GCC 11.1.0

[Jouke Witteveen]

On Fri, Dec 3, 2021 at 1:26 PM Edward Welbourne  wrote:
>
> Jouke Witteveen (3 December 2021 13:22) wrote:
> > The next warning I get (GCC11) is a _return-local-addr_ warning in
> > src/read.c:2534 (find_percent_cached). Maybe GCC doesn't recognize alloca 
> > as a
> > heap allocation? Just guessing; this warning was not obvious to me.
>
> alloca() is not a heap allocation:
> 
>
> DESCRIPTION
>The  alloca() function allocates size bytes of space in the stack frame
>of the caller.  This temporary space is automatically  freed  when  the
>function that called alloca() returns to its caller.
>
> 
> It is a stack allocation.  So if make is returning an alloca()'d buffer,
> then that is indeed a return of a local address.
>

Ah, thanks! I see what is happening now. GCC fails to see that in case
allocations have happened, new is non-zero and the little block at the
end of the function will make sure that the final string allocation
will be copied to the string cache (and the returned address will
hence not be local).

I currently have no idea how to make this easier to understand for GCC.

[bug #60798] Make does not compile with GCC 11.1.0

[Jouke Witteveen]

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

I couldn't find any signs of GCC becoming clever enough to detect why this
code is correct. Minor code changes can silence the _stringop-overflow_
warnings.

The next warning I get (GCC11) is a _return-local-addr_ warning in
src/read.c:2534 (find_percent_cached). Maybe GCC doesn't recognize alloca as a
heap allocation? Just guessing; this warning was not obvious to me.

(file #52420)
___

Additional Item Attachment:

File name: SV-60798-Silence-bogus-GCC10-stringop-overflow-warni.patch Size:2
KB
   




___

Reply to this item at:

  

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

[PATCH] * doc/make.texi (Errors): Document integer width

[Jouke Witteveen]

---

There was some talk of supporting arbitrarily large integers. This was
mostly an academic discussion, since the currently supported range
(64 bits) should cover all use cases.

On Wed, Dec 1, 2021 at 8:49 PM Paul Smith  wrote:
>
> allowing practicality to drive simplification in code turns software
> development from an abstract mathematical science into a practical
> engineering problem :).

In fact, returning an error on out-of-range numbers might uncover bugs
that would have otherwise gone unnoticed.

The documentation change below simply mentions the supported width for
completeness.

Regards,
- Jouke


 doc/make.texi | 5 +
 1 file changed, 5 insertions(+)

diff --git a/doc/make.texi b/doc/make.texi
index 3dea24f1..cb2842b5 100644
--- a/doc/make.texi
+++ b/doc/make.texi
@@ -13035,6 +13035,11 @@ This means you haven't provided the requisite number 
of arguments for
 this function.  See the documentation of the function for a description
 of its arguments.  @xref{Functions, ,Functions for Transforming Text}.
 
+@item Numerical result out of range: `@var{xxx}'.  Stop.
+This means you have provided a numerical argument outside the range of
+supported values. This is likely unintended, because integers in
+@code{make} have a width of at least @w{64 bits}.
+
 @item missing target pattern.  Stop.
 @itemx multiple target patterns.  Stop.
 @itemx target pattern contains no `%'.  Stop.
-- 
2.34.1

Re: [PATCH 3/3] Introduce $(compare ...) for numerical comparison

[Jouke Witteveen]

On Sun, Nov 28, 2021 at 3:20 PM Paul Smith  wrote:
>
> On Sun, 2021-11-28 at 14:57 +0100, Jouke Witteveen wrote:
> > > Since the two arguments are equal, it doesn't matter which of LHS
> > > or RHS we return.
> >
> > They could differ for instance when one of them contains a '+'-sign.
> > My reason for using LHS is that we already have a string for it.
>
> I don't think that it's necessary return the exact string.  If the user
> wanted a string match they can do that other ways.  Returning the
> "absolute value" (stripping leading +/-, leading 0's, etc.) seems more
> useful to me.

I fully agree, but was not aware of the robustness of INTSTR_LENGTH.
It felt a bit fragile when I spotted its definition in makeint.h.

> > > However, now that I think about it I need to change the code more: we
> > > need to be using "long long" here not just "long".  While on Linux etc.
> > > a "long" is 8 bytes, on Windows "long" is only 4 bytes.
> >
> > I was hoping this would not be necessary, and I cannot think of a
> > typical use case where make is a good fit for dealing with large
> > integers. The benefit of "long" is that strtol is more widely
> > available than strtoll.
>
> I see what you mean, but I _really_ don't like the idea of GNU make
> working differently on different platforms, even if such use cases are
> rare.  I can imagine a situation where, for example, someone wants to
> compare the sizes of files and if one of the files is >4G then it will
> work on Linux and fail on Windows.

File sizes are an interesting application indeed. If you want me to
change the patches to use strtoll, I would need some help since I am
not sure how to set things up so that we get a fallback implementation
on platforms where it is missing.

Re: [PATCH 3/3] Introduce $(compare ...) for numerical comparison

[Jouke Witteveen]

On Sun, Nov 28, 2021 at 3:33 PM Paul Smith  wrote:
>
> On Sun, 2021-11-28 at 08:24 +0100, Jouke Witteveen wrote:
> > On the user side, strcmp could now probably be defined something like
> > $(and $(intcmp $(words $1),$(words $2)),$(findstring x $1 x,x $2 x))
>
> I don't think this is equivalent since a putative strcmp would also do
> greater / less than comparison (like intcmp does).  Of course that
> leads into all sorts of i18n / locale issues, but likely we'd just punt
> (like C's strcmp does) and use ASCII ordering; we've already taken that
> route elsewhere.

This was only intended as a demonstration of how the two-argument
strcmp function could be implemented already (I initially forgot the
spaces around the 'x's). This would serve as a test for exact
string-wise equality.

If anything, this could mean that there is no reason to spend time on
adding strcmp now. It's all a digression and I agree with everything
you said about it.

Re: [PATCH 3/3] Introduce $(compare ...) for numerical comparison

[Jouke Witteveen]

On Sun, Nov 28, 2021 at 2:45 PM Paul Smith  wrote:
>
> On Sun, 2021-11-28 at 08:24 +0100, Jouke Witteveen wrote:
> > Thanks for sending this message, I would have otherwise prepared and
> > sent an updated patch series today. My plan was to expand to RHS in
> > the two-argument case if both values are equal. I assume you also
> > updated the documentation where needed? If so, there's nothing I have
> > to add and I'm looking forward to the new functionality.
>
> Yes, I updated the docs.
>
> Since the two arguments are equal, it doesn't matter which of LHS or
> RHS we return.

They could differ for instance when one of them contains a '+'-sign.
My reason for using LHS is that we already have a string for it. By
using sprintf, we need to make the buffer big enough, which in turn
requires INTSTR_LENGTH to be fitting for whatever width a long has on
the current platform.

> The change I made was:
>
>   argv += 2;
>
>   if (*argv == NULL)
> {
>   if (lhs == rhs)
> {
>   char buf[INTSTR_LENGTH+1];
>   sprintf (buf, "%ld", lhs);
>   o = variable_buffer_output(o, buf, strlen (buf));
> }
>   return o;
> }
>
> However, now that I think about it I need to change the code more: we
> need to be using "long long" here not just "long".  While on Linux etc.
> a "long" is 8 bytes, on Windows "long" is only 4 bytes.

I was hoping this would not be necessary, and I cannot think of a
typical use case where make is a good fit for dealing with large
integers. The benefit of "long" is that strtol is more widely
available than strtoll.

Regards,
- Jouke

Re: [PATCH 3/3] Introduce $(compare ...) for numerical comparison

[Jouke Witteveen]

On Sun, Nov 28, 2021 at 5:02 AM Paul Smith  wrote:
>
> On Mon, 2021-11-15 at 20:49 +0100, Jouke Witteveen wrote:
> > It may be obscure, but how about we implement this as well? Sure, the
> > two-argument form of $(compare) will be a little inconsistent, but it
> > may be useful.
>
> I applied this three-patch set.
>
> I left the argument order as you originally specified it, and I added
> support for a 2-argument form that provides a trivial equality as
> above.  Maybe it's weird but I think it might be useful.
>
> The one change I made is changing the name of the function from
> "compare" to "intcmp": I felt "compare" was too generic.  Sometime we
> may want to introduce other types of comparison such as "strcmp" etc.
>
> Let me know what you think about this: I've only applied it to my local
> Git repository but not pushed it yet.
>

Hi Paul,

Thanks for sending this message, I would have otherwise prepared and
sent an updated patch series today. My plan was to expand to RHS in
the two-argument case if both values are equal. I assume you also
updated the documentation where needed? If so, there's nothing I have
to add and I'm looking forward to the new functionality.

On the user side, strcmp could now probably be defined something like
$(and $(intcmp $(words $1),$(words $2)),$(findstring x$1x,x$2x))

Thanks,
- Jouke

Re: [PATCH 3/3] Introduce $(compare ...) for numerical comparison

[Jouke Witteveen]

On Tue, Nov 16, 2021 at 10:28 AM Edward Welbourne
 wrote:
>
> On Sun, Nov 14, 2021 at 8:42 PM Paul Smith  wrote:
> > It's even possible to allow $(compare ,) with no other
> > arguments and say that if they are equal then it expands to the value,
> > else it expands to the empty string, to give a very short-circuited
> > equality statement.
>
> I should point out one bug in that: if the two parameters are equal
> *but empty* you're going to mistake that for them being different.

Empty strings will not be accepted as  or , since those
arguments are parsed as numbers. A case like
  $(compare +0,-0)
would expand to "0" (without quotes), which is how the value would be
formatted by the standard C routines.
I don't think there is a bug here.

Regards,
- Jouke

Re: [PATCH 3/3] Introduce $(compare ...) for numerical comparison

[Jouke Witteveen]

On Sun, Nov 14, 2021 at 8:42 PM Paul Smith  wrote:
>
> On Wed, 2021-11-10 at 18:19 +0100, Jouke Witteveen wrote:
> > On Mon, Nov 8, 2021 at 4:08 PM Paul Smith  wrote:
> >
> > > On Fri, 2021-07-16 at 14:04 +0200, Jouke Witteveen wrote:
> > > > $(compare 
> > > > @var{lhs},@var{rhs},@var{lt-part}[,@var{eq-part}[,@var{gt-part}]])
> > > Let me ask this: would it be better to use a format of:
> > >$(compare , , [, [, ]])
> >
> >   $(if $(compare ,,,they_are_equal,),,)
> >
> > In fact, in the original design you could get away with just the
> > three-argument version of $(compare) in combination with $(if),
> > $(and) and $(or). This is not the case for the alternative design.
>
> Can you explain this, perhaps with an example or two?  I don't quite
> follow.  It seems that:
>
>   $(if $(compare ,,equal),,)
>
> works fine with the alternative syntax.

What I was trying to say is that the two versions differ only in the
abilities provided by their thee-argument forms. It's not much of a
reason to favor one over the other, but the three-argument form of the
original proposal (less-than) is 'complete' in the sense that all of
<, >, =, and their negations can be expressed as boolean combinations
of it. The same is not true of the three-argument form of the
alternative design (e.g. less-than cannot be expressed using the
three-argument version of that design alone).

> It's even possible to allow $(compare ,) with no other
> arguments and say that if they are equal then it expands to the value,
> else it expands to the empty string, to give a very short-circuited
> equality statement.

It may be obscure, but how about we implement this as well? Sure, the
two-argument form of $(compare) will be a little inconsistent, but it
may be useful.

> > I also took a look at other languages. Nearly everywhere I could find
> > three-way comparisons, the possible outcomes are listed as LT, EQ,
> > GT, in that order.
>
> I'm certainly willing to be convinced that the original idea is better,
> but this argument doesn't hold much water for me.  I don't think that
> changing the order of the extra arguments will cause much cognitive
> dissonance for people.

Interesting. The fact that I saw the mental model of the LT, EQ, GT
ordering of outcomes reflected so often was convincing for me that
this would be the more unsurprising design. I'm not saying the
alternative will cause cognitive dissonance, just that the occasional
user of make might not need to consult the reference documentation as
much trying to recall how the compare function works. Admittedly, the
difference, if any, will likely be negligible.

Regards,
- Jouke

P.S.
I started this series saying that I see no merit in discussing
possible extensions and alternatives. Regarding version comparison (a
minefield if you ask me), I'd just like to point out that you can do
basic parsing of a version with
  $(let major minor patch, $(subst ., ,$(version)),)

Re: [PATCH 3/3] Introduce $(compare ...) for numerical comparison

[Jouke Witteveen]

On Mon, Nov 8, 2021 at 4:08 PM Paul Smith  wrote:
>
> On Fri, 2021-07-16 at 14:04 +0200, Jouke Witteveen wrote:
> > +@item $(compare 
> > @var{lhs},@var{rhs},@var{lt-part}[,@var{eq-part}[,@var{gt-part}]])
>
> Let me ask this: would it be better to use a format of:
>
>   $(compare , , [, [, ]])
>
> Then the rule is, if the values are equal we get the  part, if lhs
> is less than rhs we get , and if lhs is greater than rhs we get
> .
>
> If  is not present then the invocation devolves to:
>
>   $(compare , , , )
>
> that is, the fourth arg is used for not equal.
>
> If  is also not present then  is used if the value is equal
> else it expands to the empty string.
>

Cool, an alternative design that has something to go for it! Here is
why I like the original design better.

The only real difference is in four-argument usage. This alternative
trades the 'ordered' nature of the arguments for favoring equality
testing. To some degree, equality testing is already possible through
string-based equality testing, and even if you really want to do a
numerical equality check, the original design allows you to do

  $(if $(compare ,,,they_are_equal,),,)

In fact, in the original design you could get away with just the
three-argument version of $(compare) in combination with $(if), $(and)
and $(or). This is not the case for the alternative design.

I also took a look at other languages. Nearly everywhere I could find
three-way comparisons, the possible outcomes are listed as LT, EQ, GT,
in that order. Fortran apparently had an "Arithmetic IF", that was
also ordered as in the original design of $(compare).

Regards,
- Jouke

Re: [PATCH 3/3] Introduce $(compare ...) for numerical comparison

[Jouke Witteveen]

On Mon, Sep 13, 2021 at 8:39 PM Jouke Witteveen  wrote:
>
> On Fri, Jul 16, 2021 at 2:04 PM Jouke Witteveen  wrote:
> >
> > Numbers can come from $(words ...), automatic variables such as
> > $(MAKELEVEL), from environment variables, or from shell output such as
> > through $(shell expr ...).  The $(compare ...) function allows
> > conditional evaluation controlled by numerical variables.
> >
> > * NEWS: Announce this feature.
> > * doc/make.texi (Functions for Conditionals): Document 'compare'.
> > * src/function.c (func_compare): Create the 'compare' built-in function.
> > * tests/scripts/functions/compare: Test the 'compare' built-in function.
> > ---
> >
> > This is a cleaned-up and documented version of a proposal submitted a
> > year ago. The interface was conceived by Edward Welbourne and myself.
> > Personally, I would not introduce mathematical operators to make, but
> > given that numbers are a thing and they do pop up in variables, a
> > comparison function makes sense to me.
> >
> > Thanks for considering!
> > - Jouke
>
> I would still be interested in seeing this patch series I sent two
> months ago considered. I am not in a rush: this is just a friendly
> reminder.
>
> Regards,
> - Jouke

Another friendly reminder. Four months have passed since I sent this
patch series. If there is anything I can do to advance it, please let
me know.

I would like to emphasize that I do not think it is a good idea to
extend the proposed functionality any further. Integer arithmetic with
`signed long`s is the preferred style in Posix shells, making integers
far more prevalent than floats in the domain that Make operates in. As
for math-beyond-comparison, there is no obvious syntax for variadic
functions for non-commutative mathematical operations. For these
reasons, I think the current $(compare) proposal should be considered
on its own merit, without discussing possible extensions.

Regards,
- Jouke

Re: [PATCH 3/3] Introduce $(compare ...) for numerical comparison

[Jouke Witteveen]

On Fri, Jul 16, 2021 at 2:04 PM Jouke Witteveen  wrote:
>
> Numbers can come from $(words ...), automatic variables such as
> $(MAKELEVEL), from environment variables, or from shell output such as
> through $(shell expr ...).  The $(compare ...) function allows
> conditional evaluation controlled by numerical variables.
>
> * NEWS: Announce this feature.
> * doc/make.texi (Functions for Conditionals): Document 'compare'.
> * src/function.c (func_compare): Create the 'compare' built-in function.
> * tests/scripts/functions/compare: Test the 'compare' built-in function.
> ---
>
> This is a cleaned-up and documented version of a proposal submitted a
> year ago. The interface was conceived by Edward Welbourne and myself.
> Personally, I would not introduce mathematical operators to make, but
> given that numbers are a thing and they do pop up in variables, a
> comparison function makes sense to me.
>
> Thanks for considering!
> - Jouke

I would still be interested in seeing this patch series I sent two
months ago considered. I am not in a rush: this is just a friendly
reminder.

Regards,
- Jouke

[PATCH 3/3] Introduce $(compare ...) for numerical comparison

[Jouke Witteveen]

Numbers can come from $(words ...), automatic variables such as
$(MAKELEVEL), from environment variables, or from shell output such as
through $(shell expr ...).  The $(compare ...) function allows
conditional evaluation controlled by numerical variables.

* NEWS: Announce this feature.
* doc/make.texi (Functions for Conditionals): Document 'compare'.
* src/function.c (func_compare): Create the 'compare' built-in function.
* tests/scripts/functions/compare: Test the 'compare' built-in function.
---

This is a cleaned-up and documented version of a proposal submitted a
year ago. The interface was conceived by Edward Welbourne and myself.
Personally, I would not introduce mathematical operators to make, but
given that numbers are a thing and they do pop up in variables, a
comparison function makes sense to me.

Thanks for considering!
- Jouke

 NEWS|  4 +++
 doc/make.texi   | 36 -
 src/function.c  | 46 ++
 tests/scripts/functions/compare | 57 +
 4 files changed, 142 insertions(+), 1 deletion(-)
 create mode 100644 tests/scripts/functions/compare

diff --git a/NEWS b/NEWS
index 5356260..6a3a744 100644
--- a/NEWS
+++ b/NEWS
@@ -40,6 +40,10 @@ 
https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=109&se
   user-defined function and they will not impact global variable assignments.
   Implementation provided by Jouke Witteveen 
 
+* New feature: The $(compare ...) function
+  This function allows conditional evaluation controlled by a numerical
+  comparison.
+
 * New debug option "print" will show the recipe to be run, even when silent
   mode is set.
 
diff --git a/doc/make.texi b/doc/make.texi
index 1ebf6ae..14a59f6 100644
--- a/doc/make.texi
+++ b/doc/make.texi
@@ -7654,7 +7654,7 @@ the file names to refer to an existing file or directory. 
 Use the
 @section Functions for Conditionals
 @findex if
 @cindex conditional expansion
-There are three functions that provide conditional expansion.  A key
+There are four functions that provide conditional expansion.  A key
 aspect of these functions is that not all of the arguments are
 expanded initially.  Only those arguments which need to be expanded,
 will be expanded.
@@ -7701,6 +7701,32 @@ empty string the processing stops and the result of the 
expansion is
 the empty string.  If all arguments expand to a non-empty string then
 the result of the expansion is the expansion of the last argument.
 
+@item $(compare 
@var{lhs},@var{rhs},@var{lt-part}[,@var{eq-part}[,@var{gt-part}]])
+@findex compare
+The @code{compare} function provides support for numerical comparison of
+integers.  This function has no counterpart among the GNU @code{make}
+makefile conditionals.
+
+The left-hand side, @var{lhs}, and right-hand side, @var{rhs}, are
+expanded and parsed as integral numbers in base 10.  Expansion of the
+remaining arguments is controlled by how the numerical left-hand side
+compares to the numerical right-hand side.
+
+If the left-hand side is strictly less than the right-hand side, the
+entire @code{compare} function evaluates to the expansion of the third
+argument, @var{lt-part}.  If both sides compare equal, then the fourth
+argument, @var{eq-part}, is expanded and becomes the result of the
+@code{compare} function.  If the left-hand side is strictly greater than
+the right-hand side, then the @code{compare} function evaluates to the
+expansion of the fifth argument, @var{gt-part}.
+
+In case of missing arguments, @var{gt-part} defaults to @var{eq-part},
+and @var{eq-part} defaults to the empty string.  Thus both
+@samp{$(compare 9,7,hello)} and @samp{$(compare 9,7,hello,world,)}
+evaluate to the empty string, while @samp{$(compare 9,7,hello,world)}
+(notice the absence of a comma after @code{world}) evaluates to
+@samp{world}.
+
 @end table
 
 @node Let Function, Foreach Function, Conditional Functions, Functions
@@ -12523,6 +12549,14 @@ all expansions result in a non-empty string, 
substitute the expansion
 of the last @var{condition}.@*
 @xref{Conditional Functions, ,Functions for Conditionals}.
 
+@item $(compare 
@var{lhs},@var{rhs},@var{lt-part}[,@var{eq-part}[,@var{gt-part}]])
+Compare @var{lhs} and @var{rhs} numerically; substitute the expansion of
+@var{lt-part}, @var{eq-part}, or @var{gt-part} depending on whether the
+left-hand side is less-than, equal-to, or greater-than the right-hand
+side, respectively.  If @var{gt-part} is left out, it defaults to
+@var{eq-part}.@*
+@xref{Conditional Functions, ,Functions for Conditionals}.
+
 @item $(call @var{var},@var{param},@dots{})
 Evaluate the variable @var{var} replacing any references to @code{$(1)},
 @code{$(2)} with the first, second, etc.@: @var{param} values.@*
diff --git a/src/function.c b/src/function.c
index 964169a..587786a 100644
--- a/src/function.c
+++ b/src/function.c
@@ -1272,6 +12

[PATCH 2/3] Use strtol instead of atoi

[Jouke Witteveen]

strtol is part of C89 and a fallback is provided by gnulib

* src/function.c (func_word, func_wordlist): Use strtol instead of atoi
* test/scripts/functions/word: Add out-of-range verification testing
---
 bootstrap.conf   |  1 +
 src/function.c   | 51 ++--
 tests/scripts/functions/word | 12 ++---
 3 files changed, 36 insertions(+), 28 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index 7c4c9ab..c9c3668 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -50,4 +50,5 @@ findprog-in
 getloadavg
 host-cpu-c-abi
 strerror
+strtol
 make-glob"
diff --git a/src/function.c b/src/function.c
index 396f129..964169a 100644
--- a/src/function.c
+++ b/src/function.c
@@ -766,19 +766,24 @@ strip_whitespace (const char **begpp, const char **endpp)
   return (char *)*begpp;
 }
 
-static void
-check_numeric (const char *s, const char *msg)
+static long
+parse_numeric (const char *s, const char *msg)
 {
-  const char *end = s + strlen (s) - 1;
   const char *beg = s;
-  strip_whitespace (&s, &end);
-
-  for (; s <= end; ++s)
-if (!ISDIGIT (*s))  /* ISDIGIT only evals its arg once: see makeint.h.  */
-  break;
+  const char *end = s + strlen (s) - 1;
+  char *endp;
+  long num;
+  strip_whitespace (&beg, &end);
 
-  if (s <= end || end - beg < 0)
-OSS (fatal, *expanding_var, "%s: '%s'", msg, beg);
+  errno = 0;
+  num = strtol (beg, &endp, 10);
+  if (errno == ERANGE)
+OSS (fatal, *expanding_var, "%s: '%s'", strerror (errno), s);
+  else if (endp == beg || endp <= end)
+/* Empty or non-numeric input */
+OSS (fatal, *expanding_var, "%s: '%s'", msg, s);
+
+  return num;
 }
 
 
@@ -788,13 +793,11 @@ func_word (char *o, char **argv, const char *funcname 
UNUSED)
 {
   const char *end_p;
   const char *p;
-  int i;
+  long i;
 
-  /* Check the first argument.  */
-  check_numeric (argv[0], _("non-numeric first argument to 'word' function"));
-  i = atoi (argv[0]);
-
-  if (i == 0)
+  i = parse_numeric (argv[0],
+ _("non-numeric first argument to 'word' function"));
+  if (i <= 0)
 O (fatal, *expanding_var,
_("first argument to 'word' function must be greater than 0"));
 
@@ -812,20 +815,18 @@ func_word (char *o, char **argv, const char *funcname 
UNUSED)
 static char *
 func_wordlist (char *o, char **argv, const char *funcname UNUSED)
 {
-  int start, count;
+  long start, stop, count;
 
-  /* Check the arguments.  */
-  check_numeric (argv[0],
- _("non-numeric first argument to 'wordlist' function"));
-  check_numeric (argv[1],
- _("non-numeric second argument to 'wordlist' function"));
+  start = parse_numeric (argv[0],
+ _("non-numeric first argument to 'wordlist' 
function"));
+  stop = parse_numeric (argv[1],
+_("non-numeric second argument to 'wordlist' 
function"));
 
-  start = atoi (argv[0]);
   if (start < 1)
 ON (fatal, *expanding_var,
-"invalid first argument to 'wordlist' function: '%d'", start);
+"invalid first argument to 'wordlist' function: '%ld'", start);
 
-  count = atoi (argv[1]) - start + 1;
+  count = stop - start + 1;
 
   if (count > 0)
 {
diff --git a/tests/scripts/functions/word b/tests/scripts/functions/word
index 4dcc940..044bc94 100644
--- a/tests/scripts/functions/word
+++ b/tests/scripts/functions/word
@@ -51,6 +51,7 @@ run_make_test('FOO = foo bar biz baz
 word-e1: ; @echo $(word ,$(FOO))
 word-e2: ; @echo $(word abc ,$(FOO))
 word-e3: ; @echo $(word 1a,$(FOO))
+word-e4: ; @echo $(word 999,$(FOO))
 
 wordlist-e1: ; @echo $(wordlist ,,$(FOO))
 wordlist-e2: ; @echo $(wordlist abc ,,$(FOO))
@@ -69,19 +70,24 @@ run_make_test(undef,
   "#MAKEFILE#:5: *** non-numeric first argument to 'word' 
function: '1a'.  Stop.",
   512);
 
+run_make_test(undef,
+  'word-e4',
+  "#MAKEFILE#:6: *** Numerical result out of range: 
'999'.  Stop.",
+  512);
+
 run_make_test(undef,
   'wordlist-e1',
-  "#MAKEFILE#:7: *** non-numeric first argument to 'wordlist' 
function: ''.  Stop.",
+  "#MAKEFILE#:8: *** non-numeric first argument to 'wordlist' 
function: ''.  Stop.",
   512);
 
 run_make_test(undef,
   'wordlist-e2',
-  "#MAKEFILE#:8: *** non-numeric first argument to 'wordlist' 
function: 'abc '.  Stop.",
+  "#MAKEFILE#:9: *** non-numeric first argument to 'wordlist' 
function: 'abc '.  Stop.",
   512);
 
 run_make_test(undef,
   'wordlist-e3',
-  "#MAKEFILE#:9: *** non-numeric second argument to 'wordlist' 
function: ' 12a '.  Stop.",
+  "#MAKEFILE#:10: *** non-numeric second argument to 'wordlist' 
function: ' 12a '.  Stop.",
   512);
 
 # Test error conditions again, but this time in a variable reference
-- 
2.32.0

[PATCH 1/3] * src/makeint.h: Removed unused atol declaration

[Jouke Witteveen]

---
 src/makeint.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/makeint.h b/src/makeint.h
index fcfb7bd..ca6d49d 100644
--- a/src/makeint.h
+++ b/src/makeint.h
@@ -631,7 +631,6 @@ void spin (const char* suffix);
 
 #if !defined (__GNU_LIBRARY__) && !defined (POSIX) && !defined 
(_POSIX_VERSION) && !defined(WINDOWS32)
 
-long int atol ();
 # ifndef VMS
 long int lseek ();
 # endif
-- 
2.32.0

Re: [PATCH] More correctly describe the scope of variables

[Jouke Witteveen]

On Fri, Jan 29, 2021 at 8:37 AM Jouke Witteveen  wrote:
> On Fri, Dec 25, 2020 at 7:00 PM Jouke Witteveen  wrote:
> >
> > * NEWS: Use "local" instead of the incorrect "lexically-scoped".
> > * doc/make.texi: Refer to let/foreach variables as local variables.
> > ---
> > This is an erratum on the addition of $(let ...). During an early review
> > of $(let ...), thutt cautioned that it did not implement "full semantic
> > scoping" [sic]. While I did not understand fully what they meant by
> > that, I countered that it was not intended to determine a scope based on
> > for example which file a definition of a rule occurred in, but simply by
> > the parentheses delimiting the let expression. Only in that sense was it
> > lexical scoping. Technically, make variables are dynamically scoped.
> >
> > This patch replaces "lexically scoped" not by "dynamically scoped", but
> > by "local", since that is the whole point after all. It also includes
> > variables with local scope (from let and foreach) in several other
> > places where variables are discussed, and makes explicit that variables
> > in make are dynamically scoped.
>
> I think this is ready to go in and that it would be wise to not
> release with the unchanged NEWS.
> Was this patch simply missed due to end-of-year activities?

Another month, another reminder.

Cheers,
- Jouke

Re: [PATCH] More correctly describe the scope of variables

[Jouke Witteveen]

On Fri, Dec 25, 2020 at 7:00 PM Jouke Witteveen  wrote:
>
> * NEWS: Use "local" instead of the incorrect "lexically-scoped".
> * doc/make.texi: Refer to let/foreach variables as local variables.
> ---
> This is an erratum on the addition of $(let ...). During an early review
> of $(let ...), thutt cautioned that it did not implement "full semantic
> scoping" [sic]. While I did not understand fully what they meant by
> that, I countered that it was not intended to determine a scope based on
> for example which file a definition of a rule occurred in, but simply by
> the parentheses delimiting the let expression. Only in that sense was it
> lexical scoping. Technically, make variables are dynamically scoped.
>
> This patch replaces "lexically scoped" not by "dynamically scoped", but
> by "local", since that is the whole point after all. It also includes
> variables with local scope (from let and foreach) in several other
> places where variables are discussed, and makes explicit that variables
> in make are dynamically scoped.

I think this is ready to go in and that it would be wise to not
release with the unchanged NEWS.
Was this patch simply missed due to end-of-year activities?

Regards,
- Jouke

Re: [PATCH] More correctly describe the scope of variables

[Jouke Witteveen]

On Fri, Dec 25, 2020 at 7:58 PM Pete Dietl  wrote:
> Now for Make, there are no semantics for telling whether or not it is 
> lexically or dynamically scoped. This is because previously the only local 
> variables were the function arguments. Since these arguments are always named 
> the same thing, even if the language were dynamically scoped, successive 
> function calls would create new function argument variables which shadow the 
> previous ones. If you try to use eval to create functions or variables on the 
> fly, it works but both are added to the global scope.

There is an easy demonstration (adapted from
https://en.wikipedia.org/wiki/Scope_(computer_science) ):

x=1
g=$(info $x)
f=$(foreach x,3,$g)
$f
$(info $x)

Here, `foreach` can be replaced by `let`. The first output is "3",
demonstrating dynamic scoping.
Behavior aside, the source code of GNU make is structured so that make
has dynamic scoping.

Apart from these remarks, I agree with your analysis and have also
observed that assignments in $(eval) will assign at the global scope
(and not the scope at the top of the stack). I am not sure whether
that is a feature or a bug.

Regards,
- Jouke

>
> On Fri, Dec 25, 2020 at 10:00 AM Jouke Witteveen  
> wrote:
>>
>> * NEWS: Use "local" instead of the incorrect "lexically-scoped".
>> * doc/make.texi: Refer to let/foreach variables as local variables.
>> ---
>> This is an erratum on the addition of $(let ...). During an early review
>> of $(let ...), thutt cautioned that it did not implement "full semantic
>> scoping" [sic]. While I did not understand fully what they meant by
>> that, I countered that it was not intended to determine a scope based on
>> for example which file a definition of a rule occurred in, but simply by
>> the parentheses delimiting the let expression. Only in that sense was it
>> lexical scoping. Technically, make variables are dynamically scoped.
>>
>> This patch replaces "lexically scoped" not by "dynamically scoped", but
>> by "local", since that is the whole point after all. It also includes
>> variables with local scope (from let and foreach) in several other
>> places where variables are discussed, and makes explicit that variables
>> in make are dynamically scoped.
>>
>>  NEWS  |  4 ++--
>>  doc/make.texi | 21 +++--
>>  2 files changed, 17 insertions(+), 8 deletions(-)
>>

[PATCH] More correctly describe the scope of variables

[Jouke Witteveen]

* NEWS: Use "local" instead of the incorrect "lexically-scoped".
* doc/make.texi: Refer to let/foreach variables as local variables.
---
This is an erratum on the addition of $(let ...). During an early review
of $(let ...), thutt cautioned that it did not implement "full semantic
scoping" [sic]. While I did not understand fully what they meant by
that, I countered that it was not intended to determine a scope based on
for example which file a definition of a rule occurred in, but simply by
the parentheses delimiting the let expression. Only in that sense was it
lexical scoping. Technically, make variables are dynamically scoped.

This patch replaces "lexically scoped" not by "dynamically scoped", but
by "local", since that is the whole point after all. It also includes
variables with local scope (from let and foreach) in several other
places where variables are discussed, and makes explicit that variables
in make are dynamically scoped.

 NEWS  |  4 ++--
 doc/make.texi | 21 +++--
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index 5d71488..5356260 100644
--- a/NEWS
+++ b/NEWS
@@ -35,8 +35,8 @@ 
https://sv.gnu.org/bugs/index.php?group=make&report_id=111&fix_release_id=109&se
   The configure script should verify the compiler has these features.
 
 * New feature: The $(let ...) function
-  This function allows user-defined functions to provide a lexically-scoped
-  set of variables: values can be assigned to these variables from within the
+  This function allows user-defined functions to define a set of local
+  variables: values can be assigned to these variables from within the
   user-defined function and they will not impact global variable assignments.
   Implementation provided by Jouke Witteveen 
 
diff --git a/doc/make.texi b/doc/make.texi
index fe64ec2..d7891c9 100644
--- a/doc/make.texi
+++ b/doc/make.texi
@@ -276,7 +276,7 @@ Functions for Transforming Text
 * Text Functions::  General-purpose text manipulation functions.
 * File Name Functions:: Functions for manipulating file names.
 * Conditional Functions::   Functions that implement conditions.
-* Let Function::Lexically scoped variables.
+* Let Function::Local variables.
 * Foreach Function::Repeat some text with controlled variation.
 * File Function::   Write text to a file.
 * Call Function::   Expand a user-defined function.
@@ -5204,7 +5204,9 @@ variables are called @dfn{macros}.)
 Variables and functions in all parts of a makefile are expanded when
 read, except for in recipes, the right-hand sides of variable
 definitions using @samp{=}, and the bodies of variable definitions
-using the @code{define} directive.@refill
+using the @code{define} directive.  The value a variable expands to is
+that of its most recent definition at the time of expansion.  In other
+words, variables are dynamically scoped.
 
 Variables can represent lists of file names, options to pass to compilers,
 programs to run, directories to look in for source files, directories to
@@ -5792,6 +5794,11 @@ You can specify a value in the makefile, either
 with an assignment (@pxref{Setting, ,Setting Variables}) or with a
 verbatim definition (@pxref{Multi-Line, ,Defining Multi-Line 
Variables}).@refill
 
+@item
+You can specify a short-lived value with the @code{let} function
+(@pxref{Let Function}) or with the @code{foreach} function
+(@pxref{Foreach Function}).
+
 @item
 Variables in the environment become @code{make} variables.
 @xref{Environment, ,Variables from the Environment}.
@@ -6274,10 +6281,12 @@ the Shell}.@refill
 
 Variable values in @code{make} are usually global; that is, they are the
 same regardless of where they are evaluated (unless they're reset, of
-course).  One exception to that is automatic variables
+course).  Exceptions to that are variables defined with the @code{let}
+function (@pxref{Let Function}) or the @code{foreach} function
+(@pxref{Foreach Function}, and automatic variables
 (@pxref{Automatic Variables}).
 
-The other exception is @dfn{target-specific variable values}.  This
+Another exception are @dfn{target-specific variable values}.  This
 feature allows you to define different values for the same variable,
 based on the target that @code{make} is currently building.  As with
 automatic variables, these values are only available within the context
@@ -7039,7 +7048,7 @@ be substituted.
 * Text Functions::  General-purpose text manipulation functions.
 * File Name Functions:: Functions for manipulating file names.
 * Conditional Functions::   Functions that implement conditions.
-* Let Function::Lexically scoped variables.
+* Let Function::Local variables.
 * Foreach Function::Repeat some text with controlled variation.
 * File Function::  

[PATCH 1/2 V2] [SV 51286] Support for lexically scoped variables

[Jouke Witteveen]

* src/function.c: Introduce the 'let' built-in function
* tests/scripts/functions/let: Tests for the 'let' built-in function
---

This replaces:
> [PATCH 1/2] * src/function.c: Introduce the 'let' built-in function

and goes together with:
> [PATCH 2/2 V2] * doc/make.texi: Document the let function

On Fri, Oct 23, 2020 at 3:14 PM Paul Smith  wrote:
> Sorry for the delay.  I have been super-busy (you'd think working from
> home would give one _more_ time but it seems not).

No problem. I am not in a hurry, but would love to see this land in the
next release.

> The thing most critically missing is a set of regression tests we can
> use to ensure that the feature is working (including various corner
> cases you would like to ensure), so that we can be sure it _keeps_
> working.  Also I run these tests under valgrind and ASAN and similar to
> check for memory leaks and other errors.

The tests introduced here are mostly inspired by those of foreach.

Regards,
- Jouke

 src/function.c  | 53 +-
 tests/scripts/functions/let | 89 +
 2 files changed, 141 insertions(+), 1 deletion(-)
 create mode 100644 tests/scripts/functions/let

diff --git a/src/function.c b/src/function.c
index 0917e0c..c4ff030 100644
--- a/src/function.c
+++ b/src/function.c
@@ -908,6 +908,55 @@ func_foreach (char *o, char **argv, const char *funcname 
UNUSED)
   return o;
 }
 
+static char *
+func_let (char *o, char **argv, const char *funcname UNUSED)
+{
+  /* expand only the first two.  */
+  char *varnames = expand_argument (argv[0], NULL);
+  char *list = expand_argument (argv[1], NULL);
+  const char *body = argv[2];
+
+  const char *vp;
+  const char *vp_next = varnames;
+  const char *list_iterator = list;
+  char *p;
+  size_t len;
+  size_t vlen;
+
+  push_new_variable_scope ();
+
+  /* loop through LIST for all but the last VARNAME */
+  vp = find_next_token (&vp_next, &vlen);
+  NEXT_TOKEN (vp_next);
+  while (*vp_next != '\0')
+{
+  p = find_next_token (&list_iterator, &len);
+  if (*list_iterator != '\0')
+{
+  ++list_iterator;
+  p[len] = '\0';
+}
+  define_variable (vp, vlen, p ? p : "", o_automatic, 0);
+
+  vp = find_next_token (&vp_next, &vlen);
+  NEXT_TOKEN (vp_next);
+}
+  /* set the last VARNAME to the remainder of LIST */
+  if (vp)
+define_variable (vp, vlen, next_token (list_iterator), o_automatic, 0);
+
+  /* Expand the body in the context of the arguments, adding the result to
+ the variable buffer.  */
+
+  o = variable_expand_string (o, body, SIZE_MAX);
+
+  pop_variable_scope ();
+  free (varnames);
+  free (list);
+
+  return o + strlen (o);
+}
+
 struct a_word
 {
   struct a_word *next;
@@ -2337,7 +2386,8 @@ func_abspath (char *o, char **argv, const char *funcname 
UNUSED)
comma-separated values are treated as arguments.
 
EXPAND_ARGS means that all arguments should be expanded before invocation.
-   Functions that do namespace tricks (foreach) don't automatically expand.  */
+   Functions that do namespace tricks (foreach, let) don't automatically
+   expand.  */
 
 static char *func_call (char *o, char **argv, const char *funcname);
 
@@ -2373,6 +2423,7 @@ static struct function_table_entry function_table_init[] =
   FT_ENTRY ("words", 0,  1,  1,  func_words),
   FT_ENTRY ("origin",0,  1,  1,  func_origin),
   FT_ENTRY ("foreach",   3,  3,  0,  func_foreach),
+  FT_ENTRY ("let",   3,  3,  0,  func_let),
   FT_ENTRY ("call",  1,  0,  1,  func_call),
   FT_ENTRY ("info",  0,  1,  1,  func_error),
   FT_ENTRY ("error", 0,  1,  1,  func_error),
diff --git a/tests/scripts/functions/let b/tests/scripts/functions/let
new file mode 100644
index 000..d3dbe1a
--- /dev/null
+++ b/tests/scripts/functions/let
@@ -0,0 +1,89 @@
+#-*-perl-*-
+# $Id$
+
+$description = "Test the let function.";
+
+$details = "This is a test of the let function in gnu make.
+This function destructures a list of values and binds each
+value to a variable name in a list of variable names.
+Superfluous variable names are assigned the empty string and
+the remaining values are assigned to the last variable name.
+The binding holds for the duration of the evaluation of the
+given text and no longer.  The general form of the command
+is $(let \$vars,\$list,\$text).  Several types of let
+assignments are tested\n";
+
+
+# Allow empty variable names and empty value list.
+# We still expand the list and body.
+run_make_test('
+null =
+x = $(let $(null),$(info side-effect),abc)
+y = $(let y,,$ydef)
+
+all: ; @echo $x$y',
+  '', "side-effect\nabcdef\n");
+
+# The example macro from the manual.
+run_make_test('
+reverse = $(let first rest,$1,$(if $(rest),$(call reverse,$(rest)) )$(first))
+
+all: ; @echo $(call reverse, \
+ moe   miny  meeny eeny \
+  )

Re: [PATCH 1/2] * src/function.c: Introduce the 'let' built-in function

[Jouke Witteveen]

On Fri, Oct 9, 2020 at 4:36 PM Jouke Witteveen  wrote:
>
> ---
> This was sent before at the end of last year. Meanwhile, the copyright of my
> contributions is assigned to the FSF, so I picked this up again and added some
> documentation (next patch).
>
> The previous discussion was titled "[RFC] Scoped variables, supercharged".
>

Any news on this? There was some minor discussion on the second
version of the second half of this patch, but that was not really
related to this patch series.
These patches fix bug 51286.

Regards,
- Jouke

[PATCH 2/2 V2] * doc/make.texi: Document the let function

[Jouke Witteveen]

---
Thanks to Martin Dorey for spotting a typo in the first version of this
patch.  I fixed a few others typos too in this revision.  This revision is
sent just so that the previous version does not get applied accidentally.
There may still be parts that could be improved.

The first patch in this series of two remains unchanged.

Regards,
- Jouke

 doc/make.texi | 88 ---
 1 file changed, 76 insertions(+), 12 deletions(-)

diff --git a/doc/make.texi b/doc/make.texi
index 21573c0..4289f84 100644
--- a/doc/make.texi
+++ b/doc/make.texi
@@ -276,6 +276,7 @@ Functions for Transforming Text
 * Text Functions::  General-purpose text manipulation functions.
 * File Name Functions:: Functions for manipulating file names.
 * Conditional Functions::   Functions that implement conditions.
+* Let Function::Lexically scoped variables.
 * Foreach Function::Repeat some text with controlled variation.
 * File Function::   Write text to a file.
 * Call Function::   Expand a user-defined function.
@@ -7032,6 +7033,7 @@ be substituted.
 * Text Functions::  General-purpose text manipulation functions.
 * File Name Functions:: Functions for manipulating file names.
 * Conditional Functions::   Functions that implement conditions.
+* Let Function::Lexically scoped variables.
 * Foreach Function::Repeat some text with controlled variation.
 * File Function::   Write text to a file.
 * Call Function::   Expand a user-defined function.
@@ -7632,7 +7634,7 @@ the file names to refer to an existing file or directory. 
 Use the
 @code{wildcard} function to test for existence.
 @end table
 
-@node Conditional Functions, Foreach Function, File Name Functions, Functions
+@node Conditional Functions, Let Function, File Name Functions, Functions
 @section Functions for Conditionals
 @findex if
 @cindex conditional expansion
@@ -7685,14 +7687,70 @@ the result of the expansion is the expansion of the 
last argument.
 
 @end table
 
-@node Foreach Function, File Function, Conditional Functions, Functions
+@node Let Function, Foreach Function, Conditional Functions, Functions
+@section The @code{let} Function
+@findex let
+@cindex variables, lexically scoped
+
+The @code{let} function provides a means to limit the scope of a variable.
+The substitution of a value bound to a name in a @code{let} expression
+happens only in the text within the lexical scope defined by the @code{let}
+expression.
+
+Additionally, the @code{let} function enables list unpacking.  In that
+regard, it resembles the @code{read} command in the shell, @code{sh}.
+
+The syntax of the @code{let} function is:
+
+@example
+$(let @var{var} [@var{var} ...],@var{list},@var{text})
+@end example
+
+@noindent
+The first two arguments, the @var{var} list and @var{list}, are expanded
+before anything else is done; note that the last argument, @var{text}, is
+@strong{not} expanded at the same time.  Then, each word of the expanded
+value of @var{list} is bound to each of the variable names, @var{var}, in
+turn, with the final variable name being bound to the remainder of the
+expanded @var{list}.  In other words, the first word of @var{list} is
+bound to the first variable @var{var}, the second word to the second
+variable @var{var}, and so on.  If there are fewer words than there are
+@var{var} operands, the remaining @var{var}s are set to empty strings.  If
+there are fewer @var{var}s than words, the last @var{var} is set to what
+is left of the expanded @var{list} after the words bound to the earlier
+@var{var}s are removed.  After all variables are thus bound, @var{text} is
+expanded.
+
+This macro reverses the order of the words in the list that it is given as
+its first argument:
+
+@example
+reverse = $(let first rest,$1,$(if $(rest),$(call reverse,$(rest)) )$(first))
+@end example
+
+@noindent
+The @var{first} and @var{rest} variables are no longer available after the
+macro is expanded.  If variables by those names existed beforehand, they
+are not affected by the expansion of the @code{reverse} macro.
+
+The @code{let} function has no permanent effect on the variables
+@var{var}; their value and flavor after the @code{let} function call are
+the same as they were beforehand.  The values which are taken from
+@var{list} are in effect only temporarily, during the execution of
+@code{let}.  The variables @var{var} are simply-expanded variables
+during the execution of @code{let}.  If @var{var} was undefined
+before the @code{let} function call, it is undefined after the call.
+@xref{Flavors, ,The Two Flavors of Variables}.@refill
+
+@node Foreach Function, File Function, Let Function, Functions
 @section The @code{foreach} Function
 @findex foreach
 @cindex words, iterating over
 
-The @code{foreach} function is very different from other functions.  It
-causes one piece of text to

[PATCH 2/2] * doc/make.text: Document the let function

[Jouke Witteveen]

---
I think this covers the let function, but comments/suggestions are welcome in
case I missed something.

Regards,
- Jouke

 doc/make.texi | 87 ---
 1 file changed, 75 insertions(+), 12 deletions(-)

diff --git a/doc/make.texi b/doc/make.texi
index 21573c0..f20f4fc 100644
--- a/doc/make.texi
+++ b/doc/make.texi
@@ -276,6 +276,7 @@ Functions for Transforming Text
 * Text Functions::  General-purpose text manipulation functions.
 * File Name Functions:: Functions for manipulating file names.
 * Conditional Functions::   Functions that implement conditions.
+* Let Function::Lexically scoped variables.
 * Foreach Function::Repeat some text with controlled variation.
 * File Function::   Write text to a file.
 * Call Function::   Expand a user-defined function.
@@ -7032,6 +7033,7 @@ be substituted.
 * Text Functions::  General-purpose text manipulation functions.
 * File Name Functions:: Functions for manipulating file names.
 * Conditional Functions::   Functions that implement conditions.
+* Let Function::Lexically scoped variables.
 * Foreach Function::Repeat some text with controlled variation.
 * File Function::   Write text to a file.
 * Call Function::   Expand a user-defined function.
@@ -7632,7 +7634,7 @@ the file names to refer to an existing file or directory. 
 Use the
 @code{wildcard} function to test for existence.
 @end table
 
-@node Conditional Functions, Foreach Function, File Name Functions, Functions
+@node Conditional Functions, Let Function, File Name Functions, Functions
 @section Functions for Conditionals
 @findex if
 @cindex conditional expansion
@@ -7685,14 +7687,69 @@ the result of the expansion is the expansion of the 
last argument.
 
 @end table
 
-@node Foreach Function, File Function, Conditional Functions, Functions
+@node Let Function, Foreach Function, Conditional Functions, Functions
+@section The @code{let} Function
+@findex let
+@cindex variables, lexically scoped
+
+The @code{let} function provides a means to limit the scope of a variable.
+The substitution of a value bound to a name in a @code{let} expression
+happens only in the text within the lexical scope defined by the @code{let}
+expression.
+
+Additionally, the @code{let} function enables list unpackinging.  In that
+regard, it resembles the @code{read} command in the shell, @code{sh}.
+
+The syntax of the @code{let} function is:
+
+@example
+$(let @var{var} [@var{var} ...],@var{list},@var{text})
+@end example
+
+@noindent
+The first two arguments, the @var{var} list and @var{list}, are expanded
+before anything else is done; note that the last argument, @var{text}, is
+@strong{not} expanded at the same time.  Then, each word of the expanded
+value of @var{list} is bound to each of the variable names, @var{var}, in
+turn, with the final variable name being bound to the remainder of the
+expanded @var{list}.  In other words, the first word of @var{list} is
+bound to the first variable @var{var}, the second word to the second
+variable @var{var}, and so on.  If there are fewer words than there are
+@var{var} operands, the remaining @var{var}s are set to empty string.  If
+there are fewer @var{var}s than words, the last @var{var} is set to what
+is left of the expanded @var{list} after the words bound to the earlier
+@var{var}s are removed.  After all variables are thus bound, @var{text} is
+expanded.
+
+This macro reverses the order of the words in the list that it is given as
+its first argument:
+
+@example
+reverse = $(let first rest,$1,$(if $(rest),$(call reverse,$(rest)) )$(first))
+@end example
+
+The @var{first} and @var{rest} variables are no longer available after the
+macro is expanded.  If variables by those names existed beforehand, they
+are not affected by the expansion of the @code{reverse} macro.
+
+The @code{let} function has no permanent effect on the variables
+@var{var}; their value and flavor after the @code{let} function call are
+the same as they were beforehand.  The values which are taken from
+@var{list} are in effect only temporarily, during the execution of
+@code{let}.  The variables @var{var} are simply-expanded variables
+during the execution of @code{let}.  If @var{var} was undefined
+before the @code{foreach} function call, it is undefined after the call.
+@xref{Flavors, ,The Two Flavors of Variables}.@refill
+
+@node Foreach Function, File Function, Let Function, Functions
 @section The @code{foreach} Function
 @findex foreach
 @cindex words, iterating over
 
-The @code{foreach} function is very different from other functions.  It
-causes one piece of text to be used repeatedly, each time with a different
-substitution performed on it.  It resembles the @code{for} command in the
+The @code{foreach} function is similar to the @code{let} function, but very
+different from other function

[PATCH 1/2] * src/function.c: Introduce the 'let' built-in function

[Jouke Witteveen]

---
This was sent before at the end of last year. Meanwhile, the copyright of my
contributions is assigned to the FSF, so I picked this up again and added some
documentation (next patch).

The previous discussion was titled "[RFC] Scoped variables, supercharged".

 src/function.c | 51 +-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/src/function.c b/src/function.c
index 0917e0c..b0031a6 100644
--- a/src/function.c
+++ b/src/function.c
@@ -908,6 +908,53 @@ func_foreach (char *o, char **argv, const char *funcname 
UNUSED)
   return o;
 }
 
+static char *
+func_let (char *o, char **argv, const char *funcname UNUSED)
+{
+  /* expand only the first two.  */
+  char *varnames = expand_argument (argv[0], NULL);
+  char *list = expand_argument (argv[1], NULL);
+  const char *body = argv[2];
+
+  const char *list_iterator = list;
+  char *p;
+  size_t len;
+  size_t vlen;
+  const char *vp_next = varnames;
+  const char *vp = find_next_token (&vp_next, &vlen);
+
+  push_new_variable_scope ();
+
+  /* loop through LIST for all but the last VARNAME */
+  NEXT_TOKEN (vp_next);
+  while (*vp_next != '\0')
+{
+  p = find_next_token (&list_iterator, &len);
+  if (*list_iterator != '\0')
+{
+  ++list_iterator;
+  p[len] = '\0';
+}
+  define_variable (vp, vlen, p ? p : "", o_automatic, 0);
+
+  vp = find_next_token (&vp_next, &vlen);
+  NEXT_TOKEN (vp_next);
+}
+  if (vp)
+define_variable (vp, vlen, next_token (list_iterator), o_automatic, 0);
+
+  /* Expand the body in the context of the arguments, adding the result to
+ the variable buffer.  */
+
+  o = variable_expand_string (o, body, SIZE_MAX);
+
+  pop_variable_scope ();
+  free (varnames);
+  free (list);
+
+  return o + strlen (o);
+}
+
 struct a_word
 {
   struct a_word *next;
@@ -2337,7 +2384,8 @@ func_abspath (char *o, char **argv, const char *funcname 
UNUSED)
comma-separated values are treated as arguments.
 
EXPAND_ARGS means that all arguments should be expanded before invocation.
-   Functions that do namespace tricks (foreach) don't automatically expand.  */
+   Functions that do namespace tricks (foreach, let) don't automatically
+   expand.  */
 
 static char *func_call (char *o, char **argv, const char *funcname);
 
@@ -2373,6 +2421,7 @@ static struct function_table_entry function_table_init[] =
   FT_ENTRY ("words", 0,  1,  1,  func_words),
   FT_ENTRY ("origin",0,  1,  1,  func_origin),
   FT_ENTRY ("foreach",   3,  3,  0,  func_foreach),
+  FT_ENTRY ("let",   3,  3,  0,  func_let),
   FT_ENTRY ("call",  1,  0,  1,  func_call),
   FT_ENTRY ("info",  0,  1,  1,  func_error),
   FT_ENTRY ("error", 0,  1,  1,  func_error),
-- 
2.28.0

Re: [PATCH 2/2] compare function

[Jouke Witteveen]

On Tue, Jun 9, 2020 at 10:53 AM Edward Welbourne  wrote:
>
> Jouke Witteveen (8 June 2020 22:20)
> > It differs from his original proposal in that it behaves differently
> > when given 4 or 5 arguments. In short, it's signature is
> >
> >  $(compare lhs,rhs,less-than[,greater-or-equal])
> >  $(compare lhs,rhs,less-than,equal,greater-than)
> >
> > Documentation and a proper commit message is still missing, but this
> > proof-of-concept shows that it is in fact pretty simple to implement.
>
> Simplicity of implementation was not far from my thoughts when I
> proposed it; it is also easy to understand ;^>
>
> I note that your implementation only supports numeric lhs and rhs.
> Might it be worth falling back, if either of them isn't numeric, to
> doing a strcmp() comparison instead ?  Possibly after stripping leading
> space from each.  There is value in lexical comparison, too, after all.

I am not sure it is wise to mix integer and string contexts. Having a
$(compare) function that deals exclusively with integer context has
some appeal to me.

> I am not convinced it is good to have $5 default to $4, which is what
> you've done in effect.  IIRC, most of make's existing functions treat
> missing argument as empty; if my memory is correct, it would then be
> more consistent to have
>
>   $(compare $lhs,$rhs,$less[,$same[,$more]])
>
> with each of $same and $more empty if omitted.  (I make $less
> non-optional because it's perverse to do a comparison and return false
> regardless; but it could be optional, "for symmetry", if you like;
> albeit that might lead to a temptation to make it default to a non-empty
> string, so that $(compare $lhs,$rhs) serves as $lhs < $rhs as a
> conditional; but this would violate the very symmetry that motivated
> making $less optional.)

$(if) also has a minimum of two arguments, so let's not make $less optional.
Having $5 default to $4 was entirely deliberate. It means that the
semantics of the four-argument version mimics $(if) in the sense that
the expression evaluates to either the third or the fourth argument.
If you want the behavior you describe, you could simply add a ','
after the fourth argument, i.e. specify an empty fifth argument.
Because this is so short, I think it is far more elegant than wrapping
everything in an $(if).

> As I noted before, the cases where two branches want the same verbose
> value can readily enough be handled by using $(if ...) on a condition
> using $(compare ...) with two of its output values empty and the other
> some non-empty string (due to Lisp influence, I'd use t), or vice versa;
> or you could use $(let ...) to put the repeated value in a local
> variable that you use in both intended branches, when this more verbose
> form proves more readable (as it sometimes shall).

$(let) won't deal with side effects properly though. In effect, let
variables are simply expanded.

> > Additionally, I feel that the interface is clean. In that way, it
> > differs from the various proposals for integer operations. After
> > thinking about them some more, I came to dislike all current proposals
> > because of the unintuitive behavior of subtraction and division.
>
> I think this depends a lot on your intuitions, which can be trained.  I
> find $(math $op, $a $b ... $z) = $a $op $b $op ... $op $z to be
> entirely intuitive, but then I've been exposed to many ways of doing
> algebra and perpetrated some related to this.  (I would describe this as
> a "bulk action", for reference; see
>   http://www.chaos.org.uk/~eddy/maths/found/bulk.xhtml
> for a more formal treatment of the associative (i.e. + and * but not -
> and /) case.  It generalises sum and product.  I think some folk call
> these monads.  However, the fact that I write stuff like that may fairly
> be considered grounds for doubting my intuitions are shared by many
> others.)

Indeed, the proposals for sum and product make sense, but those for
subtraction and division cannot be made to work, since the operations
are not associative. It is not the syntax that bothers me, but the
semantics. As stated a few times now,

  $(math -,3)

could evaluate to '3' (in line with your expansion rule above) or to
'-3' (in line with intuition and because we would need a way to do
unary negation anyway). The design tries to achieve too many things at
once and will always fail to accomplish some of them.

> > We should only support integer mathematics, so division is always
> > going to be integer division, which suggests that we need a modulus
> > operator as well. Moreover, 1/$x will not be supported, so we can't
> > implement the same behavior for $(math -,$x) as for $(math

[PATCH 1/2] Use strtol instead of atoi

[Jouke Witteveen]

strtol is part of C89 and a fallback is provided by gnulib

* src/function.c (func_word, func_wordlist): Use strtol instead of atoi
* test/scripts/functions/word: Add out-of-range verification testing
---

Here is a proposal to start a more thorough support of numerical values.
While earlier discussion indicated that at least 64bit of range is
required, only long integers (guaranteed to have at least 32bit of range)
are part of C89. To keep things simple and compatible, I therefore chose
to use long. Note that on systems where large files (>4GB) may be
encountered, it is not unlikely for long to support 64bit of range.

In the code below, I have added a strtol line to bootstrap.conf. I am
completely unsure whether this is needed or even supported. My thought was
that gnulib provides a fallback strtol implementation for systems with
incomplete C89 support. Someone more knowledgable of gnulib should verify
this.

One change that these changes bring about is that a sign (+ or -) in front
of a numerical value is now supported.

 bootstrap.conf   |  1 +
 src/function.c   | 49 ++--
 tests/scripts/functions/word | 12 ++---
 3 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index 7c4c9ab..c9c3668 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -50,4 +50,5 @@ findprog-in
 getloadavg
 host-cpu-c-abi
 strerror
+strtol
 make-glob"
diff --git a/src/function.c b/src/function.c
index 0917e0c..ccad300 100644
--- a/src/function.c
+++ b/src/function.c
@@ -766,19 +766,24 @@ strip_whitespace (const char **begpp, const char **endpp)
   return (char *)*begpp;
 }
 
-static void
-check_numeric (const char *s, const char *msg)
+static long
+parse_numeric (const char *s, const char *msg)
 {
-  const char *end = s + strlen (s) - 1;
   const char *beg = s;
-  strip_whitespace (&s, &end);
-
-  for (; s <= end; ++s)
-if (!ISDIGIT (*s))  /* ISDIGIT only evals its arg once: see makeint.h.  */
-  break;
+  const char *end = s + strlen (s) - 1;
+  char *endp;
+  long num;
+  strip_whitespace (&beg, &end);
 
-  if (s <= end || end - beg < 0)
-OSS (fatal, *expanding_var, "%s: '%s'", msg, beg);
+  errno = 0;
+  num = strtol (beg, &endp, 10);
+  if (errno == ERANGE)
+OSS (fatal, *expanding_var, "%s: '%s'", strerror (errno), s);
+  else if (endp == beg || endp <= end)
+/* Empty or non-numeric input */
+OSS (fatal, *expanding_var, "%s: '%s'", msg, s);
+
+  return num;
 }
 
 
@@ -788,13 +793,11 @@ func_word (char *o, char **argv, const char *funcname 
UNUSED)
 {
   const char *end_p;
   const char *p;
-  int i;
+  long i;
 
-  /* Check the first argument.  */
-  check_numeric (argv[0], _("non-numeric first argument to 'word' function"));
-  i = atoi (argv[0]);
-
-  if (i == 0)
+  i = parse_numeric (argv[0],
+ _("non-numeric first argument to 'word' function"));
+  if (i <= 0)
 O (fatal, *expanding_var,
_("first argument to 'word' function must be greater than 0"));
 
@@ -812,20 +815,18 @@ func_word (char *o, char **argv, const char *funcname 
UNUSED)
 static char *
 func_wordlist (char *o, char **argv, const char *funcname UNUSED)
 {
-  int start, count;
+  long start, stop, count;
 
-  /* Check the arguments.  */
-  check_numeric (argv[0],
- _("non-numeric first argument to 'wordlist' function"));
-  check_numeric (argv[1],
- _("non-numeric second argument to 'wordlist' function"));
+  start = parse_numeric (argv[0],
+ _("non-numeric first argument to 'wordlist' 
function"));
+  stop = parse_numeric (argv[1],
+_("non-numeric second argument to 'wordlist' 
function"));
 
-  start = atoi (argv[0]);
   if (start < 1)
 ON (fatal, *expanding_var,
 "invalid first argument to 'wordlist' function: '%d'", start);
 
-  count = atoi (argv[1]) - start + 1;
+  count = stop - start + 1;
 
   if (count > 0)
 {
diff --git a/tests/scripts/functions/word b/tests/scripts/functions/word
index 4dcc940..044bc94 100644
--- a/tests/scripts/functions/word
+++ b/tests/scripts/functions/word
@@ -51,6 +51,7 @@ run_make_test('FOO = foo bar biz baz
 word-e1: ; @echo $(word ,$(FOO))
 word-e2: ; @echo $(word abc ,$(FOO))
 word-e3: ; @echo $(word 1a,$(FOO))
+word-e4: ; @echo $(word 999,$(FOO))
 
 wordlist-e1: ; @echo $(wordlist ,,$(FOO))
 wordlist-e2: ; @echo $(wordlist abc ,,$(FOO))
@@ -69,19 +70,24 @@ run_make_test(undef,
   "#MAKEFILE#:5: *** non-numeric first argument to 'word' 
function: '1a'.  Stop.",
   512);
 
+run_make_test(undef,
+  'word-e4',
+  "#MAKEFILE#:6: *** Numerical result out of range: 
'999'.  Stop.",
+  512);
+
 run_make_test(undef,
   'wordlist-e1',
-  "#MAKEFILE#:7: *** non-numeric first argument to 'wordlist' 
function: ''.  Stop.",
+  "#MAKEFILE#:8: *** non-numeric fir

[PATCH 2/2] compare function

[Jouke Witteveen]

This is an implementation of a $(compare) function as proposed by
Edward Welbourne. It differs from his original proposal in that it behaves
differently when given 4 or 5 arguments. In short, it's signature is

  $(compare lhs,rhs,less-than[,greater-or-equal])
  $(compare lhs,rhs,less-than,equal,greater-than)

Documentation and a proper commit message is still missing, but this
proof-of-concept shows that it is in fact pretty simple to implement.

Additionally, I feel that the interface is clean. In that way, it differs
from the various proposals for integer operations. After thinking about
them some more, I came to dislike all current proposals because of the
unintuitive behavior of subtraction and division. We should only support
integer mathematics, so division is always going to be integer division,
which suggests that we need a modulus operator as well. Moreover, 1/$x
will not be supported, so we can't implement the same behavior for
$(math -,$x) as for $(math /,$x). While verbose, the only clean way I can
think of is simply $(add $x,$y), $(subtract $x,$y), etc. All supporting
precisely two arguments. Multi argument versions can be defined like

  sum = $(let first rest,$1, \
  $(if $(rest),$(add $(first),$(call sum,$(rest))), \
   $(first)))

I'd like to repeat that on POSIX systems, mathematical expressions can be
evaluated using

  expr = $(shell expr '$1')

Indeed, not all environments are POSIX, but this does show that $(compare)
on its own (without any mathematical operators in make) can be very useful.
---
 src/function.c  | 47 +++
 tests/scripts/functions/compare | 57 +
 2 files changed, 104 insertions(+)
 create mode 100644 tests/scripts/functions/compare

diff --git a/src/function.c b/src/function.c
index ccad300..aa23349 100644
--- a/src/function.c
+++ b/src/function.c
@@ -1220,6 +1220,52 @@ func_sort (char *o, char **argv, const char *funcname 
UNUSED)
   return o;
 }
 
+/*
+  $(compare lhs,rhs,lt-part[,eq-part[,gt-part]])
+
+  LT-PART is evaluated when LHS is strictly less than RHS. Otherwise, if
+  GT-PART is not provided, EQ-PART is evaluated (if it exists). In case
+  both EQ-PART and GT-PART are provided, EQ-PART is evaluated when LHS equals
+  RHS, and GT-PART is evaluated when LHS is strictly greater than RHS.
+
+  Only one of the PARTs is evaluated, so $(compare ...) can be used to create
+  side-effects (with $(shell ...), for example).
+*/
+
+static char *
+func_compare (char *o, char **argv, const char *funcname UNUSED)
+{
+  char *lhs_str = expand_argument (argv[0], NULL);
+  char *rhs_str = expand_argument (argv[1], NULL);
+  long lhs, rhs;
+
+  lhs = parse_numeric (lhs_str,
+   _("non-numeric first argument to 'compare' function"));
+  rhs = parse_numeric (rhs_str,
+   _("non-numeric second argument to 'compare' function"));
+  free (lhs_str);
+  free (rhs_str);
+
+  argv += 2;
+  if (lhs >= rhs)
+{
+  ++argv;
+  if (lhs > rhs && *argv && *(argv + 1))
+++argv;
+}
+
+  if (*argv)
+{
+  char *expansion = expand_argument (*argv, NULL);
+
+  o = variable_buffer_output (o, expansion, strlen (expansion));
+
+  free (expansion);
+}
+
+  return o;
+}
+
 /*
   $(if condition,true-part[,false-part])
 
@@ -2378,6 +2424,7 @@ static struct function_table_entry function_table_init[] =
   FT_ENTRY ("info",  0,  1,  1,  func_error),
   FT_ENTRY ("error", 0,  1,  1,  func_error),
   FT_ENTRY ("warning",   0,  1,  1,  func_error),
+  FT_ENTRY ("compare",   3,  5,  0,  func_compare),
   FT_ENTRY ("if",2,  3,  0,  func_if),
   FT_ENTRY ("or",1,  0,  0,  func_or),
   FT_ENTRY ("and",   1,  0,  0,  func_and),
diff --git a/tests/scripts/functions/compare b/tests/scripts/functions/compare
new file mode 100644
index 000..574397f
--- /dev/null
+++ b/tests/scripts/functions/compare
@@ -0,0 +1,57 @@
+#-*-perl-*-
+$description = "Test the compare function.\n";
+
+$details = "Try various uses of compare and ensure they all give the correct
+results.\n";
+
+open(MAKEFILE, "> $makefile");
+
+print MAKEFILE <<'EOF';
+# Negative
+n = -10
+# Zero
+z = 0
+# Positive
+p = 10
+
+all:
+   @echo 1_1 $(compare $n,$n,$(shell echo lt))
+   @echo 1_2 $(compare $n,$z,$(shell echo lt))
+   @echo 1_3 $(compare $z,$n,$(shell echo lt))
+   @echo 2_1 $(compare $n,$p,lt,ge)
+   @echo 2_2 $(compare $z,$z,lt,ge)
+   @echo 2_3 $(compare $p,$n,lt,ge)
+   @echo 3_0 $(compare $p,$n,lt,eq,)
+   @echo 3_1 $(compare $z,$p,lt,eq,gt)
+   @echo 3_2 $(compare $p,$z,lt,eq,gt)
+   @echo 3_3 $(compare $p,$p,lt,eq,gt)
+EOF
+close(MAKEFILE);
+
+&run_make_with_options($makefile, "", &get_logfile);
+$answer = "1_1\n1_2 lt\n1_3\n2_1 lt\n2_2 ge\n2_3 ge\n3_0\n3_1 lt\n3_2 gt\n3_3 
eq\n";
+&compare_output

Re: math expressions (was: Re: Tail call elimination)

[Jouke Witteveen]

On Wed, May 27, 2020 at 8:47 PM Pete Dietl  wrote:
>
> A few questions.
>
> Technically, the C standard allows for machines which don't use 2's 
> complement.
> So should we consider our LONG_MIN to be -2^63 + 1?
>
> Also, signed arithmetic overflow is undefined behavior, so should we also
> indicate that we have undefined behavior or should we use some
> function that reliably detects and
> wraps signed arithmetic?
>
> Bit shifting to the right with signed integers is undefined behavior
> too. Usually this is an arithmetic shift,
> but it's not guaranteed. Should we try to guarantee this with some
> function or should we leave this as undefined behavior?

Here is a thought: The current support for numeric variables is
limited to unsigned numbers. We could choose to stick with that!
This would also deal with the earlier remark by Paul Smith regarding
the nasty nature of switching the sign of a numbers. In fairness,
switching a sign is really a syntactic operation:
minus = $(patsubst --%,%,$(1:%=-%))

> Before I make my proposal, what do you think of supporting the
> following operators:
> +, -, /, *, not, and, or, comp, <<, >>
>
> Should the logical operators have english names or should they be C
> symbols like !, &, |, ~ ?

I would definitely start with only addition, subtraction,
multiplication, and division:
$(math OP, LIST)
Note that when LIST is empty, addition and subtraction should yield
"0", while multiplication and division should yield "1". Every word in
list that is not a number must yield a "non-numeric argument" error,
number parsing should be shared with the current functions that accept
numeric arguments.

One more thing: I really like the proposal by Edward Welbourne for a
three-way comparison function. A numeric equivalent to $(if). His
$(conditional) proposal does not require any boolean tokens and feels
familiar with how things are working right now. One small issue is
that
$(conditional $(a),$(b),$(lt),$(eq))
should probably be empty when a is greater than b. A more ergonomic,
though probably too surprising alternative, is to use the second
argument also for the greater-than case if a third argument is
missing.

Let me know what your plans are, I am willing to help (but have limited time).
FSF copyright forms are in the process of being cleared at my work.

Regards,
- Jouke

Re: Tail call elimination

[Jouke Witteveen]

On Mon, May 18, 2020 at 8:50 PM Paul Smith  wrote:
>
> On Mon, 2020-05-11 at 16:32 -0500, Pete Dietl wrote:
> > I would like to know your thoughts about adding something like $(expr
> > ) to evaluate integer expressions and comparisons.
>
> I have no problem with some basic math facilities.  We already have
> functions like $(word ...), $(words ...), and $(wordlist ...), which
> sort of need math to be fully useful.

Each of these has an obvious 'output', which is not the case for
something like a comparison operator. This is also an objection
against $(eq) and $(not), which are hidden behind the EXPERIMENTAL
compilation flag.

> Adding in some simple comparison operators seems reasonable to me.
>
> > $(if $(expr $(major1) <= $(major2) \
>
> I assume the missing ")," at the end of this was an oversight?

Note that version comparison can quickly get rather non-trivial. For
that reason, I would advise against implementing it in make. Instead,
simply use a dedicated program via $(shell). On my Arch Linux system,
I have a vercmp program tailored to this exact use case. Note that the
return value of a call to $(shell) is available in $(.SHELLSTATUS).

Otherwise, POSIX prescribes an expr command, so with:
expr = $(shell expr '$1')
you can already do $(call expr,2 * 3 + 5).

Regards,
- Jouke

Re: Tail call elimination

[Jouke Witteveen]

On Mon, May 11, 2020 at 11:33 PM Pete Dietl  wrote:
>
> Indeed I understand these concerns.
> So the consensus seems to be that I may go ahead and attempt to implement 
> this.
>
> Other than the (let) and tail call optimization, I would like to know your 
> thoughts about
> adding something like $(expr ) to evaluate integer expressions and 
> comparisons.

Not that my opinion should carry any weight, but I do not like the
idea of adding an arithmetic context to make. It should not be
necessary for the use-cases of make. Bash has an arithmetic context,
and while it works (and is useful), it sort of sticks out like a sore
thumb design-wise [1].

I did not understand the concerns regarding Turing completeness.
Unless I am overlooking something, make has been Turing complete at
least as long as it has $(call).

Regards,
- Jouke

[1] It's weirder than you may think:
https://mywiki.wooledge.org/ArithmeticExpression

Re: Let construct

[Jouke Witteveen]

Hi Pete,

A month ago, I got in touch with Paul Smith about some paperwork
needed for assigning copyright to the FSF. I did not hear back from
him since and assume he is busy with other things at the moment. Since
Make is quite old, I figured there is no rush.

Probably the most important thing for the let proposal will be to have
good documentation. It should explain the behavior in a clear and
concise way, with some examples, but maybe also give some more
technical background: How it relates to lexical scoping (and how this
can be tricky in the case of Make because expressions may not be
evaluated at the moment a newcomer expects them to be), how it relates
to `let` in other languages, and probably also how it relates to the
Posix `read` shell command.

Since this is at least as tricky to get right as the actual code, I
chose to postpone it to after getting the green light for the feature.
In other words: I have not written anything yet, so if you feel
adventurous, you could draft something. It is often easier to
criticize an existing text than to make a new one.

Regards,
- Jouke

On Fri, May 1, 2020 at 3:43 PM Pete Dietl  wrote:
>
> Hey there.
> I just wanted to touch base about the status of the proposal of a (let) 
> construct. I think it’ll be a very useful feature and want to see it make 
> progress. :) I’m willing to help with it if need be.

[bug #51974] call on multiline (define/endef) behavior not well-documented

[Jouke Witteveen]

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

Either the 'new' documentation is not entirely correct, or there is a bug in
the current expansion logic (or I am misunderstanding something).

The documentation states:


4. Expand elements of the line which appear in an immediate expansion context
(see How make Reads a Makefile).
5. Scan the line for a separator character, such as ‘:’ or ‘=’, to
determine whether the line is a macro assignment or a rule (see Recipe
Syntax).
6. Internalize the resulting operation and read the next line.

An important consequence of this is that a macro can expand to an entire rule,
if it is one line long.


The last statement assumes that a line containing only a variable reference,
such as


$(myrule)


presents an immediate expansion context, although it does not meet any of the
stated forms of an immediate expansion context.

If it were, then this should also work:


myassignment = var = val

$(myassignment)


Step 5 of the list above would apply to the last line of this example (it
expands to 'var = val'). However, make fails with a "missing separator" error.
A slightly different error, "empty variable name", results from this
variation:


myassignment = var := val

$(myassignment)


___

Reply to this item at:

  

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

[bug #51286] Support for additional local make variables

[Jouke Witteveen]

Follow-up Comment #3, bug #51286 (project make):

> Dit I?

Wow, indeed, my memory is quite far off. Sorry!

I was thinking of an exchange regarding a request for renaming by thutt. Your
only expressed concern was that the behavior of let bindings might not be
clear to people not familiar with languages that have such bindings, i.e.

> In Lisp, "let" creates "local bindings" not a new scope.  I'm not sure if
that is more or less clear for people not familiar with Lisp.

My remark on "not fitting the design" was probably my mind trying to think of
good reasons to keep $(let) out of make. It may have been my own concern more
than yours ;-).

___

Reply to this item at:

  

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

[bug #51286] Support for additional local make variables

[Jouke Witteveen]

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

If the value you are trying to assign to a variable is guaranteed to contain
no spaces, you could use
  $(foreach var,,<...>)
but that is a bit of a hack of course.

More complete lexical scoping was proposed on the mailing list
(https://lists.gnu.org/archive/html/bug-make/2019-12/msg00017.html) in the
form of let expressions
  $(let ,,<...>)
but Paul Smith expressed his concern that let expressions may be to foreign to
the audience of make and that they do not fit the overall design of make
nicely.

I have attached the working prototype for implementing let expressions, which
lack documentation. Essentially, each word of  is assigned to each
variable name in , where the last variable name gets all remaining words
in . If there are more words in  than in , the corresponding
variables will be empty.

(file #48701)
___

Additional Item Attachment:

File name: let-function.patch Size:2 KB




___

Reply to this item at:

  

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

Re: [RFC] Scoped variables, supercharged

[Jouke Witteveen]

On Thu, Dec 26, 2019 at 2:03 PM Jouke Witteveen  wrote:
> I would like make to have scoped variables. Here, I will propose an
> implementation of them. This implementation is currently without tests and
> documentation. Hopefully, the proposal is acceptable and I can add the
> tests and documentation.

Was this proposal ever rejected? I think it may have gotten lost a bit
when dealing with the release of GNU Make 4.3.
Also, the reviews of my proposal seemed to have judged it based on
something it is not and does not try to be. All it does is a little
pattern matching and lexical scoping, basically reordering existing
$(foreach) implementation code.

As an extra example of how useful let expressions can be, consider the
following staple of functional programming:

# Right Fold
#  $1: Macro name
#  $2: List
Fold = $(let first rest,$2, \
$(if $(rest), \
$(call $1,$(first),$(call $0,$1,$(rest))), \
$(first)))

Regards,
- Jouke

> Consider a situation in which we have macros F and G, and some variable X,
> and our makefile includes:
>
>   $(call F,$(call G,$(X)),$(call G,$(X)))
>
> Here, we duplicate the call to G. To make that more transparent (and
> assuming G does not introduce side-effects), we could write the above as:
>
>   Y := $(call G,$(X))
>   $(call F,$(Y),$(Y))
>   undefine Y
>
> However, this would interfere with any existing variable Y.
> Alternatively, we could try:
>
>   $(foreach Y,$(call G,$(X)), \
> $(call F,$(Y),$(Y)))
>
> but that would not work if $(call G,$(X)) yields a list.
>
> A solution would be a new function, 'let', which allows us to write
>
>   $(let Y,$(call G,$(X)), \
> $(call F,$(Y),$(Y)))
>
> This function can be implemented easily. It can even be given superpowers.
> If the first argument to the new let-function is a single name, it is
> assigned the full second argument. If it is multiple names, say n, we can
> assign the first n-1 names to the first n-1 words of the second argument
> and the final name to the remainder of the arguments (adding empty words
> as necessary).
>
> This also solves http://savannah.gnu.org/bugs/?51286 and makes something
> like
>
>   reverse = $(let first rest,$1,$(if $(rest),$(call reverse,$(rest)) 
> )$(first))
>
> possible.
>
> I have included an implementation bbelow, borrowing from the
> implementation of foreach and call. Let me know if I can go forward with
> this idea and prepare a patch including tests and documentation.
>
> Regards,
> - Jouke
>
> ---
>  src/function.c | 51 +-
>  1 file changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/src/function.c b/src/function.c
> index 4ebff16..1c1c38b 100644
> --- a/src/function.c
> +++ b/src/function.c
> @@ -908,6 +908,53 @@ func_foreach (char *o, char **argv, const char *funcname 
> UNUSED)
>return o;
>  }
>
> +static char *
> +func_let (char *o, char **argv, const char *funcname UNUSED)
> +{
> +  /* expand only the first two.  */
> +  char *varnames = expand_argument (argv[0], NULL);
> +  char *list = expand_argument (argv[1], NULL);
> +  const char *body = argv[2];
> +
> +  const char *list_iterator = list;
> +  char *p;
> +  size_t len;
> +  size_t vlen;
> +  const char *vp_next = varnames;
> +  const char *vp = find_next_token (&vp_next, &vlen);
> +
> +  push_new_variable_scope ();
> +
> +  /* loop through LIST for all but the last VARNAME */
> +  NEXT_TOKEN (vp_next);
> +  while (*vp_next != '\0')
> +{
> +  p = find_next_token (&list_iterator, &len);
> +  if (*list_iterator != '\0')
> +{
> +  ++list_iterator;
> +  p[len] = '\0';
> +}
> +  define_variable (vp, vlen, p ? p : "", o_automatic, 0);
> +
> +  vp = find_next_token (&vp_next, &vlen);
> +  NEXT_TOKEN (vp_next);
> +}
> +  if (vp)
> +define_variable (vp, vlen, next_token (list_iterator), o_automatic, 0);
> +
> +  /* Expand the body in the context of the arguments, adding the result to
> + the variable buffer.  */
> +
> +  o = variable_expand_string (o, body, SIZE_MAX);
> +
> +  pop_variable_scope ();
> +  free (varnames);
> +  free (list);
> +
> +  return o + strlen (o);
> +}
> +
>  struct a_word
>  {
>struct a_word *next;
> @@ -2337,7 +2384,8 @@ func_abspath (char *o, char **argv, const char 
> *funcname UNUSED)
> comma-separated values are treated as arguments.
>
> EXPAND_ARGS means that all arguments should be expanded before invocation.
> -   Functions that do namespace tricks (foreach) don't automatically expand.  
> */
> + 

Re: [RFC] Scoped variables, supercharged

[Jouke Witteveen]

On Fri, Dec 27, 2019 at 4:29 PM  wrote:
>
> Jouke Witteveen writes:
>  > On Thu, Dec 26, 2019 at 10:52 PM Paul Smith  wrote:
>  > >
>
>  
>
>  > > I believe thutt is thinking of creating new scopes in makefiles 
> themselves,
>  > > not within a variable/function context, for example within included
>  > > makefiles or even within subsections of a particular makefile.
>
>  Yes.

Well, this request is not that ;-). Maybe it can help with it and make
something like it possible through some clever construct, but it was
designed purely for lexical scoping.

>  > Ah, now I understand. With $(let), the scope is determined by the
>  > parentheses, but theoretically, a file-scope could be a thing too.
>  >
>  > > For example something where you could say:
>  > >
>  > >FOO = bar
>  > >
>  > >push scope
>  > >local FOO = baz
>  > >
>  > >$(FOO): blah
>  > >pop scope
>  > >
>  > >$(FOO): yuck
>  > >
>  > > Of course if you wanted to write a bunch of define/endif stuff and use 
> eval
>  > > you could use $(let ...) to do something like this but it can get pretty
>  > > gross.
>  >
>  > Let's run a little test. I have a Makefile containing:
>  > 
>  > X=global
>  > $(let X,local,$(eval include inc.mk))
>  > $(info $X)
>  > 
>  > And a file callend inc.mk containing:
>  > 
>  > $(info $X)
>  > X=included
>  > $(info $X)
>  > 
>  > We can see what happens even without recompiling make by replacing the
>  > instance of $(let) by $(foreach). The resulting behavior is the same
>  > (because 'local' is only one word). We get the following:
>  > 
>  > $ make
>  > local
>  > local
>  > included
>  > 
>
>  Interesting.  I'd like to raise a few points.
>
>  o I would find it objectionable to have to $(eval) the include of
>files to gain scoping.  At the very least, this would adversely
>affect the readability of Makefiles.

These experiments were meant to test $(let), not to show any advisable
use of it. At least, that was my understanding of the suggestions by
Paul Smith. The points I was trying to make are that
1) the scope stack can act differently from what you expect, and
2) the obvious shortcomings of $(let) are also shortcomings of $(foreach).

>  o I don't think this meachanism correctly captures scoping.  The
>state of $(X) can be set by the master Makefile, or by an include
>file.  Furthermore, it can be changed by another included file.

$(let) provides lexical scoping in line with the use of let
expressions in other languages. The binding occurs at the time of
expansion. After expansion, the expression is fully expanded and the
scoped variables have completely disappeared.

>But, the actual use of $(X) in a recipe can be deferred to much
>later.  If there are several locations that set $(X) to a different
>value, what value will be used when the recipe is expanded?
>
>  
>
>  > One way to think of why $(let) would be a reasonable addition to make
>  > is precisely because it does not deviate from established behavior too
>  > much. Yet, it provides a clear value. Outside of the scoping, it also
>  > enables basic list unpacking similar to `read` in the shell. My
>  > example with reverse demonstrates this aspect too.
>
>  I agree that there is a clear value.  But I still caution that it
>  should not be confused with full semantic scoping in a whole build
>  system implemented in Make.

I am not sure what you mean by 'full semantic scoping', but I do feel
that you try to make of $(let) something that it is not. I am sure you
are aware that you can give variables target-specific values. This may
be of use when one build system is used for targets on multiple
platforms. Also, make is very liberal in its variable names, so with
proper hygiene and discipline, you can append some namespace-like
prefix to all the variables used in a file.

Regards,
- Jouke

Re: [RFC] Scoped variables, supercharged

[Jouke Witteveen]

On Thu, Dec 26, 2019 at 10:52 PM Paul Smith  wrote:
>
> On Thu, 2019-12-26 at 21:24 +0100, Jouke Witteveen wrote:
> > >   Your proposal has the potential to create variables that would have
> > >   scope local to a single invocation of a user-defined function, but it
> > >   wouldn't provide scoping to Make-proper.  For that reason alone, I
> > >   would suggest narrowing down the naming of the feature.  Perhaps
> > >   something like:
> >
> > What other meaning of 'scoping to Make-proper' do you have in mind? I
> > fail to see a scoping scenario that is not covered by my $(let)
> > proposal.
>
> I believe thutt is thinking of creating new scopes in makefiles themselves,
> not within a variable/function context, for example within included
> makefiles or even within subsections of a particular makefile.

Ah, now I understand. With $(let), the scope is determined by the
parentheses, but theoretically, a file-scope could be a thing too.

> For example something where you could say:
>
>FOO = bar
>
>push scope
>local FOO = baz
>
>$(FOO): blah
>pop scope
>
>$(FOO): yuck
>
> Of course if you wanted to write a bunch of define/endif stuff and use eval
> you could use $(let ...) to do something like this but it can get pretty
> gross.

Let's run a little test. I have a Makefile containing:

X=global
$(let X,local,$(eval include inc.mk))
$(info $X)

And a file callend inc.mk containing:

$(info $X)
X=included
$(info $X)

We can see what happens even without recompiling make by replacing the
instance of $(let) by $(foreach). The resulting behavior is the same
(because 'local' is only one word). We get the following:

$ make
local
local
included


To me, it would make more sense if the second line read 'included' and
the last line read 'global', so maybe there is a bug in how make
currently deals with variable scopes.

> That is a separate feature that wouldn't help you do what you want to do,
> but that's probably why thutt suggests finding a more specific term.  In
> Lisp, "let" creates "local bindings" not a new scope.  I'm not sure if that
> is more or less clear for people not familiar with Lisp.

I modeled $(let) after let expressions of Haskell. I am sure many
other (especially functional) languages have similar let constructs.
There is even a Wikipedia page on the 'Let expression', which mainly
deals with its use in mathematics, but is still very much applicable
to my $(let) proposal. For familiarity in the greater programming
language ecosystem, I think $(let) bindings with a scope defined by
the parentheses is a sensible choice of naming and behavior.

> Here's another question: will this "do what you expect" if you invoke an
> include within an eval within a let?  For example:
>
>   $(let FOO BAR,,$(eval include myfile.mk))
>
> will ensure that any "global" settings of FOO or BAR that are made inside
> of myfile.mk are forgotten about afterwards?
>
> What other sorts of tricks or issues could we get into using eval within
> let, I wonder...

Whatever breaks with the above expression probably also breaks with

  $(foreach FOO,_,$(foreach BAR,_,$(eval include myfile.mk)))

One way to think of why $(let) would be a reasonable addition to make
is precisely because it does not deviate from established behavior too
much. Yet, it provides a clear value. Outside of the scoping, it also
enables basic list unpacking similar to `read` in the shell. My
example with reverse demonstrates this aspect too.

Regards,
- Jouke

Re: [RFC] Scoped variables, supercharged

[Jouke Witteveen]

On Thu, Dec 26, 2019 at 6:13 PM  wrote:
> Jouke Witteveen writes:
>  > I would like make to have scoped variables. Here, I will propose an
>  > implementation of them. This implementation is currently without tests and
>  > documentation. Hopefully, the proposal is acceptable and I can add the
>  > tests and documentation.
>  >
>  > Consider a situation in which we have macros F and G, and some variable X,
>  > and our makefile includes:
>  >
>  >   $(call F,$(call G,$(X)),$(call G,$(X)))
>
>  Your proposal has the potential to create variables that would have
>  scope local to a single invocation of a user-defined function, but it
>  wouldn't provide scoping to Make-proper.  For that reason alone, I
>  would suggest narrowing down the naming of the feature.  Perhaps
>  something like:

What other meaning of 'scoping to Make-proper' do you have in mind? I
fail to see a scoping scenario that is not covered by my $(let)
proposal.

>Function local variables

Except that it has nothing to do with functions. Instead of the call
statements in my example, you can think of any long statement that
uses the same substatement (that has no side-effects) multiple times.

>  Have you considered how this might affect target- and
>  pattern-specific variables?
>
>  What would the affect be of a local variable overriding the name of a
>  global variable?

Global variables are shielded in the same way that $(call ...) shields
$0, $1, $2, ... in nested invocations.

>  Finally, have you taken a look at the so-called Gnu Make Standard
>  Library (GMSL)?  The implementation of dictionaries in that piece of
>  software reduces the need to introduce changes to Gnu Make to support
>  variable scoping -- but I accept that might be a controversial view.

I am aware of that library. I'd say it would benefit greatly from the
addition of my $(let) built-in to make.

[RFC] Scoped variables, supercharged

[Jouke Witteveen]

Hi,

I would like make to have scoped variables. Here, I will propose an
implementation of them. This implementation is currently without tests and
documentation. Hopefully, the proposal is acceptable and I can add the
tests and documentation.

Consider a situation in which we have macros F and G, and some variable X,
and our makefile includes:

  $(call F,$(call G,$(X)),$(call G,$(X)))

Here, we duplicate the call to G. To make that more transparent (and
assuming G does not introduce side-effects), we could write the above as:

  Y := $(call G,$(X))
  $(call F,$(Y),$(Y))
  undefine Y

However, this would interfere with any existing variable Y.
Alternatively, we could try:

  $(foreach Y,$(call G,$(X)), \
$(call F,$(Y),$(Y)))

but that would not work if $(call G,$(X)) yields a list.

A solution would be a new function, 'let', which allows us to write

  $(let Y,$(call G,$(X)), \
$(call F,$(Y),$(Y)))

This function can be implemented easily. It can even be given superpowers.
If the first argument to the new let-function is a single name, it is
assigned the full second argument. If it is multiple names, say n, we can
assign the first n-1 names to the first n-1 words of the second argument
and the final name to the remainder of the arguments (adding empty words
as necessary).

This also solves http://savannah.gnu.org/bugs/?51286 and makes something
like

  reverse = $(let first rest,$1,$(if $(rest),$(call reverse,$(rest)) )$(first))

possible.

I have included an implementation bbelow, borrowing from the
implementation of foreach and call. Let me know if I can go forward with
this idea and prepare a patch including tests and documentation.

Regards,
- Jouke

---
 src/function.c | 51 +-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/src/function.c b/src/function.c
index 4ebff16..1c1c38b 100644
--- a/src/function.c
+++ b/src/function.c
@@ -908,6 +908,53 @@ func_foreach (char *o, char **argv, const char *funcname 
UNUSED)
   return o;
 }
 
+static char *
+func_let (char *o, char **argv, const char *funcname UNUSED)
+{
+  /* expand only the first two.  */
+  char *varnames = expand_argument (argv[0], NULL);
+  char *list = expand_argument (argv[1], NULL);
+  const char *body = argv[2];
+
+  const char *list_iterator = list;
+  char *p;
+  size_t len;
+  size_t vlen;
+  const char *vp_next = varnames;
+  const char *vp = find_next_token (&vp_next, &vlen);
+
+  push_new_variable_scope ();
+
+  /* loop through LIST for all but the last VARNAME */
+  NEXT_TOKEN (vp_next);
+  while (*vp_next != '\0')
+{
+  p = find_next_token (&list_iterator, &len);
+  if (*list_iterator != '\0')
+{
+  ++list_iterator;
+  p[len] = '\0';
+}
+  define_variable (vp, vlen, p ? p : "", o_automatic, 0);
+
+  vp = find_next_token (&vp_next, &vlen);
+  NEXT_TOKEN (vp_next);
+}
+  if (vp)
+define_variable (vp, vlen, next_token (list_iterator), o_automatic, 0);
+
+  /* Expand the body in the context of the arguments, adding the result to
+ the variable buffer.  */
+
+  o = variable_expand_string (o, body, SIZE_MAX);
+
+  pop_variable_scope ();
+  free (varnames);
+  free (list);
+
+  return o + strlen (o);
+}
+
 struct a_word
 {
   struct a_word *next;
@@ -2337,7 +2384,8 @@ func_abspath (char *o, char **argv, const char *funcname 
UNUSED)
comma-separated values are treated as arguments.
 
EXPAND_ARGS means that all arguments should be expanded before invocation.
-   Functions that do namespace tricks (foreach) don't automatically expand.  */
+   Functions that do namespace tricks (foreach, let) don't automatically
+   expand.  */
 
 static char *func_call (char *o, char **argv, const char *funcname);
 
@@ -2373,6 +2421,7 @@ static struct function_table_entry function_table_init[] =
   FT_ENTRY ("words", 0,  1,  1,  func_words),
   FT_ENTRY ("origin",0,  1,  1,  func_origin),
   FT_ENTRY ("foreach",   3,  3,  0,  func_foreach),
+  FT_ENTRY ("let",   3,  3,  0,  func_let),
   FT_ENTRY ("call",  1,  0,  1,  func_call),
   FT_ENTRY ("info",  0,  1,  1,  func_error),
   FT_ENTRY ("error", 0,  1,  1,  func_error),
-- 
2.24.1

Re: [PATCH v2 2/3] [SV 54161] Fix second expansion of $*

[Jouke Witteveen]

On Wed, Dec 18, 2019 at 4:19 PM Paul Smith  wrote:
>
> On Wed, 2019-12-18 at 16:02 +0100, Jouke Witteveen wrote:
> > Your se_implicit test case got left out of your commit. I guess this
> > was unintentional.
>
> I'm not sure what you mean?  It's there...?

My mistake. I must have done something wrong, because now I see it.

> > Presumably, the test case was based on [SV 54161]. The bug can now be
> > closed.
>
> Yes, will do that later.
>

Re: [PATCH v2 2/3] [SV 54161] Fix second expansion of $*

[Jouke Witteveen]

On Wed, Dec 18, 2019 at 3:11 PM Paul Smith  wrote:
> I applied your changes although updates were needed.
>
> For the future, please note that (a) changes should contain updates to the
> regression tests to show the error and ensure the fix works, (b) commit
> messages need to be formatted properly so that they can be generated into
> ChangeLog entries, (c) we cannot use GNU libc-only functions like memrchr()
> since GNU make is highly portable and runs on Windows, VMS, etc., not to
> mention BSD, MacOS, and other POSIX systems where glibc is not available.
>
> I added a regression test based on the Savannah bug report, reformatted the
> commit messages, and added an implementation of memrchr() for systems where
> it doesn't exist.

Thanks for fixing my commit for me! I'll take these points into
account next time.
Many other libc implementations have memrchr, but honestly, this third
commit was just something I noticed when reading the code, and not
directly related to the bug I was trying to fix.

Your se_implicit test case got left out of your commit. I guess this
was unintentional.
Presumably, the test case was based on [SV 54161]. The bug can now be closed.

Regards,
- Jouke

Re: [PATCH v2 2/3] [SV 54161] Fix second expansion of $*

[Jouke Witteveen]

On Sat, Oct 26, 2019 at 12:24 PM Jouke Witteveen  wrote:
>
> Make sure the second expansion of $* in the prerequisites matches that of
> $* in the recipe.  This means that if we would add the dir of the stem to
> the prerequisites, we should replace % by $(*F) in the first expansion.
> Otherwise, we replace it to $*, but make sure that the value of $* used
> for expanding prerequisites matches that of $* in the recipe.
> ---

Is there anything I can do to further review and/or acceptance of this
patch I sent last month?

Regards,
- Jouke

Re: [PATCH v2 3/3] Oddities in dealing with the path of a stem

[Jouke Witteveen]

On 10/26/19, Jouke Witteveen  wrote:
> Make the code match the comments.
> ---

Note that memrchr is a GNU extension, available since glibc 2.1.91 (19
years old). If this particular patch is accepted, we might want to
drop the glibc version check in lib/glob.c.

This patch does fix a bug though. The old code did not implement what
the comment said it did: If a filename with multiple slashes ended in
a slash, the filename was considered not to contain any slashes at
all!

The last line makes sure that we only set pathlen if there is a path.
Previously, we would set the variable to a bogus value if there was no
path.

>  src/implicit.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/src/implicit.c b/src/implicit.c
> index e400dc6..f7b8c15 100644
> --- a/src/implicit.c
> +++ b/src/implicit.c
> @@ -266,7 +266,7 @@ pattern_search (struct file *file, int archive,
>/* Set LASTSLASH to point at the last slash in FILENAME
>   but not counting any slash at the end.  (foo/bar/ counts as
>   bar/ in directory foo/, not empty in directory foo/bar/.)  */
> -  lastslash = strrchr (filename, '/');
> +  lastslash = memrchr (filename, '/', namelen - 1);
>  #ifdef VMS
>if (lastslash == NULL)
>  lastslash = strrchr (filename, ']');
> @@ -279,18 +279,16 @@ pattern_search (struct file *file, int archive,
>/* Handle backslashes (possibly mixed with forward slashes)
>   and the case of "d:file".  */
>{
> -char *bslash = strrchr (filename, '\\');
> +char *bslash = memrchr (filename, '\\', namelen - 1);
>  if (lastslash == 0 || bslash > lastslash)
>lastslash = bslash;
>  if (lastslash == 0 && filename[0] && filename[1] == ':')
>lastslash = filename + 1;
>}
>  #endif
> -  if (lastslash != 0 && lastslash[1] == '\0')
> -lastslash = 0;
>  }
>
> -  pathlen = lastslash - filename + 1;
> +  pathlen = lastslash ? lastslash - filename + 1 : 0;
>
>/* First see which pattern rules match this target and may be
> considered.
>   Put them in TRYRULES.  */
> --
> 2.23.0
>
>

[PATCH v2 2/3] [SV 54161] Fix second expansion of $*

[Jouke Witteveen]

Make sure the second expansion of $* in the prerequisites matches that of
$* in the recipe.  This means that if we would add the dir of the stem to
the prerequisites, we should replace % by $(*F) in the first expansion.
Otherwise, we replace it to $*, but make sure that the value of $* used
for expanding prerequisites matches that of $* in the recipe.
---
 src/implicit.c | 56 +++---
 1 file changed, 39 insertions(+), 17 deletions(-)

diff --git a/src/implicit.c b/src/implicit.c
index 7bdc8ba..e400dc6 100644
--- a/src/implicit.c
+++ b/src/implicit.c
@@ -220,8 +220,9 @@ pattern_search (struct file *file, int archive,
   struct patdeps *deplist = xmalloc (max_deps * sizeof (struct patdeps));
   struct patdeps *pat = deplist;
 
-  /* Names of possible dependencies are constructed in this buffer.  */
-  char *depname = alloca (namelen + max_pattern_dep_length);
+  /* Names of possible dependencies are constructed in this buffer.
+ We may replace % by $(*F) for second expansion, increasing the length.  */
+  char *depname = alloca (namelen + max_pattern_dep_length + 4);
 
   /* The start and length of the stem of FILENAME for the current rule.  */
   const char *stem = 0;
@@ -479,9 +480,10 @@ pattern_search (struct file *file, int archive,
 }
 }
 
-  if (stemlen > GET_PATH_MAX)
+  if (stemlen + (check_lastslash ? pathlen : 0) > GET_PATH_MAX)
 {
-  DBS (DB_IMPLICIT, (_("Stem too long: '%.*s'.\n"),
+  DBS (DB_IMPLICIT, (_("Stem too long: '%s%.*s'.\n"),
+ check_lastslash ? pathdir : "",
  (int) stemlen, stem));
   continue;
 }
@@ -489,8 +491,19 @@ pattern_search (struct file *file, int archive,
   DBS (DB_IMPLICIT, (_("Trying pattern rule with stem '%.*s'.\n"),
  (int) stemlen, stem));
 
-  strncpy (stem_str, stem, stemlen);
-  stem_str[stemlen] = '\0';
+  if (!check_lastslash)
+{
+  memcpy (stem_str, stem, stemlen);
+  stem_str[stemlen] = '\0';
+}
+  else
+{
+  /* We want to prepend the directory from
+ the original FILENAME onto the stem.  */
+  memcpy (stem_str, filename, pathlen);
+  memcpy (stem_str + pathlen, stem, stemlen);
+  stem_str[pathlen + stemlen] = '\0';
+}
 
   /* If there are no prerequisites, then this rule matches.  */
   if (rule->deps == 0)
@@ -541,7 +554,7 @@ pattern_search (struct file *file, int archive,
 }
   memcpy (o, nptr, p - nptr);
   o += p - nptr;
-  memcpy (o, stem_str, stemlen);
+  memcpy (o, stem, stemlen);
   o += stemlen;
   strcpy (o, p + 1);
 }
@@ -592,10 +605,10 @@ pattern_search (struct file *file, int archive,
  again.  This is not good if you have certain characters
  in your stem (like $).
 
- Instead, we will replace % with $* and allow the second
- expansion to take care of it for us.  This way (since $*
- is a simple variable) there won't be additional
- re-expansion of the stem.  */
+ Instead, we will replace % with $* or $(*F) and allow the
+ second expansion to take care of it for us.  This way
+ (since $* and $(*F) are simple variables) there won't be
+ additional re-expansion of the stem.  */
 
   p = lindex (nptr, nptr + len, '%');
   if (p == 0)
@@ -606,13 +619,22 @@ pattern_search (struct file *file, int archive,
   else
 {
   size_t i = p - nptr;
-  memcpy (depname, nptr, i);
-  memcpy (depname + i, "$*", 2);
-  memcpy (depname + i + 2, p + 1, len - i - 1);
-  depname[len + 2 - 1] = '\0';
-
+  char *o = depname;
+  memcpy (o, nptr, i);
+  o += i;
   if (check_lastslash)
-add_dir = 1;
+{
+  add_dir = 1;
+  memcpy (o, "$(*F)", 5);
+  o += 5;
+}
+  else
+{
+  memcpy (o, "$*", 2);
+  o += 2;
+}
+  memcpy (o, p + 1, len - i - 1);
+  o[len - i - 1] = '\0';
 }
 
   /* Set up for the next word.  */
-- 
2.23.0

[PATCH v2 3/3] Oddities in dealing with the path of a stem

[Jouke Witteveen]

Make the code match the comments.
---
 src/implicit.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/implicit.c b/src/implicit.c
index e400dc6..f7b8c15 100644
--- a/src/implicit.c
+++ b/src/implicit.c
@@ -266,7 +266,7 @@ pattern_search (struct file *file, int archive,
   /* Set LASTSLASH to point at the last slash in FILENAME
  but not counting any slash at the end.  (foo/bar/ counts as
  bar/ in directory foo/, not empty in directory foo/bar/.)  */
-  lastslash = strrchr (filename, '/');
+  lastslash = memrchr (filename, '/', namelen - 1);
 #ifdef VMS
   if (lastslash == NULL)
 lastslash = strrchr (filename, ']');
@@ -279,18 +279,16 @@ pattern_search (struct file *file, int archive,
   /* Handle backslashes (possibly mixed with forward slashes)
  and the case of "d:file".  */
   {
-char *bslash = strrchr (filename, '\\');
+char *bslash = memrchr (filename, '\\', namelen - 1);
 if (lastslash == 0 || bslash > lastslash)
   lastslash = bslash;
 if (lastslash == 0 && filename[0] && filename[1] == ':')
   lastslash = filename + 1;
   }
 #endif
-  if (lastslash != 0 && lastslash[1] == '\0')
-lastslash = 0;
 }
 
-  pathlen = lastslash - filename + 1;
+  pathlen = lastslash ? lastslash - filename + 1 : 0;
 
   /* First see which pattern rules match this target and may be considered.
  Put them in TRYRULES.  */
-- 
2.23.0

[PATCH v2 1/3] Get rid of superfluous variables in stem tracking

[Jouke Witteveen]

---
 src/implicit.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/src/implicit.c b/src/implicit.c
index 707eff5..7bdc8ba 100644
--- a/src/implicit.c
+++ b/src/implicit.c
@@ -443,7 +443,6 @@ pattern_search (struct file *file, int archive,
   unsigned int deps_found = 0;
   /* NPTR points to the part of the prereq we haven't processed.  */
   const char *nptr = 0;
-  const char *dir = NULL;
   int order_only = 0;
   unsigned int matches;
 
@@ -478,7 +477,6 @@ pattern_search (struct file *file, int archive,
   memcpy (pathdir, filename, pathlen);
   pathdir[pathlen] = '\0';
 }
-  dir = pathdir;
 }
 
   if (stemlen > GET_PATH_MAX)
@@ -646,7 +644,7 @@ pattern_search (struct file *file, int archive,
   /* Parse the expanded string. */
   struct dep *dp = PARSE_FILE_SEQ (&p, struct dep,
order_only ? MAP_NUL : 
MAP_PIPE,
-   add_dir ? dir : NULL, 
PARSEFS_NONE);
+   add_dir ? pathdir : 
NULL, PARSEFS_NONE);
   *dptr = dp;
 
   for (d = dp; d != NULL; d = d->next)
@@ -931,17 +929,13 @@ pattern_search (struct file *file, int archive,
 }
   else
 {
-  size_t dirlen = (lastslash + 1) - filename;
-  char *sp;
-
   /* We want to prepend the directory from
  the original FILENAME onto the stem.  */
-  fullstemlen = dirlen + stemlen;
-  sp = alloca (fullstemlen + 1);
-  memcpy (sp, filename, dirlen);
-  memcpy (sp + dirlen, stem, stemlen);
-  sp[fullstemlen] = '\0';
-  file->stem = strcache_add (sp);
+  fullstemlen = pathlen + stemlen;
+  memcpy (stem_str, filename, pathlen);
+  memcpy (stem_str + pathlen, stem, stemlen);
+  stem_str[fullstemlen] = '\0';
+  file->stem = strcache_add (stem_str);
 }
 
   file->cmds = rule->cmds;
-- 
2.23.0

Re: [PATCH 2/2] [SV 54161] Fix second expansion of $*

[Jouke Witteveen]

On Sat, Oct 12, 2019 at 3:08 PM Jouke Witteveen  wrote:
>
> On Sat, Oct 12, 2019 at 2:50 PM Paul Smith  wrote:
> >
> > On Sat, 2019-10-12 at 13:11 +0200, Jouke Witteveen wrote:
> > > Before, this was only expanded to $(*F) in prerequisites.
> >
> > Sorry but I need more information than this; I can't understand this
> > change.
> >
> > The bug in Savannah, as I understand it, is that directory prefixes
> > which should be present are missing during prerequisite expansion.
> >
> > How does switching from $* to $(*F) (which is explicitly a file only)
> > help solve this problem?
> >
> > Cheers!
> >
>
> Haha, yeah, this seems counterintuitive, doesn't it!
> The true cause of the issue is not immediately visible in the patch:
> stem_str is used as a temporary value of file->stem and this value
> gets assigned to $* in the second expansion. To be consistent with the
> value of $* in the recipe, we should thus prepend the path to stem_str
> in some cases. This is what my patch does.
>
> However, as a sort of 'hack', the current code replaces % by $* on the
> first expansion, relying on the second expansion to do the right
> thing. This does not work after fixing $*, as directories have a
> special treatment in implicit rules (they are prepended to
> prerequisites separately). The reason this currently works, is because
> $* is set incorrectly. By changing it to $(*F), everything is fine
> again also with the fixed $* :-).

I realized this morning that my testing was too focused on one
particular scenario and that whether % is replaced by $* or by $(*F)
should depend on whether check_lastslash is set or not (i.e. whether
add_dir is 1 or 0). I'll send a revised patch soon.

- Jouke

___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make

Re: [PATCH 2/2] [SV 54161] Fix second expansion of $*

[Jouke Witteveen]

On Sat, Oct 12, 2019 at 2:50 PM Paul Smith  wrote:
>
> On Sat, 2019-10-12 at 13:11 +0200, Jouke Witteveen wrote:
> > Before, this was only expanded to $(*F) in prerequisites.
>
> Sorry but I need more information than this; I can't understand this
> change.
>
> The bug in Savannah, as I understand it, is that directory prefixes
> which should be present are missing during prerequisite expansion.
>
> How does switching from $* to $(*F) (which is explicitly a file only)
> help solve this problem?
>
> Cheers!
>

Haha, yeah, this seems counterintuitive, doesn't it!
The true cause of the issue is not immediately visible in the patch:
stem_str is used as a temporary value of file->stem and this value
gets assigned to $* in the second expansion. To be consistent with the
value of $* in the recipe, we should thus prepend the path to stem_str
in some cases. This is what my patch does.

However, as a sort of 'hack', the current code replaces % by $* on the
first expansion, relying on the second expansion to do the right
thing. This does not work after fixing $*, as directories have a
special treatment in implicit rules (they are prepended to
prerequisites separately). The reason this currently works, is because
$* is set incorrectly. By changing it to $(*F), everything is fine
again also with the fixed $* :-).

I hope this clarifies what the patch does.

Regards,
- Jouke

___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make

[PATCH 1/2] Get rid of superfluous variables in stem tracking

[Jouke Witteveen]

---
Some minor things I came across when working on a fix for [SV 54161],
which I'll send next.

Sure, the comments say that we should get rid of stem_str, but the
reality is that this variable is probably here to stay. Using it for
constructing the stem makes the kinship to another place where the same
thing happens (see next mail) clearer.

 src/implicit.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/src/implicit.c b/src/implicit.c
index 707eff5..7bdc8ba 100644
--- a/src/implicit.c
+++ b/src/implicit.c
@@ -443,7 +443,6 @@ pattern_search (struct file *file, int archive,
   unsigned int deps_found = 0;
   /* NPTR points to the part of the prereq we haven't processed.  */
   const char *nptr = 0;
-  const char *dir = NULL;
   int order_only = 0;
   unsigned int matches;
 
@@ -478,7 +477,6 @@ pattern_search (struct file *file, int archive,
   memcpy (pathdir, filename, pathlen);
   pathdir[pathlen] = '\0';
 }
-  dir = pathdir;
 }
 
   if (stemlen > GET_PATH_MAX)
@@ -646,7 +644,7 @@ pattern_search (struct file *file, int archive,
   /* Parse the expanded string. */
   struct dep *dp = PARSE_FILE_SEQ (&p, struct dep,
order_only ? MAP_NUL : 
MAP_PIPE,
-   add_dir ? dir : NULL, 
PARSEFS_NONE);
+   add_dir ? pathdir : 
NULL, PARSEFS_NONE);
   *dptr = dp;
 
   for (d = dp; d != NULL; d = d->next)
@@ -931,17 +929,13 @@ pattern_search (struct file *file, int archive,
 }
   else
 {
-  size_t dirlen = (lastslash + 1) - filename;
-  char *sp;
-
   /* We want to prepend the directory from
  the original FILENAME onto the stem.  */
-  fullstemlen = dirlen + stemlen;
-  sp = alloca (fullstemlen + 1);
-  memcpy (sp, filename, dirlen);
-  memcpy (sp + dirlen, stem, stemlen);
-  sp[fullstemlen] = '\0';
-  file->stem = strcache_add (sp);
+  fullstemlen = pathlen + stemlen;
+  memcpy (stem_str, filename, pathlen);
+  memcpy (stem_str + pathlen, stem, stemlen);
+  stem_str[fullstemlen] = '\0';
+  file->stem = strcache_add (stem_str);
 }
 
   file->cmds = rule->cmds;
-- 
2.23.0


___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make

[PATCH 2/2] [SV 54161] Fix second expansion of $*

[Jouke Witteveen]

Before, this was only expanded to $(*F) in prerequisites.
---
 src/implicit.c | 49 +++--
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/src/implicit.c b/src/implicit.c
index 7bdc8ba..a887e92 100644
--- a/src/implicit.c
+++ b/src/implicit.c
@@ -220,8 +220,9 @@ pattern_search (struct file *file, int archive,
   struct patdeps *deplist = xmalloc (max_deps * sizeof (struct patdeps));
   struct patdeps *pat = deplist;
 
-  /* Names of possible dependencies are constructed in this buffer.  */
-  char *depname = alloca (namelen + max_pattern_dep_length);
+  /* Names of possible dependencies are constructed in this buffer.
+ We replace a % by $(*F) for second expansion, increasing the length.  */
+  char *depname = alloca (namelen + max_pattern_dep_length + 4);
 
   /* The start and length of the stem of FILENAME for the current rule.  */
   const char *stem = 0;
@@ -479,9 +480,10 @@ pattern_search (struct file *file, int archive,
 }
 }
 
-  if (stemlen > GET_PATH_MAX)
+  if (stemlen + (check_lastslash ? pathlen : 0) > GET_PATH_MAX)
 {
-  DBS (DB_IMPLICIT, (_("Stem too long: '%.*s'.\n"),
+  DBS (DB_IMPLICIT, (_("Stem too long: '%s%.*s'.\n"),
+ check_lastslash ? pathdir : "",
  (int) stemlen, stem));
   continue;
 }
@@ -489,8 +491,19 @@ pattern_search (struct file *file, int archive,
   DBS (DB_IMPLICIT, (_("Trying pattern rule with stem '%.*s'.\n"),
  (int) stemlen, stem));
 
-  strncpy (stem_str, stem, stemlen);
-  stem_str[stemlen] = '\0';
+  if (!check_lastslash)
+{
+  memcpy (stem_str, stem, stemlen);
+  stem_str[stemlen] = '\0';
+}
+  else
+{
+  /* We want to prepend the directory from
+ the original FILENAME onto the stem.  */
+  memcpy (stem_str, filename, pathlen);
+  memcpy (stem_str + pathlen, stem, stemlen);
+  stem_str[pathlen + stemlen] = '\0';
+}
 
   /* If there are no prerequisites, then this rule matches.  */
   if (rule->deps == 0)
@@ -541,7 +554,7 @@ pattern_search (struct file *file, int archive,
 }
   memcpy (o, nptr, p - nptr);
   o += p - nptr;
-  memcpy (o, stem_str, stemlen);
+  memcpy (o, stem, stemlen);
   o += stemlen;
   strcpy (o, p + 1);
 }
@@ -586,15 +599,15 @@ pattern_search (struct file *file, int archive,
   continue;
 }
 
-  /* If the dependency name has %, substitute the stem.  If we
- just replace % with the stem value then later, when we do
- the 2nd expansion, we will re-expand this stem value
- again.  This is not good if you have certain characters
- in your stem (like $).
+  /* If the dependency name has %, substitute the filename of
+ the stem.  If we just replace % with the stem value then
+ later, when we do the 2nd expansion, we will re-expand
+ this stem value again.  This is not good if you have
+ certain characters in your stem (like $).
 
- Instead, we will replace % with $* and allow the second
- expansion to take care of it for us.  This way (since $*
- is a simple variable) there won't be additional
+ Instead, we will replace % with $(*F) and allow the second
+ expansion to take care of it for us.  This way (since
+ $(*F) is a simple variable) there won't be additional
  re-expansion of the stem.  */
 
   p = lindex (nptr, nptr + len, '%');
@@ -607,9 +620,9 @@ pattern_search (struct file *file, int archive,
 {
   size_t i = p - nptr;
   memcpy (depname, nptr, i);
-  memcpy (depname + i, "$*", 2);
-  memcpy (depname + i + 2, p + 1, len - i - 1);
-  depname[len + 2 - 1] = '\0';
+  memcpy (depname + i, "$(*F)", 5);
+  memcpy (depname + i + 5, p + 1, len - i - 1);
+  depname[len + 5 - 1] = '\0';
 
   if (check_lastslash)
 add_dir = 1;
-- 
2.23.0


___
Bug-make mailing list
Bug-make@gnu.org
https://lists.gnu.org/mailman/listinfo/bug-make