Re: [PATCH v4 07/31] kconfig: add built-in function support

[Ulf Magnusson]

On Tue, May 22, 2018 at 6:50 AM, Ulf Magnusson  wrote:
> On Tue, May 22, 2018 at 5:11 AM, Masahiro Yamada
>  wrote:
>> 2018-05-22 0:10 GMT+09:00 Ulf Magnusson :
>>> On Mon, May 21, 2018 at 4:32 PM, Ulf Magnusson  wrote:
 On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson  wrote:
> On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg  wrote:
>> Will the following be equal:
>>
>> $(foo,abc,$(x),$(y))
>> $(foo, abc, $(x), $(y))
>>
>> make is rather annoying as space is significant, but there seems no good 
>> reason
>> for kconfig to inheritate this.
>> So unless there are good arguments consider alloing the spaces.
>> If the current implmentation already supports optional spaces then I 
>> just missed
>> it whie reviewing.
>>
>> Sam
>
> +1 from me.
>
> I also find the rules for whitespace in Make confusing, and always
> have to look them up when doing trickier stuff. Maybe they're the
> result of people not considering whitespace initially, and stuff
> getting tacked on later. GNU Make adds some alternate syntaxes with
> quotes.
>
> I was going to mention shell, but it looks like you already did. :)
>
> If we go with Make-like syntax, maybe we could at least have a variant
> with fewer whitespace gotchas.
>
> Cheers,
> Ulf

 Maybe it'd be a pain to implement, but something like $(foo $(x) "two
 words" "interpolated $(stuff)") seems pretty nice, with three
 arguments there.
>>>
>>> Guess that might interact poorly with $(shell foo "bar baz") though.
>>> Kinda nice to have a syntax that doesn't overlap with shell when
>>> building shell commands.
>>
>>
>> Right.  I can easily imagine
>> that would end up with more gotchas due to quoting and escaping.
>
> Yeah, you're right. It's probably trying to fix something that isn't
> broken. Make's syntax really isn't bad there, just slightly
> non-obvious at first...
>
> Think it's fine now. Better to commit to the syntax than trying to be
> "helpful" by adding a bunch of random exceptions too. That probably
> gives a bigger mess in the end...
>
> Cheers,
> Ulf

I'm fine with the comma-after-function-name change though. That just
makes it more consistent.

Cheers,
Ulf

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Ulf Magnusson]

On Tue, May 22, 2018 at 6:50 AM, Ulf Magnusson  wrote:
> On Tue, May 22, 2018 at 5:11 AM, Masahiro Yamada
>  wrote:
>> 2018-05-22 0:10 GMT+09:00 Ulf Magnusson :
>>> On Mon, May 21, 2018 at 4:32 PM, Ulf Magnusson  wrote:
 On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson  wrote:
> On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg  wrote:
>> Will the following be equal:
>>
>> $(foo,abc,$(x),$(y))
>> $(foo, abc, $(x), $(y))
>>
>> make is rather annoying as space is significant, but there seems no good 
>> reason
>> for kconfig to inheritate this.
>> So unless there are good arguments consider alloing the spaces.
>> If the current implmentation already supports optional spaces then I 
>> just missed
>> it whie reviewing.
>>
>> Sam
>
> +1 from me.
>
> I also find the rules for whitespace in Make confusing, and always
> have to look them up when doing trickier stuff. Maybe they're the
> result of people not considering whitespace initially, and stuff
> getting tacked on later. GNU Make adds some alternate syntaxes with
> quotes.
>
> I was going to mention shell, but it looks like you already did. :)
>
> If we go with Make-like syntax, maybe we could at least have a variant
> with fewer whitespace gotchas.
>
> Cheers,
> Ulf

 Maybe it'd be a pain to implement, but something like $(foo $(x) "two
 words" "interpolated $(stuff)") seems pretty nice, with three
 arguments there.
>>>
>>> Guess that might interact poorly with $(shell foo "bar baz") though.
>>> Kinda nice to have a syntax that doesn't overlap with shell when
>>> building shell commands.
>>
>>
>> Right.  I can easily imagine
>> that would end up with more gotchas due to quoting and escaping.
>
> Yeah, you're right. It's probably trying to fix something that isn't
> broken. Make's syntax really isn't bad there, just slightly
> non-obvious at first...
>
> Think it's fine now. Better to commit to the syntax than trying to be
> "helpful" by adding a bunch of random exceptions too. That probably
> gives a bigger mess in the end...
>
> Cheers,
> Ulf

I'm fine with the comma-after-function-name change though. That just
makes it more consistent.

Cheers,
Ulf

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Ulf Magnusson]

On Tue, May 22, 2018 at 5:11 AM, Masahiro Yamada
 wrote:
> 2018-05-22 0:10 GMT+09:00 Ulf Magnusson :
>> On Mon, May 21, 2018 at 4:32 PM, Ulf Magnusson  wrote:
>>> On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson  wrote:
 On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg  wrote:
> Will the following be equal:
>
> $(foo,abc,$(x),$(y))
> $(foo, abc, $(x), $(y))
>
> make is rather annoying as space is significant, but there seems no good 
> reason
> for kconfig to inheritate this.
> So unless there are good arguments consider alloing the spaces.
> If the current implmentation already supports optional spaces then I just 
> missed
> it whie reviewing.
>
> Sam

 +1 from me.

 I also find the rules for whitespace in Make confusing, and always
 have to look them up when doing trickier stuff. Maybe they're the
 result of people not considering whitespace initially, and stuff
 getting tacked on later. GNU Make adds some alternate syntaxes with
 quotes.

 I was going to mention shell, but it looks like you already did. :)

 If we go with Make-like syntax, maybe we could at least have a variant
 with fewer whitespace gotchas.

 Cheers,
 Ulf
