[4/5] kconfig: Introduce "showif" to factor out conditions on visibility

On Thu, 2015-05-14 at 08:36 -0700, Josh Triplett wrote:
> kconfig implicitly creates a submenu whenever a series of symbols all
> have dependencies or prompt-visibility expressions that all depend on a
> preceeding symbol.  For instance, the series of symbols following
> "menuconfig EXPERT" that all have "if EXPERT" on their prompt will all
> appear as a submenu of EXPERT.
> 
> However, this implicit submenuing will break if any intervening symbol
> loses its "if EXPERT"; doing so causes the subsequent symbols to appear
> in the parent menu ("General setup").  This has happened many times, and
> it's easy to miss that the entire block should have that same
> expression.
> 
> For submenus created from "depends" dependencies, these can be converted
> to a single wrapping "if expr ... endif" block around all the submenu
> items.  However, there's no equivalent for invisible items, for which
> the prompt is hidden but the symbol may potentially be enabled.  For
> instance, many items in the EXPERT menu are hidden if EXPERT is
> disabled, but they have "default y" or are determined by some other
> expression.
> 
> To handle this case, introduce a new kconfig construct, "showif", which
> does the same thing as "if" but for visibility expressions rather than
> dependencies.  Every symbol in a "showif expr ... endif" block
> effectively has "if expr" added to its prompt.
> 
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>
> Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
>  scripts/kconfig/menu.c               |   4 +-
>  scripts/kconfig/zconf.gperf          |   1 +
>  scripts/kconfig/zconf.hash.c_shipped | 258 ++++++++--------
>  scripts/kconfig/zconf.tab.c_shipped  | 561 +++++++++++++++++++----------------
>  scripts/kconfig/zconf.y              |  28 +-
>  5 files changed, 459 insertions(+), 393 deletions(-)

Documentation/kbuild/kconfig-language.txt is suspiciously absent from
this list.

> --- a/scripts/kconfig/menu.c
> +++ b/scripts/kconfig/menu.c
> @@ -344,7 +344,9 @@ void menu_finalize(struct menu *parent)
>  		basedep = expr_eliminate_dups(expr_transform(basedep));
>  		last_menu = NULL;
>  		for (menu = parent->next; menu; menu = menu->next) {
> -			dep = menu->prompt ? menu->prompt->visible.expr : menu->dep;
> +			dep = menu->prompt ? menu->prompt->visible.expr
> +			    : menu->visibility ? menu->visibility
> +			    : menu->dep;
>  			if (!expr_contains_symbol(dep, sym))
>  				break;
>  			if (expr_depends_symbol(dep, sym))

If this hunk was obvious to you, and not the result of some trial and
error, then I think there's an update to MAINTAINERS I should send.

> --- a/scripts/kconfig/zconf.hash.c_shipped
> +++ b/scripts/kconfig/zconf.hash.c_shipped

I was happy to see gperf embeds the command-line to create this file in
the output. Is there some option to bison that does that too?

> --- a/scripts/kconfig/zconf.y
> +++ b/scripts/kconfig/zconf.y
> @@ -31,7 +31,7 @@ struct symbol *symbol_hash[SYMBOL_HASHSIZE];
>  static struct menu *current_menu, *current_entry;
>  
>  %}
> -%expect 30
> +%expect 31

As above: if you didn't add this just to shut up bison there's a patch
to MAINTAINERS I'd like to submit.

>  %union
>  {
> @@ -55,6 +55,7 @@ static struct menu *current_menu, *current_entry;
>  %token <id>T_HELP
>  %token <string> T_HELPTEXT
>  %token <id>T_IF
> +%token <id>T_SHOWIF
>  %token <id>T_ENDIF
>  %token <id>T_DEPENDS
>  %token <id>T_OPTIONAL
> @@ -84,7 +85,7 @@ static struct menu *current_menu, *current_entry;
>  %type <expr> if_expr
>  %type <id> end
>  %type <id> option_name
> -%type <menu> if_entry menu_entry choice_entry
> +%type <menu> if_entry showif_entry menu_entry choice_entry
>  %type <string> symbol_option_arg word_opt
>  
>  %destructor {
> @@ -92,7 +93,7 @@ static struct menu *current_menu, *current_entry;
>  		$$->file->name, $$->lineno);
>  	if (current_menu == $$)
>  		menu_end_menu();
> -} if_entry menu_entry choice_entry
> +} if_entry showif_entry menu_entry choice_entry
>  
>  %{
>  /* Include zconf.hash.c here so it can see the token constants. */
> @@ -125,6 +126,7 @@ option_name:
>  common_stmt:
>  	  T_EOL
>  	| if_stmt
> +	| showif_stmt
>  	| comment_stmt
>  	| config_stmt
>  	| menuconfig_stmt
> @@ -321,6 +323,14 @@ if_entry: T_IF expr nl
>  	$$ = menu_add_menu();
>  };
>  
> +showif_entry: T_SHOWIF expr nl

Naive question: nl and not T_EOL?

> +{
> +	printd(DEBUG_PARSE, "%s:%d:showif\n", zconf_curname(), zconf_lineno());
> +	menu_add_entry(NULL);
> +	menu_add_visibility($2);
> +	$$ = menu_add_menu();

This is what "if FOO" "endif" does too: add a, say, anonymous menu. That
_feels_ wrong, but I don't dare to dive deeper in the yacc code.

> +};
> +
>  if_end: end
>  {
>  	if (zconf_endtoken($1, T_IF, T_ENDIF)) {
> @@ -329,9 +339,20 @@ if_end: end
>  	}
>  };
>  
> +showif_end: end
> +{
> +	if (zconf_endtoken($1, T_SHOWIF, T_ENDIF)) {
> +		menu_end_menu();
> +		printd(DEBUG_PARSE, "%s:%d:endif\n", zconf_curname(), zconf_lineno());
> +	}
> +};
> +
>  if_stmt: if_entry if_block if_end
>  ;
>  
> +showif_stmt: showif_entry if_block showif_end
> +;
> +
>  if_block:
>  	  /* empty */
>  	| if_block common_stmt
> @@ -524,6 +545,7 @@ static const char *zconf_tokenname(int token)
>  	case T_CHOICE:		return "choice";
>  	case T_ENDCHOICE:	return "endchoice";
>  	case T_IF:		return "if";
> +	case T_SHOWIF:		return "showif";
>  	case T_ENDIF:		return "endif";
>  	case T_DEPENDS:		return "depends";
>  	case T_VISIBLE:		return "visible";

So this seems to work. I'd like to kick this around for some more time,
but unless some of the people that actually understand yacc and the
funky logic on which kconfig stands, start to shout something like this
should probably go in.

That being said, I'd like to bicycle-shed "showif". I'd rather reuse
"visibility", which is obscure but already used, apparently for the same
reason, and neatly matches the code this touches. I'll paste a (rough)

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[4/5] kconfig: Introduce "showif" to factor out conditions on visibility

Commit Message

Comments

Patch