Re: [PATCH 08/23] kconfig: add 'macro' keyword to support user-defined function
On Fri, Feb 16, 2018 at 11:44:25PM -0500, Nicolas Pitre wrote: > On Sat, 17 Feb 2018, Ulf Magnusson wrote: > > > On Sat, Feb 17, 2018 at 3:30 AM, Nicolas Pitre wrote: > > > On Sat, 17 Feb 2018, Ulf Magnusson wrote: > > > > > >> On Fri, Feb 16, 2018 at 02:49:31PM -0500, Nicolas Pitre wrote: > > >> > On Sat, 17 Feb 2018, Masahiro Yamada wrote: > > >> > > > >> > > Now, we got a basic ability to test compiler capability in Kconfig. > > >> > > > > >> > > config CC_HAS_STACKPROTECTOR > > >> > > bool > > >> > > default $(shell $CC -Werror -fstack-protector -c -x c > > >> > > /dev/null -o /dev/null) > > >> > > > > >> > > This works, but it is ugly to repeat this long boilerplate. > > >> > > > > >> > > We want to describe like this: > > >> > > > > >> > > config CC_HAS_STACKPROTECTOR > > >> > > bool > > >> > > default $(cc-option -fstack-protector) > > >> > > > > >> > > It is straight-forward to implement a new function, but I do not like > > >> > > to hard-code specialized functions like this. Hence, here is another > > >> > > feature to add functions from Kconfig files. > > >> > > > > >> > > A user-defined function can be defined as a string type symbol with > > >> > > a special keyword 'macro'. It can be referenced in the same way as > > >> > > built-in functions. This feature was also inspired by Makefile where > > >> > > user-defined functions are referenced by $(call func-name, args...), > > >> > > but I omitted the 'call' to makes it shorter. > > >> > > > > >> > > The macro definition can contain $(1), $(2), ... which will be > > >> > > replaced > > >> > > with arguments from the caller. > > >> > > > > >> > > Example code: > > >> > > > > >> > > config cc-option > > >> > > string > > >> > > macro $(shell $CC -Werror $(1) -c -x c /dev/null -o > > >> > > /dev/null) > > >> > > > >> > I think this syntax for defining a macro shouldn't start with the > > >> > "config" keyword, unless you want it to be part of the config symbol > > >> > space and land it in .config. And typing it as a "string" while it > > >> > actually returns y/n (hence a bool) is also strange. > > >> > > > >> > What about this instead: > > >> > > > >> > macro cc-option > > >> > bool $(shell $CC -Werror $(1) -c -x c /dev/null -o /dev/null) > > >> > > > >> > This makes it easier to extend as well if need be. > > >> > > > >> > > > >> > Nicolas > > >> > > >> I haven't gone over the patchset in detail yet and might be missing > > >> something here, but if this is just meant to be a textual shorthand, > > >> then why give it a type at all? > > > > > > It is meant to be like a user-defined function. > > > > > >> Do you think a simpler syntax like this would make sense? > > >> > > >> macro cc-option "$(shell $CC -Werror $(1) -c -x c /dev/null -o > > >> /dev/null)" > > >> > > >> That's the most general version, where you could use it for other stuff > > >> besides $(shell ...) as well, just to keep parity. > > > > > > This is not extendable. Let's imagine that you might want to implement > > > some kind of conditionals some day e.g.: > > > > > > macro complex_test > > > bool $(shell foo) if LOCKDEP_SUPPORT > > > bool y if DEBUG_DRIVER > > > bool n > > > > I still don't quite get the semantics here. How would the behavior > > change if the type was changed to say string or int in some or all of > > the lines? > > I admit this wouldn't make sense to have multiple different types. In > this example, the bool keyword acts as syntactic sugar more than > anything else. > > > Since the current model is to evaluate $() while the Kconfig files are > > being parsed, would this require evaluating Kconfig expressions during > > parsing? There is a relatively clean and (somewhat) easy to understand > > parsing/evaluation separation at the moment, which I like. > > Agreed. Let's forget about the conditionals then. > > > Nicolas This is also related to why it feels off to me to (at least for its own sake) make macro definitions mimic symbol definitions. To me, parsing being a different domain makes it "okay" to use a different syntax for macros compared to symbol definitions, especially if it happens to be handier. It even makes things less confusing, because there's less risk of mixing up the two domains (it's rare to mix up the preprocessor with C "proper", since the syntax is so different). More practically, I'm not sure that macro foo "definition" would be that hard to extend in practice, if you'd ever need to. You could always add a new keyword: fancy-macro/function/whatever foo ... I admit it'd be a bit ugly if you'd ever end up with something like macro foo "definition" bit_ugly It's still not the end of the world though, IMO, and I suspect there'd be better-looking options if you'd need to extend things on the macro side. That macro syntax seems like the simplest possible thing to me, with no obvious major dr
Re: [PATCH 08/23] kconfig: add 'macro' keyword to support user-defined function
On Sat, 17 Feb 2018, Ulf Magnusson wrote: > On Sat, Feb 17, 2018 at 3:30 AM, Nicolas Pitre wrote: > > On Sat, 17 Feb 2018, Ulf Magnusson wrote: > > > >> On Fri, Feb 16, 2018 at 02:49:31PM -0500, Nicolas Pitre wrote: > >> > On Sat, 17 Feb 2018, Masahiro Yamada wrote: > >> > > >> > > Now, we got a basic ability to test compiler capability in Kconfig. > >> > > > >> > > config CC_HAS_STACKPROTECTOR > >> > > bool > >> > > default $(shell $CC -Werror -fstack-protector -c -x c > >> > > /dev/null -o /dev/null) > >> > > > >> > > This works, but it is ugly to repeat this long boilerplate. > >> > > > >> > > We want to describe like this: > >> > > > >> > > config CC_HAS_STACKPROTECTOR > >> > > bool > >> > > default $(cc-option -fstack-protector) > >> > > > >> > > It is straight-forward to implement a new function, but I do not like > >> > > to hard-code specialized functions like this. Hence, here is another > >> > > feature to add functions from Kconfig files. > >> > > > >> > > A user-defined function can be defined as a string type symbol with > >> > > a special keyword 'macro'. It can be referenced in the same way as > >> > > built-in functions. This feature was also inspired by Makefile where > >> > > user-defined functions are referenced by $(call func-name, args...), > >> > > but I omitted the 'call' to makes it shorter. > >> > > > >> > > The macro definition can contain $(1), $(2), ... which will be replaced > >> > > with arguments from the caller. > >> > > > >> > > Example code: > >> > > > >> > > config cc-option > >> > > string > >> > > macro $(shell $CC -Werror $(1) -c -x c /dev/null -o > >> > > /dev/null) > >> > > >> > I think this syntax for defining a macro shouldn't start with the > >> > "config" keyword, unless you want it to be part of the config symbol > >> > space and land it in .config. And typing it as a "string" while it > >> > actually returns y/n (hence a bool) is also strange. > >> > > >> > What about this instead: > >> > > >> > macro cc-option > >> > bool $(shell $CC -Werror $(1) -c -x c /dev/null -o /dev/null) > >> > > >> > This makes it easier to extend as well if need be. > >> > > >> > > >> > Nicolas > >> > >> I haven't gone over the patchset in detail yet and might be missing > >> something here, but if this is just meant to be a textual shorthand, > >> then why give it a type at all? > > > > It is meant to be like a user-defined function. > > > >> Do you think a simpler syntax like this would make sense? > >> > >> macro cc-option "$(shell $CC -Werror $(1) -c -x c /dev/null -o > >> /dev/null)" > >> > >> That's the most general version, where you could use it for other stuff > >> besides $(shell ...) as well, just to keep parity. > > > > This is not extendable. Let's imagine that you might want to implement > > some kind of conditionals some day e.g.: > > > > macro complex_test > > bool $(shell foo) if LOCKDEP_SUPPORT > > bool y if DEBUG_DRIVER > > bool n > > I still don't quite get the semantics here. How would the behavior > change if the type was changed to say string or int in some or all of > the lines? I admit this wouldn't make sense to have multiple different types. In this example, the bool keyword acts as syntactic sugar more than anything else. > Since the current model is to evaluate $() while the Kconfig files are > being parsed, would this require evaluating Kconfig expressions during > parsing? There is a relatively clean and (somewhat) easy to understand > parsing/evaluation separation at the moment, which I like. Agreed. Let's forget about the conditionals then. Nicolas
Re: [PATCH 08/23] kconfig: add 'macro' keyword to support user-defined function
On Sat, Feb 17, 2018 at 3:30 AM, Nicolas Pitre wrote: > On Sat, 17 Feb 2018, Ulf Magnusson wrote: > >> On Fri, Feb 16, 2018 at 02:49:31PM -0500, Nicolas Pitre wrote: >> > On Sat, 17 Feb 2018, Masahiro Yamada wrote: >> > >> > > Now, we got a basic ability to test compiler capability in Kconfig. >> > > >> > > config CC_HAS_STACKPROTECTOR >> > > bool >> > > default $(shell $CC -Werror -fstack-protector -c -x c /dev/null >> > > -o /dev/null) >> > > >> > > This works, but it is ugly to repeat this long boilerplate. >> > > >> > > We want to describe like this: >> > > >> > > config CC_HAS_STACKPROTECTOR >> > > bool >> > > default $(cc-option -fstack-protector) >> > > >> > > It is straight-forward to implement a new function, but I do not like >> > > to hard-code specialized functions like this. Hence, here is another >> > > feature to add functions from Kconfig files. >> > > >> > > A user-defined function can be defined as a string type symbol with >> > > a special keyword 'macro'. It can be referenced in the same way as >> > > built-in functions. This feature was also inspired by Makefile where >> > > user-defined functions are referenced by $(call func-name, args...), >> > > but I omitted the 'call' to makes it shorter. >> > > >> > > The macro definition can contain $(1), $(2), ... which will be replaced >> > > with arguments from the caller. >> > > >> > > Example code: >> > > >> > > config cc-option >> > > string >> > > macro $(shell $CC -Werror $(1) -c -x c /dev/null -o /dev/null) >> > >> > I think this syntax for defining a macro shouldn't start with the >> > "config" keyword, unless you want it to be part of the config symbol >> > space and land it in .config. And typing it as a "string" while it >> > actually returns y/n (hence a bool) is also strange. >> > >> > What about this instead: >> > >> > macro cc-option >> > bool $(shell $CC -Werror $(1) -c -x c /dev/null -o /dev/null) >> > >> > This makes it easier to extend as well if need be. >> > >> > >> > Nicolas >> >> I haven't gone over the patchset in detail yet and might be missing >> something here, but if this is just meant to be a textual shorthand, >> then why give it a type at all? > > It is meant to be like a user-defined function. > >> Do you think a simpler syntax like this would make sense? >> >> macro cc-option "$(shell $CC -Werror $(1) -c -x c /dev/null -o >> /dev/null)" >> >> That's the most general version, where you could use it for other stuff >> besides $(shell ...) as well, just to keep parity. > > This is not extendable. Let's imagine that you might want to implement > some kind of conditionals some day e.g.: > > macro complex_test > bool $(shell foo) if LOCKDEP_SUPPORT > bool y if DEBUG_DRIVER > bool n I still don't quite get the semantics here. How would the behavior change if the type was changed to say string or int in some or all of the lines? Since the current model is to evaluate $() while the Kconfig files are being parsed, would this require evaluating Kconfig expressions during parsing? There is a relatively clean and (somewhat) easy to understand parsing/evaluation separation at the moment, which I like. Do you have anything in mind that would be cleaner and simpler to implement in this way compared to using plain symbols? > > There is no real advantage to simplify the macro definition to its > simplest expression, unlike its actual usage. Maybe I'm being grumpy, but this feels like it's adding complexity rather than reducing it. I like the rest of this patchset, because the behavior is easy to understand and fits well with Kconfig's evaluation model: $() is just a kind of preprocessor that runs during parsing and does value substitution based on shell commands, possibly along with some helper macros to avoid repetition. I think we should think hard about whether we actually need anything more than that before complicating Kconfig even further "just in case." If the goal is simplification, then it's bad if we eventually end up with a bigger mess than the Makefiles. > >> Are there any cases where something more advanced than that might be >> warranted (e.g., macros that expand to complete expressions)? > > Maybe not now, but there is no need to close the door on the possibility > either. > > > Nicolas Kconfig has no notion of types for expressions by the way. The simplest way to look at it is that all symbols have a tristate value (which is n for non-bool/tristate symbols) and a string value. Which one gets used depends on the context. In A && B, the tristate values are used, and in A = B the string values are compared. In something like 'default "foo bar"', "foo bar" is actually a constant symbol. If we were to drop the straightforward preprocessor model, then constant symbols would no longer necessarily be constant. I have a feeling that that might turn Kconfig's internals even messier. Constant (and undefined) symbols en
Re: [PATCH 08/23] kconfig: add 'macro' keyword to support user-defined function
On Sat, 17 Feb 2018, Ulf Magnusson wrote: > On Fri, Feb 16, 2018 at 02:49:31PM -0500, Nicolas Pitre wrote: > > On Sat, 17 Feb 2018, Masahiro Yamada wrote: > > > > > Now, we got a basic ability to test compiler capability in Kconfig. > > > > > > config CC_HAS_STACKPROTECTOR > > > bool > > > default $(shell $CC -Werror -fstack-protector -c -x c /dev/null > > > -o /dev/null) > > > > > > This works, but it is ugly to repeat this long boilerplate. > > > > > > We want to describe like this: > > > > > > config CC_HAS_STACKPROTECTOR > > > bool > > > default $(cc-option -fstack-protector) > > > > > > It is straight-forward to implement a new function, but I do not like > > > to hard-code specialized functions like this. Hence, here is another > > > feature to add functions from Kconfig files. > > > > > > A user-defined function can be defined as a string type symbol with > > > a special keyword 'macro'. It can be referenced in the same way as > > > built-in functions. This feature was also inspired by Makefile where > > > user-defined functions are referenced by $(call func-name, args...), > > > but I omitted the 'call' to makes it shorter. > > > > > > The macro definition can contain $(1), $(2), ... which will be replaced > > > with arguments from the caller. > > > > > > Example code: > > > > > > config cc-option > > > string > > > macro $(shell $CC -Werror $(1) -c -x c /dev/null -o /dev/null) > > > > I think this syntax for defining a macro shouldn't start with the > > "config" keyword, unless you want it to be part of the config symbol > > space and land it in .config. And typing it as a "string" while it > > actually returns y/n (hence a bool) is also strange. > > > > What about this instead: > > > > macro cc-option > > bool $(shell $CC -Werror $(1) -c -x c /dev/null -o /dev/null) > > > > This makes it easier to extend as well if need be. > > > > > > Nicolas > > I haven't gone over the patchset in detail yet and might be missing > something here, but if this is just meant to be a textual shorthand, > then why give it a type at all? It is meant to be like a user-defined function. > Do you think a simpler syntax like this would make sense? > > macro cc-option "$(shell $CC -Werror $(1) -c -x c /dev/null -o > /dev/null)" > > That's the most general version, where you could use it for other stuff > besides $(shell ...) as well, just to keep parity. This is not extendable. Let's imagine that you might want to implement some kind of conditionals some day e.g.: macro complex_test bool $(shell foo) if LOCKDEP_SUPPORT bool y if DEBUG_DRIVER bool n There is no real advantage to simplify the macro definition to its simplest expression, unlike its actual usage. > Are there any cases where something more advanced than that might be > warranted (e.g., macros that expand to complete expressions)? Maybe not now, but there is no need to close the door on the possibility either. Nicolas
Re: [PATCH 08/23] kconfig: add 'macro' keyword to support user-defined function
On Fri, Feb 16, 2018 at 02:49:31PM -0500, Nicolas Pitre wrote: > On Sat, 17 Feb 2018, Masahiro Yamada wrote: > > > Now, we got a basic ability to test compiler capability in Kconfig. > > > > config CC_HAS_STACKPROTECTOR > > bool > > default $(shell $CC -Werror -fstack-protector -c -x c /dev/null -o > > /dev/null) > > > > This works, but it is ugly to repeat this long boilerplate. > > > > We want to describe like this: > > > > config CC_HAS_STACKPROTECTOR > > bool > > default $(cc-option -fstack-protector) > > > > It is straight-forward to implement a new function, but I do not like > > to hard-code specialized functions like this. Hence, here is another > > feature to add functions from Kconfig files. > > > > A user-defined function can be defined as a string type symbol with > > a special keyword 'macro'. It can be referenced in the same way as > > built-in functions. This feature was also inspired by Makefile where > > user-defined functions are referenced by $(call func-name, args...), > > but I omitted the 'call' to makes it shorter. > > > > The macro definition can contain $(1), $(2), ... which will be replaced > > with arguments from the caller. > > > > Example code: > > > > config cc-option > > string > > macro $(shell $CC -Werror $(1) -c -x c /dev/null -o /dev/null) > > I think this syntax for defining a macro shouldn't start with the > "config" keyword, unless you want it to be part of the config symbol > space and land it in .config. And typing it as a "string" while it > actually returns y/n (hence a bool) is also strange. > > What about this instead: > > macro cc-option > bool $(shell $CC -Werror $(1) -c -x c /dev/null -o /dev/null) > > This makes it easier to extend as well if need be. > > > Nicolas I haven't gone over the patchset in detail yet and might be missing something here, but if this is just meant to be a textual shorthand, then why give it a type at all? Do you think a simpler syntax like this would make sense? macro cc-option "$(shell $CC -Werror $(1) -c -x c /dev/null -o /dev/null)" That's the most general version, where you could use it for other stuff besides $(shell ...) as well, just to keep parity. You could then always just expand $() as a string, and maybe spit out "n" and "y" in the cases Linus suggested for $(shell ...). The existing logic for constant symbols should then take care of converting that into a tristate value where appropriate. If you go with that and want to support $() outside quotes, then $(foo) would just be a shorthand for "$(foo)" Are there any cases where something more advanced than that might be warranted (e.g., macros that expand to complete expressions)? It seems pretty nice and nonmagical otherwise. Cheers, Ulf
Re: [PATCH 08/23] kconfig: add 'macro' keyword to support user-defined function
On Sat, 17 Feb 2018, Masahiro Yamada wrote: > Now, we got a basic ability to test compiler capability in Kconfig. > > config CC_HAS_STACKPROTECTOR > bool > default $(shell $CC -Werror -fstack-protector -c -x c /dev/null -o > /dev/null) > > This works, but it is ugly to repeat this long boilerplate. > > We want to describe like this: > > config CC_HAS_STACKPROTECTOR > bool > default $(cc-option -fstack-protector) > > It is straight-forward to implement a new function, but I do not like > to hard-code specialized functions like this. Hence, here is another > feature to add functions from Kconfig files. > > A user-defined function can be defined as a string type symbol with > a special keyword 'macro'. It can be referenced in the same way as > built-in functions. This feature was also inspired by Makefile where > user-defined functions are referenced by $(call func-name, args...), > but I omitted the 'call' to makes it shorter. > > The macro definition can contain $(1), $(2), ... which will be replaced > with arguments from the caller. > > Example code: > > config cc-option > string > macro $(shell $CC -Werror $(1) -c -x c /dev/null -o /dev/null) I think this syntax for defining a macro shouldn't start with the "config" keyword, unless you want it to be part of the config symbol space and land it in .config. And typing it as a "string" while it actually returns y/n (hence a bool) is also strange. What about this instead: macro cc-option bool $(shell $CC -Werror $(1) -c -x c /dev/null -o /dev/null) This makes it easier to extend as well if need be. Nicolas
[PATCH 08/23] kconfig: add 'macro' keyword to support user-defined function
Now, we got a basic ability to test compiler capability in Kconfig.
config CC_HAS_STACKPROTECTOR
bool
default $(shell $CC -Werror -fstack-protector -c -x c /dev/null -o
/dev/null)
This works, but it is ugly to repeat this long boilerplate.
We want to describe like this:
config CC_HAS_STACKPROTECTOR
bool
default $(cc-option -fstack-protector)
It is straight-forward to implement a new function, but I do not like
to hard-code specialized functions like this. Hence, here is another
feature to add functions from Kconfig files.
A user-defined function can be defined as a string type symbol with
a special keyword 'macro'. It can be referenced in the same way as
built-in functions. This feature was also inspired by Makefile where
user-defined functions are referenced by $(call func-name, args...),
but I omitted the 'call' to makes it shorter.
The macro definition can contain $(1), $(2), ... which will be replaced
with arguments from the caller.
Example code:
config cc-option
string
macro $(shell $CC -Werror $(1) -c -x c /dev/null -o /dev/null)
config CC_HAS_STACKPROTECTOR
bool
default $(cc-option -fstack-protector)
Signed-off-by: Masahiro Yamada
---
Reminder for myself:
Update Documentation/kbuild/kconfig-language.txt
scripts/kconfig/function.c | 66 +
scripts/kconfig/kconf_id.c | 1 +
scripts/kconfig/lkc_proto.h | 1 +
scripts/kconfig/zconf.y | 8 ++
4 files changed, 71 insertions(+), 5 deletions(-)
diff --git a/scripts/kconfig/function.c b/scripts/kconfig/function.c
index 60e59be..f7f154d 100644
--- a/scripts/kconfig/function.c
+++ b/scripts/kconfig/function.c
@@ -14,7 +14,8 @@ static LIST_HEAD(function_list);
struct function {
const char *name;
- char *(*func)(int argc, char *argv[]);
+ char *(*func)(struct function *f, int argc, char *argv[]);
+ void *priv;
struct list_head node;
};
@@ -30,7 +31,9 @@ static struct function *func_lookup(const char *name)
return NULL;
}
-static void func_add(const char *name, char *(*func)(int argc, char *argv[]))
+static void func_add(const char *name,
+char *(*func)(struct function *f, int argc, char *argv[]),
+void *priv)
{
struct function *f;
@@ -43,6 +46,7 @@ static void func_add(const char *name, char *(*func)(int
argc, char *argv[]))
f = xmalloc(sizeof(*f));
f->name = name;
f->func = func;
+ f->priv = priv;
list_add_tail(&f->node, &function_list);
}
@@ -50,6 +54,7 @@ static void func_add(const char *name, char *(*func)(int
argc, char *argv[]))
static void func_del(struct function *f)
{
list_del(&f->node);
+ free(f->priv);
free(f);
}
@@ -63,7 +68,7 @@ static char *func_call(int argc, char *argv[])
return NULL;
}
- return f->func(argc, argv);
+ return f->func(f, argc, argv);
}
static char *func_eval(const char *func)
@@ -106,8 +111,59 @@ char *func_eval_n(const char *func, size_t n)
return res;
}
+/* run user-defined function */
+static char *do_macro(struct function *f, int argc, char *argv[])
+{
+ char *new;
+ char *src, *p, *res;
+ size_t newlen;
+ int n;
+
+ new = xmalloc(1);
+ *new = 0;
+
+ /*
+* This is a format string. $(1), $(2), ... must be replaced with
+* function arguments.
+*/
+ src = f->priv;
+ p = src;
+
+ while ((p = strstr(p, "$("))) {
+ if (isdigit(p[2]) && p[3] == ')') {
+ n = p[2] - '0';
+ if (n < argc) {
+ newlen = strlen(new) + (p - src) +
+ strlen(argv[n]) + 1;
+ new = xrealloc(new, newlen);
+ strncat(new, src, p - src);
+ strcat(new, argv[n]);
+ src = p + 4;
+ }
+ p += 2;
+ }
+ p += 2;
+ }
+
+ newlen = strlen(new) + strlen(src) + 1;
+ new = xrealloc(new, newlen);
+ strcat(new, src);
+
+ res = expand_string_value(new);
+
+ free(new);
+
+ return res;
+}
+
+/* add user-defined function (macro) */
+void func_add_macro(const char *name, char *macro)
+{
+ func_add(name, do_macro, macro);
+}
+
/* built-in functions */
-static char *do_shell(int argc, char *argv[])
+static char *do_shell(struct function *f, int argc, char *argv[])
{
static const char *pre = "(";
static const char *post = ") >/dev/null 2>&1";
@@ -136,7 +192,7 @@ static char *do_shell(int argc, char *argv[])
void func_init(void)
{
/* register built-in functions */
- func_add("shell", do_shell);
+ func_add("shell", do_shell, NU