>>>
>>> Maybe it'd be a pain to implement, but something like $(foo $(x) "two
>>> words" "interpolated $(stuff)") seems pretty nice, with three
>>> arguments there.
>>
>> Guess that might interact poorly with $(shell foo "bar baz") though.
>> Kinda nice to have a syntax that doesn't overlap with shell when
>> building shell commands.
>
>
> Right.  I can easily imagine
> that would end up with more gotchas due to quoting and escaping.

Yeah, you're right. It's probably trying to fix something that isn't
broken. Make's syntax really isn't bad there, just slightly
non-obvious at first...

Think it's fine now. Better to commit to the syntax than trying to be
"helpful" by adding a bunch of random exceptions too. That probably
gives a bigger mess in the end...

Cheers,
Ulf

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Ulf Magnusson]

On Tue, May 22, 2018 at 5:11 AM, Masahiro Yamada
 wrote:
> 2018-05-22 0:10 GMT+09:00 Ulf Magnusson :
>> On Mon, May 21, 2018 at 4:32 PM, Ulf Magnusson  wrote:
>>> On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson  wrote:
 On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg  wrote:
> Will the following be equal:
>
> $(foo,abc,$(x),$(y))
> $(foo, abc, $(x), $(y))
>
> make is rather annoying as space is significant, but there seems no good 
> reason
> for kconfig to inheritate this.
> So unless there are good arguments consider alloing the spaces.
> If the current implmentation already supports optional spaces then I just 
> missed
> it whie reviewing.
>
> Sam

 +1 from me.

 I also find the rules for whitespace in Make confusing, and always
 have to look them up when doing trickier stuff. Maybe they're the
 result of people not considering whitespace initially, and stuff
 getting tacked on later. GNU Make adds some alternate syntaxes with
 quotes.

 I was going to mention shell, but it looks like you already did. :)

 If we go with Make-like syntax, maybe we could at least have a variant
 with fewer whitespace gotchas.

 Cheers,
 Ulf
>>>
>>> Maybe it'd be a pain to implement, but something like $(foo $(x) "two
>>> words" "interpolated $(stuff)") seems pretty nice, with three
>>> arguments there.
>>
>> Guess that might interact poorly with $(shell foo "bar baz") though.
>> Kinda nice to have a syntax that doesn't overlap with shell when
>> building shell commands.
>
>
> Right.  I can easily imagine
> that would end up with more gotchas due to quoting and escaping.

Yeah, you're right. It's probably trying to fix something that isn't
broken. Make's syntax really isn't bad there, just slightly
non-obvious at first...

Think it's fine now. Better to commit to the syntax than trying to be
"helpful" by adding a bunch of random exceptions too. That probably
gives a bigger mess in the end...

Cheers,
Ulf

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Masahiro Yamada]

2018-05-22 0:10 GMT+09:00 Ulf Magnusson :
> On Mon, May 21, 2018 at 4:32 PM, Ulf Magnusson  wrote:
>> On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson  wrote:
>>> On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg  wrote:
 Will the following be equal:

 $(foo,abc,$(x),$(y))
 $(foo, abc, $(x), $(y))

 make is rather annoying as space is significant, but there seems no good 
 reason
 for kconfig to inheritate this.
 So unless there are good arguments consider alloing the spaces.
 If the current implmentation already supports optional spaces then I just 
 missed
 it whie reviewing.

 Sam
>>>
>>> +1 from me.
>>>
>>> I also find the rules for whitespace in Make confusing, and always
>>> have to look them up when doing trickier stuff. Maybe they're the
>>> result of people not considering whitespace initially, and stuff
>>> getting tacked on later. GNU Make adds some alternate syntaxes with
>>> quotes.
>>>
>>> I was going to mention shell, but it looks like you already did. :)
>>>
>>> If we go with Make-like syntax, maybe we could at least have a variant
>>> with fewer whitespace gotchas.
>>>
>>> Cheers,
>>> Ulf
>>
>> Maybe it'd be a pain to implement, but something like $(foo $(x) "two
>> words" "interpolated $(stuff)") seems pretty nice, with three
>> arguments there.
>
> Guess that might interact poorly with $(shell foo "bar baz") though.
> Kinda nice to have a syntax that doesn't overlap with shell when
> building shell commands.


Right.  I can easily imagine
that would end up with more gotchas due to quoting and escaping.



> Still wondering if you could get rid of some of the Make gotchas
> without losing other stuff...
>
> Cheers,
> Ulf



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Masahiro Yamada]

2018-05-22 0:10 GMT+09:00 Ulf Magnusson :
> On Mon, May 21, 2018 at 4:32 PM, Ulf Magnusson  wrote:
>> On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson  wrote:
>>> On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg  wrote:
 Will the following be equal:

 $(foo,abc,$(x),$(y))
 $(foo, abc, $(x), $(y))

 make is rather annoying as space is significant, but there seems no good 
 reason
 for kconfig to inheritate this.
 So unless there are good arguments consider alloing the spaces.
 If the current implmentation already supports optional spaces then I just 
 missed
 it whie reviewing.

 Sam
>>>
>>> +1 from me.
>>>
>>> I also find the rules for whitespace in Make confusing, and always
>>> have to look them up when doing trickier stuff. Maybe they're the
>>> result of people not considering whitespace initially, and stuff
>>> getting tacked on later. GNU Make adds some alternate syntaxes with
>>> quotes.
>>>
>>> I was going to mention shell, but it looks like you already did. :)
>>>
>>> If we go with Make-like syntax, maybe we could at least have a variant
>>> with fewer whitespace gotchas.
>>>
>>> Cheers,
>>> Ulf
>>
>> Maybe it'd be a pain to implement, but something like $(foo $(x) "two
>> words" "interpolated $(stuff)") seems pretty nice, with three
>> arguments there.
>
> Guess that might interact poorly with $(shell foo "bar baz") though.
> Kinda nice to have a syntax that doesn't overlap with shell when
> building shell commands.


Right.  I can easily imagine
that would end up with more gotchas due to quoting and escaping.



> Still wondering if you could get rid of some of the Make gotchas
> without losing other stuff...
>
> Cheers,
> Ulf



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Ulf Magnusson]

On Mon, May 21, 2018 at 4:32 PM, Ulf Magnusson  wrote:
> On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson  wrote:
>> On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg  wrote:
>>> Will the following be equal:
>>>
>>> $(foo,abc,$(x),$(y))
>>> $(foo, abc, $(x), $(y))
>>>
>>> make is rather annoying as space is significant, but there seems no good 
>>> reason
>>> for kconfig to inheritate this.
>>> So unless there are good arguments consider alloing the spaces.
>>> If the current implmentation already supports optional spaces then I just 
>>> missed
>>> it whie reviewing.
>>>
>>> Sam
>>
>> +1 from me.
>>
>> I also find the rules for whitespace in Make confusing, and always
>> have to look them up when doing trickier stuff. Maybe they're the
>> result of people not considering whitespace initially, and stuff
>> getting tacked on later. GNU Make adds some alternate syntaxes with
>> quotes.
>>
>> I was going to mention shell, but it looks like you already did. :)
>>
>> If we go with Make-like syntax, maybe we could at least have a variant
>> with fewer whitespace gotchas.
>>
>> Cheers,
>> Ulf
>
> Maybe it'd be a pain to implement, but something like $(foo $(x) "two
> words" "interpolated $(stuff)") seems pretty nice, with three
> arguments there.

Guess that might interact poorly with $(shell foo "bar baz") though.
Kinda nice to have a syntax that doesn't overlap with shell when
building shell commands.

Still wondering if you could get rid of some of the Make gotchas
without losing other stuff...

Cheers,
Ulf

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Ulf Magnusson]

On Mon, May 21, 2018 at 4:32 PM, Ulf Magnusson  wrote:
> On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson  wrote:
>> On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg  wrote:
>>> Will the following be equal:
>>>
>>> $(foo,abc,$(x),$(y))
>>> $(foo, abc, $(x), $(y))
>>>
>>> make is rather annoying as space is significant, but there seems no good 
>>> reason
>>> for kconfig to inheritate this.
>>> So unless there are good arguments consider alloing the spaces.
>>> If the current implmentation already supports optional spaces then I just 
>>> missed
>>> it whie reviewing.
>>>
>>> Sam
>>
>> +1 from me.
>>
>> I also find the rules for whitespace in Make confusing, and always
>> have to look them up when doing trickier stuff. Maybe they're the
>> result of people not considering whitespace initially, and stuff
>> getting tacked on later. GNU Make adds some alternate syntaxes with
>> quotes.
>>
>> I was going to mention shell, but it looks like you already did. :)
>>
>> If we go with Make-like syntax, maybe we could at least have a variant
>> with fewer whitespace gotchas.
>>
>> Cheers,
>> Ulf
>
> Maybe it'd be a pain to implement, but something like $(foo $(x) "two
> words" "interpolated $(stuff)") seems pretty nice, with three
> arguments there.

Guess that might interact poorly with $(shell foo "bar baz") though.
Kinda nice to have a syntax that doesn't overlap with shell when
building shell commands.

Still wondering if you could get rid of some of the Make gotchas
without losing other stuff...

Cheers,
Ulf

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Ulf Magnusson]

On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson  wrote:
> On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg  wrote:
>> Will the following be equal:
>>
>> $(foo,abc,$(x),$(y))
>> $(foo, abc, $(x), $(y))
>>
>> make is rather annoying as space is significant, but there seems no good 
>> reason
>> for kconfig to inheritate this.
>> So unless there are good arguments consider alloing the spaces.
>> If the current implmentation already supports optional spaces then I just 
>> missed
>> it whie reviewing.
>>
>> Sam
>
> +1 from me.
>
> I also find the rules for whitespace in Make confusing, and always
> have to look them up when doing trickier stuff. Maybe they're the
> result of people not considering whitespace initially, and stuff
> getting tacked on later. GNU Make adds some alternate syntaxes with
> quotes.
>
> I was going to mention shell, but it looks like you already did. :)
>
> If we go with Make-like syntax, maybe we could at least have a variant
> with fewer whitespace gotchas.
>
> Cheers,
> Ulf

Maybe it'd be a pain to implement, but something like $(foo $(x) "two
words" "interpolated $(stuff)") seems pretty nice, with three
arguments there.

For variables too:

  x = foo
  y = "two words"

Or have mandatory quotes, but yeah, bit spammy there maybe.

Cheers,
Ulf

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Ulf Magnusson]

On Mon, May 21, 2018 at 4:23 PM, Ulf Magnusson  wrote:
> On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg  wrote:
>> Will the following be equal:
>>
>> $(foo,abc,$(x),$(y))
>> $(foo, abc, $(x), $(y))
>>
>> make is rather annoying as space is significant, but there seems no good 
>> reason
>> for kconfig to inheritate this.
>> So unless there are good arguments consider alloing the spaces.
>> If the current implmentation already supports optional spaces then I just 
>> missed
>> it whie reviewing.
>>
>> Sam
>
> +1 from me.
>
> I also find the rules for whitespace in Make confusing, and always
> have to look them up when doing trickier stuff. Maybe they're the
> result of people not considering whitespace initially, and stuff
> getting tacked on later. GNU Make adds some alternate syntaxes with
> quotes.
>
> I was going to mention shell, but it looks like you already did. :)
>
> If we go with Make-like syntax, maybe we could at least have a variant
> with fewer whitespace gotchas.
>
> Cheers,
> Ulf

Maybe it'd be a pain to implement, but something like $(foo $(x) "two
words" "interpolated $(stuff)") seems pretty nice, with three
arguments there.

For variables too:

  x = foo
  y = "two words"

Or have mandatory quotes, but yeah, bit spammy there maybe.

Cheers,
Ulf

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Ulf Magnusson]

On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg  wrote:
> Will the following be equal:
>
> $(foo,abc,$(x),$(y))
> $(foo, abc, $(x), $(y))
>
> make is rather annoying as space is significant, but there seems no good 
> reason
> for kconfig to inheritate this.
> So unless there are good arguments consider alloing the spaces.
> If the current implmentation already supports optional spaces then I just 
> missed
> it whie reviewing.
>
> Sam

+1 from me.

I also find the rules for whitespace in Make confusing, and always
have to look them up when doing trickier stuff. Maybe they're the
result of people not considering whitespace initially, and stuff
getting tacked on later. GNU Make adds some alternate syntaxes with
quotes.

I was going to mention shell, but it looks like you already did. :)

If we go with Make-like syntax, maybe we could at least have a variant
with fewer whitespace gotchas.

Cheers,
Ulf

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Ulf Magnusson]

On Sun, May 20, 2018 at 4:50 PM, Sam Ravnborg  wrote:
> Will the following be equal:
>
> $(foo,abc,$(x),$(y))
> $(foo, abc, $(x), $(y))
>
> make is rather annoying as space is significant, but there seems no good 
> reason
> for kconfig to inheritate this.
> So unless there are good arguments consider alloing the spaces.
> If the current implmentation already supports optional spaces then I just 
> missed
> it whie reviewing.
>
> Sam

+1 from me.

I also find the rules for whitespace in Make confusing, and always
have to look them up when doing trickier stuff. Maybe they're the
result of people not considering whitespace initially, and stuff
getting tacked on later. GNU Make adds some alternate syntaxes with
quotes.

I was going to mention shell, but it looks like you already did. :)

If we go with Make-like syntax, maybe we could at least have a variant
with fewer whitespace gotchas.

Cheers,
Ulf

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Sam Ravnborg]

Hi Masahiro,

> Hmm, your suggestion is more shell-oriented parsing.
...
> Probably, I found it was much more complex,
> then I chose Make-like simpler one.

Agree that the simple one should be the best choice for now.
We shall not add anything more complex unless it is really needed.

Sam

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Sam Ravnborg]

Hi Masahiro,

> Hmm, your suggestion is more shell-oriented parsing.
...
> Probably, I found it was much more complex,
> then I chose Make-like simpler one.

Agree that the simple one should be the best choice for now.
We shall not add anything more complex unless it is really needed.

Sam

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Masahiro Yamada]

Hi Sam,


2018-05-21 15:16 GMT+09:00 Sam Ravnborg :
> Hi Masahiro
>
>> >> + char *(*func)(int argc, char *argv[], int old_argc, char 
>> >> *old_argv[]);
>> >> +};
>> > If a typedef was provided for the function then ...
>>
>>
>> Yes, I can do this,
>> but I may rather consider to simplify the code.
>
> Simplify is better.
>
>> > Will the following be equal:
>> >
>> > $(foo,abc,$(x),$(y))
>> > $(foo, abc, $(x), $(y))
>> >
>> > make is rather annoying as space is significant, but there seems no good 
>> > reason
>> > for kconfig to inheritate this.
>> > So unless there are good arguments consider alloing the spaces.
>> > If the current implmentation already supports optional spaces then I just 
>> > missed
>> > it whie reviewing.
>>
>>
>> I have been thinking of trimming the leading whitespaces.
>> (https://patchwork.kernel.org/patch/10405549/)
>>
>> This is trade-off vs "how to pass spaces as arguments?"
>
> Maybe allow strings to be passed enclosed in ""?
> Then it is simple to add whitespace.
>
> But the use of "" should be optional in all other cases.
> And the "" should be stripped.
>

Hmm, your suggestion is more shell-oriented parsing.


In Make, there is no concept of quoting
because it does not touch single-quote, double-quote, whitespaces etc. at all.


$(info "'@@"''  '" ' "' )

will print the message as they are.


This simplifies both the grammar and the parser implementation.

If we expand the quoting by Kconfig,
we need more careful consideration.


[1] In the following, would "hello   world" be expanded by Kconfig or by shell?
$(shell, echo "hello   world")

[2] Is a quoted comma delimiter or not?
$(if,",",$(A))


If remember I first examined shell-oriented expansion,
but I stopped.

Probably, I found it was much more complex,
then I chose Make-like simpler one.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Masahiro Yamada]

Hi Sam,


2018-05-21 15:16 GMT+09:00 Sam Ravnborg :
> Hi Masahiro
>
>> >> + char *(*func)(int argc, char *argv[], int old_argc, char 
>> >> *old_argv[]);
>> >> +};
>> > If a typedef was provided for the function then ...
>>
>>
>> Yes, I can do this,
>> but I may rather consider to simplify the code.
>
> Simplify is better.
>
>> > Will the following be equal:
>> >
>> > $(foo,abc,$(x),$(y))
>> > $(foo, abc, $(x), $(y))
>> >
>> > make is rather annoying as space is significant, but there seems no good 
>> > reason
>> > for kconfig to inheritate this.
>> > So unless there are good arguments consider alloing the spaces.
>> > If the current implmentation already supports optional spaces then I just 
>> > missed
>> > it whie reviewing.
>>
>>
>> I have been thinking of trimming the leading whitespaces.
>> (https://patchwork.kernel.org/patch/10405549/)
>>
>> This is trade-off vs "how to pass spaces as arguments?"
>
> Maybe allow strings to be passed enclosed in ""?
> Then it is simple to add whitespace.
>
> But the use of "" should be optional in all other cases.
> And the "" should be stripped.
>

Hmm, your suggestion is more shell-oriented parsing.


In Make, there is no concept of quoting
because it does not touch single-quote, double-quote, whitespaces etc. at all.


$(info "'@@"''  '" ' "' )

will print the message as they are.


This simplifies both the grammar and the parser implementation.

If we expand the quoting by Kconfig,
we need more careful consideration.


[1] In the following, would "hello   world" be expanded by Kconfig or by shell?
$(shell, echo "hello   world")

[2] Is a quoted comma delimiter or not?
$(if,",",$(A))


If remember I first examined shell-oriented expansion,
but I stopped.

Probably, I found it was much more complex,
then I chose Make-like simpler one.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Sam Ravnborg]

Hi Masahiro

> >> + char *(*func)(int argc, char *argv[], int old_argc, char 
> >> *old_argv[]);
> >> +};
> > If a typedef was provided for the function then ...
> 
> 
> Yes, I can do this,
> but I may rather consider to simplify the code.

Simplify is better.

> > Will the following be equal:
> >
> > $(foo,abc,$(x),$(y))
> > $(foo, abc, $(x), $(y))
> >
> > make is rather annoying as space is significant, but there seems no good 
> > reason
> > for kconfig to inheritate this.
> > So unless there are good arguments consider alloing the spaces.
> > If the current implmentation already supports optional spaces then I just 
> > missed
> > it whie reviewing.
> 
> 
> I have been thinking of trimming the leading whitespaces.
> (https://patchwork.kernel.org/patch/10405549/)
> 
> This is trade-off vs "how to pass spaces as arguments?"

Maybe allow strings to be passed enclosed in ""?
Then it is simple to add whitespace.

But the use of "" should be optional in all other cases.
And the "" should be stripped.

Sam

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Sam Ravnborg]

Hi Masahiro

> >> + char *(*func)(int argc, char *argv[], int old_argc, char 
> >> *old_argv[]);
> >> +};
> > If a typedef was provided for the function then ...
> 
> 
> Yes, I can do this,
> but I may rather consider to simplify the code.

Simplify is better.

> > Will the following be equal:
> >
> > $(foo,abc,$(x),$(y))
> > $(foo, abc, $(x), $(y))
> >
> > make is rather annoying as space is significant, but there seems no good 
> > reason
> > for kconfig to inheritate this.
> > So unless there are good arguments consider alloing the spaces.
> > If the current implmentation already supports optional spaces then I just 
> > missed
> > it whie reviewing.
> 
> 
> I have been thinking of trimming the leading whitespaces.
> (https://patchwork.kernel.org/patch/10405549/)
> 
> This is trade-off vs "how to pass spaces as arguments?"

Maybe allow strings to be passed enclosed in ""?
Then it is simple to add whitespace.

But the use of "" should be optional in all other cases.
And the "" should be stripped.

Sam

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Masahiro Yamada]

Sam,

2018-05-20 23:50 GMT+09:00 Sam Ravnborg :
> On Thu, May 17, 2018 at 03:16:46PM +0900, Masahiro Yamada wrote:
>> This commit adds a new concept 'function' to do more text processing
>> in Kconfig.
>>
>> A function call looks like this:
>>
>>   $(function,arg1,arg2,arg3,...)
>>
>> This commit adds the basic infrastructure to expand functions.
>> Change the text expansion helpers to take arguments.
>>
>> Signed-off-by: Masahiro Yamada 
>> ---
>>
>> Changes in v4:
>>   - Error out if arguments more than FUNCTION_MAX_ARGS are passed
>>   - Use a comma as a delimiter between the function name and the
>> first argument
>>   - Check the number of arguments accepted by each function
>>   - Support delayed expansion of arguments.
>> This will be needed to implement 'if' function
>>
>> Changes in v3:
>>   - Split base infrastructure and 'shell' function
>> into separate patches.
>>
>> Changes in v2:
>>   - Use 'shell' for getting stdout from the comment.
>> It was 'shell-stdout' in the previous version.
>>   - Simplify the implementation since the expansion has been moved to
>> lexer.
>>
>>  scripts/kconfig/preprocess.c | 168 
>> ---
>>  1 file changed, 159 insertions(+), 9 deletions(-)
>>
>> diff --git a/scripts/kconfig/preprocess.c b/scripts/kconfig/preprocess.c
>> index 1bf506c..5be28ec 100644
>> --- a/scripts/kconfig/preprocess.c
>> +++ b/scripts/kconfig/preprocess.c
>> @@ -3,12 +3,17 @@
>>  // Copyright (C) 2018 Masahiro Yamada 
>>
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>>
>>  #include "list.h"
>>
>> +#define ARRAY_SIZE(arr)  (sizeof(arr) / sizeof((arr)[0]))
>> +
>> +static char *expand_string_with_args(const char *in, int argc, char 
>> *argv[]);
>> +
>>  static void __attribute__((noreturn)) pperror(const char *format, ...)
>>  {
>>   va_list ap;
>> @@ -88,9 +93,85 @@ void env_write_dep(FILE *f, const char *autoconfig_name)
>>   }
>>  }
>>
>> -static char *eval_clause(const char *in)
>> +/*
>> + * Built-in functions
>> + */
>> +struct function {
>> + const char *name;
>> + unsigned int min_args;
>> + unsigned int max_args;
>> + bool expand_args;
>> + char *(*func)(int argc, char *argv[], int old_argc, char *old_argv[]);
>> +};
> If a typedef was provided for the function then ...


Yes, I can do this,
but I may rather consider to simplify the code.


>> +
>> +static const struct function function_table[] = {
>> + /* Name MIN MAX EXP?Function */
>> +};
>> +
>> +#define FUNCTION_MAX_ARGS16
>> +
>> +static char *function_expand_arg_and_call(char *(*func)(int argc, char 
>> *argv[],
>> + int old_argc,
>> + char *old_argv[]),
>> +   int argc, char *argv[],
>> +   int old_argc, char *old_argv[])
> this could be much easier to read.
>
>> +{
>> + char *expanded_argv[FUNCTION_MAX_ARGS], *res;
>> + int i;
>> +
>> + for (i = 0; i < argc; i++)
>> + expanded_argv[i] = expand_string_with_args(argv[i],
>> +old_argc, old_argv);
>
> No check for too many arguments here - maybe it is done in some other place.

Right.
This has already been checked by eval_clause().


>> +
>> + res = func(argc, expanded_argv, 0, NULL);
>> +
>> + for (i = 0; i < argc; i++)
>> + free(expanded_argv[i]);
>> +
>> + return res;
>> +}
>> +
>> +static char *function_call(const char *name, int argc, char *argv[],
>> +int old_argc, char *old_argv[])
>> +{
>> + const struct function *f;
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(function_table); i++) {
>> + f = _table[i];
>> + if (strcmp(f->name, name))
>> + continue;
>> +
>> + if (argc < f->min_args)
>> + pperror("too few function arguments passed to '%s'",
>> + name);
>> +
>> + if (argc > f->max_args)
>> + pperror("too many function arguments passed to '%s'",
>> + name);
> Number of arguments checked here, but max_args is not assiged in this patch.

This is added to function_table[] by later patches.


>
>> +
>> + if (f->expand_args)
>> + return function_expand_arg_and_call(f->func, argc, 
>> argv,
>> + old_argc, 
>> old_argv);
>> + return f->func(argc, argv, old_argc, old_argv);
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +/*
>> + * Evaluate a clause with arguments.  argc/argv are arguments from the upper
>> + * function call.
>> + *
>> + * Returned string must be freed 

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Masahiro Yamada]

Sam,

2018-05-20 23:50 GMT+09:00 Sam Ravnborg :
> On Thu, May 17, 2018 at 03:16:46PM +0900, Masahiro Yamada wrote:
>> This commit adds a new concept 'function' to do more text processing
>> in Kconfig.
>>
>> A function call looks like this:
>>
>>   $(function,arg1,arg2,arg3,...)
>>
>> This commit adds the basic infrastructure to expand functions.
>> Change the text expansion helpers to take arguments.
>>
>> Signed-off-by: Masahiro Yamada 
>> ---
>>
>> Changes in v4:
>>   - Error out if arguments more than FUNCTION_MAX_ARGS are passed
>>   - Use a comma as a delimiter between the function name and the
>> first argument
>>   - Check the number of arguments accepted by each function
>>   - Support delayed expansion of arguments.
>> This will be needed to implement 'if' function
>>
>> Changes in v3:
>>   - Split base infrastructure and 'shell' function
>> into separate patches.
>>
>> Changes in v2:
>>   - Use 'shell' for getting stdout from the comment.
>> It was 'shell-stdout' in the previous version.
>>   - Simplify the implementation since the expansion has been moved to
>> lexer.
>>
>>  scripts/kconfig/preprocess.c | 168 
>> ---
>>  1 file changed, 159 insertions(+), 9 deletions(-)
>>
>> diff --git a/scripts/kconfig/preprocess.c b/scripts/kconfig/preprocess.c
>> index 1bf506c..5be28ec 100644
>> --- a/scripts/kconfig/preprocess.c
>> +++ b/scripts/kconfig/preprocess.c
>> @@ -3,12 +3,17 @@
>>  // Copyright (C) 2018 Masahiro Yamada 
>>
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>>
>>  #include "list.h"
>>
>> +#define ARRAY_SIZE(arr)  (sizeof(arr) / sizeof((arr)[0]))
>> +
>> +static char *expand_string_with_args(const char *in, int argc, char 
>> *argv[]);
>> +
>>  static void __attribute__((noreturn)) pperror(const char *format, ...)
>>  {
>>   va_list ap;
>> @@ -88,9 +93,85 @@ void env_write_dep(FILE *f, const char *autoconfig_name)
>>   }
>>  }
>>
>> -static char *eval_clause(const char *in)
>> +/*
>> + * Built-in functions
>> + */
>> +struct function {
>> + const char *name;
>> + unsigned int min_args;
>> + unsigned int max_args;
>> + bool expand_args;
>> + char *(*func)(int argc, char *argv[], int old_argc, char *old_argv[]);
>> +};
> If a typedef was provided for the function then ...


Yes, I can do this,
but I may rather consider to simplify the code.


>> +
>> +static const struct function function_table[] = {
>> + /* Name MIN MAX EXP?Function */
>> +};
>> +
>> +#define FUNCTION_MAX_ARGS16
>> +
>> +static char *function_expand_arg_and_call(char *(*func)(int argc, char 
>> *argv[],
>> + int old_argc,
>> + char *old_argv[]),
>> +   int argc, char *argv[],
>> +   int old_argc, char *old_argv[])
> this could be much easier to read.
>
>> +{
>> + char *expanded_argv[FUNCTION_MAX_ARGS], *res;
>> + int i;
>> +
>> + for (i = 0; i < argc; i++)
>> + expanded_argv[i] = expand_string_with_args(argv[i],
>> +old_argc, old_argv);
>
> No check for too many arguments here - maybe it is done in some other place.

Right.
This has already been checked by eval_clause().


>> +
>> + res = func(argc, expanded_argv, 0, NULL);
>> +
>> + for (i = 0; i < argc; i++)
>> + free(expanded_argv[i]);
>> +
>> + return res;
>> +}
>> +
>> +static char *function_call(const char *name, int argc, char *argv[],
>> +int old_argc, char *old_argv[])
>> +{
>> + const struct function *f;
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(function_table); i++) {
>> + f = _table[i];
>> + if (strcmp(f->name, name))
>> + continue;
>> +
>> + if (argc < f->min_args)
>> + pperror("too few function arguments passed to '%s'",
>> + name);
>> +
>> + if (argc > f->max_args)
>> + pperror("too many function arguments passed to '%s'",
>> + name);
> Number of arguments checked here, but max_args is not assiged in this patch.

This is added to function_table[] by later patches.


>
>> +
>> + if (f->expand_args)
>> + return function_expand_arg_and_call(f->func, argc, 
>> argv,
>> + old_argc, 
>> old_argv);
>> + return f->func(argc, argv, old_argc, old_argv);
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +/*
>> + * Evaluate a clause with arguments.  argc/argv are arguments from the upper
>> + * function call.
>> + *
>> + * Returned string must be freed when done
>> + */
>> +static char *eval_clause(const char *in, int argc, char 

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Sam Ravnborg]

On Thu, May 17, 2018 at 03:16:46PM +0900, Masahiro Yamada wrote:
> This commit adds a new concept 'function' to do more text processing
> in Kconfig.
> 
> A function call looks like this:
> 
>   $(function,arg1,arg2,arg3,...)
> 
> This commit adds the basic infrastructure to expand functions.
> Change the text expansion helpers to take arguments.
> 
> Signed-off-by: Masahiro Yamada 
> ---
> 
> Changes in v4:
>   - Error out if arguments more than FUNCTION_MAX_ARGS are passed
>   - Use a comma as a delimiter between the function name and the
> first argument
>   - Check the number of arguments accepted by each function
>   - Support delayed expansion of arguments.
> This will be needed to implement 'if' function
> 
> Changes in v3:
>   - Split base infrastructure and 'shell' function
> into separate patches.
> 
> Changes in v2:
>   - Use 'shell' for getting stdout from the comment.
> It was 'shell-stdout' in the previous version.
>   - Simplify the implementation since the expansion has been moved to
> lexer.
> 
>  scripts/kconfig/preprocess.c | 168 
> ---
>  1 file changed, 159 insertions(+), 9 deletions(-)
> 
> diff --git a/scripts/kconfig/preprocess.c b/scripts/kconfig/preprocess.c
> index 1bf506c..5be28ec 100644
> --- a/scripts/kconfig/preprocess.c
> +++ b/scripts/kconfig/preprocess.c
> @@ -3,12 +3,17 @@
>  // Copyright (C) 2018 Masahiro Yamada 
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  
>  #include "list.h"
>  
> +#define ARRAY_SIZE(arr)  (sizeof(arr) / sizeof((arr)[0]))
> +
> +static char *expand_string_with_args(const char *in, int argc, char *argv[]);
> +
>  static void __attribute__((noreturn)) pperror(const char *format, ...)
>  {
>   va_list ap;
> @@ -88,9 +93,85 @@ void env_write_dep(FILE *f, const char *autoconfig_name)
>   }
>  }
>  
> -static char *eval_clause(const char *in)
> +/*
> + * Built-in functions
> + */
> +struct function {
> + const char *name;
> + unsigned int min_args;
> + unsigned int max_args;
> + bool expand_args;
> + char *(*func)(int argc, char *argv[], int old_argc, char *old_argv[]);
> +};
If a typedef was provided for the function then ...

> +
> +static const struct function function_table[] = {
> + /* Name MIN MAX EXP?Function */
> +};
> +
> +#define FUNCTION_MAX_ARGS16
> +
> +static char *function_expand_arg_and_call(char *(*func)(int argc, char 
> *argv[],
> + int old_argc,
> + char *old_argv[]),
> +   int argc, char *argv[],
> +   int old_argc, char *old_argv[])
this could be much easier to read.

> +{
> + char *expanded_argv[FUNCTION_MAX_ARGS], *res;
> + int i;
> +
> + for (i = 0; i < argc; i++)
> + expanded_argv[i] = expand_string_with_args(argv[i],
> +old_argc, old_argv);

No check for too many arguments here - maybe it is done in some other place.

> +
> + res = func(argc, expanded_argv, 0, NULL);
> +
> + for (i = 0; i < argc; i++)
> + free(expanded_argv[i]);
> +
> + return res;
> +}
> +
> +static char *function_call(const char *name, int argc, char *argv[],
> +int old_argc, char *old_argv[])
> +{
> + const struct function *f;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(function_table); i++) {
> + f = _table[i];
> + if (strcmp(f->name, name))
> + continue;
> +
> + if (argc < f->min_args)
> + pperror("too few function arguments passed to '%s'",
> + name);
> +
> + if (argc > f->max_args)
> + pperror("too many function arguments passed to '%s'",
> + name);
Number of arguments checked here, but max_args is not assiged in this patch.


> +
> + if (f->expand_args)
> + return function_expand_arg_and_call(f->func, argc, argv,
> + old_argc, old_argv);
> + return f->func(argc, argv, old_argc, old_argv);
> + }
> +
> + return NULL;
> +}
> +
> +/*
> + * Evaluate a clause with arguments.  argc/argv are arguments from the upper
> + * function call.
> + *
> + * Returned string must be freed when done
> + */
> +static char *eval_clause(const char *in, int argc, char *argv[])
>  {
> - char *res, *name;
> + char *tmp, *prev, *p, *res, *name;
> + int new_argc = 0;
> + char *new_argv[FUNCTION_MAX_ARGS];
> + int nest = 0;
> + int i;
>  
>   /*
>* Returns an empty string because '$()' should be evaluated
> @@ -99,10 +180,69 @@ static char 

Re: [PATCH v4 07/31] kconfig: add built-in function support

[Sam Ravnborg]

On Thu, May 17, 2018 at 03:16:46PM +0900, Masahiro Yamada wrote:
> This commit adds a new concept 'function' to do more text processing
> in Kconfig.
> 
> A function call looks like this:
> 
>   $(function,arg1,arg2,arg3,...)
> 
> This commit adds the basic infrastructure to expand functions.
> Change the text expansion helpers to take arguments.
> 
> Signed-off-by: Masahiro Yamada 
> ---
> 
> Changes in v4:
>   - Error out if arguments more than FUNCTION_MAX_ARGS are passed
>   - Use a comma as a delimiter between the function name and the
> first argument
>   - Check the number of arguments accepted by each function
>   - Support delayed expansion of arguments.
> This will be needed to implement 'if' function
> 
> Changes in v3:
>   - Split base infrastructure and 'shell' function
> into separate patches.
> 
> Changes in v2:
>   - Use 'shell' for getting stdout from the comment.
> It was 'shell-stdout' in the previous version.
>   - Simplify the implementation since the expansion has been moved to
> lexer.
> 
>  scripts/kconfig/preprocess.c | 168 
> ---
>  1 file changed, 159 insertions(+), 9 deletions(-)
> 
> diff --git a/scripts/kconfig/preprocess.c b/scripts/kconfig/preprocess.c
> index 1bf506c..5be28ec 100644
> --- a/scripts/kconfig/preprocess.c
> +++ b/scripts/kconfig/preprocess.c
> @@ -3,12 +3,17 @@
>  // Copyright (C) 2018 Masahiro Yamada 
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  
>  #include "list.h"
>  
> +#define ARRAY_SIZE(arr)  (sizeof(arr) / sizeof((arr)[0]))
> +
> +static char *expand_string_with_args(const char *in, int argc, char *argv[]);
> +
>  static void __attribute__((noreturn)) pperror(const char *format, ...)
>  {
>   va_list ap;
> @@ -88,9 +93,85 @@ void env_write_dep(FILE *f, const char *autoconfig_name)
>   }
>  }
>  
> -static char *eval_clause(const char *in)
> +/*
> + * Built-in functions
> + */
> +struct function {
> + const char *name;
> + unsigned int min_args;
> + unsigned int max_args;
> + bool expand_args;
> + char *(*func)(int argc, char *argv[], int old_argc, char *old_argv[]);
> +};
If a typedef was provided for the function then ...

> +
> +static const struct function function_table[] = {
> + /* Name MIN MAX EXP?Function */
> +};
> +
> +#define FUNCTION_MAX_ARGS16
> +
> +static char *function_expand_arg_and_call(char *(*func)(int argc, char 
> *argv[],
> + int old_argc,
> + char *old_argv[]),
> +   int argc, char *argv[],
> +   int old_argc, char *old_argv[])
this could be much easier to read.

> +{
> + char *expanded_argv[FUNCTION_MAX_ARGS], *res;
> + int i;
> +
> + for (i = 0; i < argc; i++)
> + expanded_argv[i] = expand_string_with_args(argv[i],
> +old_argc, old_argv);

No check for too many arguments here - maybe it is done in some other place.

> +
> + res = func(argc, expanded_argv, 0, NULL);
> +
> + for (i = 0; i < argc; i++)
> + free(expanded_argv[i]);
> +
> + return res;
> +}
> +
> +static char *function_call(const char *name, int argc, char *argv[],
> +int old_argc, char *old_argv[])
> +{
> + const struct function *f;
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(function_table); i++) {
> + f = _table[i];
> + if (strcmp(f->name, name))
> + continue;
> +
> + if (argc < f->min_args)
> + pperror("too few function arguments passed to '%s'",
> + name);
> +
> + if (argc > f->max_args)
> + pperror("too many function arguments passed to '%s'",
> + name);
Number of arguments checked here, but max_args is not assiged in this patch.


> +
> + if (f->expand_args)
> + return function_expand_arg_and_call(f->func, argc, argv,
> + old_argc, old_argv);
> + return f->func(argc, argv, old_argc, old_argv);
> + }
> +
> + return NULL;
> +}
> +
> +/*
> + * Evaluate a clause with arguments.  argc/argv are arguments from the upper
> + * function call.
> + *
> + * Returned string must be freed when done
> + */
> +static char *eval_clause(const char *in, int argc, char *argv[])
>  {
> - char *res, *name;
> + char *tmp, *prev, *p, *res, *name;
> + int new_argc = 0;
> + char *new_argv[FUNCTION_MAX_ARGS];
> + int nest = 0;
> + int i;
>  
>   /*
>* Returns an empty string because '$()' should be evaluated
> @@ -99,10 +180,69 @@ static char *eval_clause(const char *in)
>   if (!*in)
>   return