kconfig: fix conditional prompt behavior for choice

When a prompt is followed by "if <expr>", the symbol is configurable
when the if-conditional evaluates to true.

A typical usage is as follows:

    menuconfig BLOCK
            bool "Enable the block layer" if EXPERT
            default y

When EXPERT=n, the prompt is hidden, but this config entry is still
active, and BLOCK is set to its default value 'y'. When EXPERT=y, the
prompt is shown, making BLOCK a user-configurable option.

This usage is common throughout the kernel tree, but it has never worked
within a choice block.

[Test Code]

    config EXPERT
            bool "Allow expert users to modify more options"

    choice
            prompt "Choose" if EXPERT

    config A
            bool "A"

    config B
            bool "B"

    endchoice

[Result]

    # CONFIG_EXPERT is not set

When the prompt is hidden, the choice block should produce the default
without asking for the user's preference. Hence, the output should be:

    # CONFIG_EXPERT is not set
    CONFIG_A=y
    # CONFIG_B is not set

Removing unnecessary hacks fixes the issue.

This commit also changes the behavior of 'select' by choice members.

[Test Code 2]

    config MODULES
            def_bool y
            modules

    config DEP
            def_tristate m

    if DEP

    choice
            prompt "choose"

    config A
            bool "A"
            select C

    endchoice

    config B
            def_bool y
            select D

    endif

    config C
            tristate

    config D
            tristate

The current output is as follows:

    CONFIG_MODULES=y
    CONFIG_DEP=m
    CONFIG_A=y
    CONFIG_B=y
    CONFIG_C=y
    CONFIG_D=m

With this commit, the output will be changed as follows:

    CONFIG_MODULES=y
    CONFIG_DEP=m
    CONFIG_A=y
    CONFIG_B=y
    CONFIG_C=m
    CONFIG_D=m

CONFIG_C will be changed to 'm' because 'select C' will inherit the
dependency on DEP, which is 'm'.

This change is aligned with the behavior of 'select' outside a choice
block; 'select D' depends on DEP, therefore D is selected by (B && DEP).

Note:

With this commit, allmodconfig will set CONFIG_USB_ROLE_SWITCH to 'm'
instead of 'y'. I did not see any build regression with this change.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 23c95e54660dbb8e76e0762fbe3d9e9209db4dc9..b1fbaf2ff792510b1a61d44f6b77096bad98d40c 100644 (file)
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -306,7 +306,7 @@ static void _menu_finalize(struct menu *parent, bool inside_choice)
        struct menu *menu, *last_menu;
        struct symbol *sym;
        struct property *prop;
-       struct expr *parentdep, *basedep, *dep, *dep2;
+       struct expr *basedep, *dep, *dep2;
 
        sym = parent->sym;
        if (parent->list) {
@@ -315,24 +315,6 @@ static void _menu_finalize(struct menu *parent, bool inside_choice)
                 * and propagate parent dependencies before moving on.
                 */
 
-               bool is_choice = false;
-
-               if (sym && sym_is_choice(sym))
-                       is_choice = true;
-
-               if (is_choice) {
-                       /*
-                        * Use the choice itself as the parent dependency of
-                        * the contained items. This turns the mode of the
-                        * choice into an upper bound on the visibility of the
-                        * choice value symbols.
-                        */
-                       parentdep = expr_alloc_symbol(sym);
-               } else {
-                       /* Menu node for 'menu', 'if' */
-                       parentdep = parent->dep;
-               }
-
                /* For each child menu node... */
                for (menu = parent->list; menu; menu = menu->next) {
                        /*
@@ -341,7 +323,7 @@ static void _menu_finalize(struct menu *parent, bool inside_choice)
                         */
                        basedep = rewrite_m(menu->dep);
                        basedep = expr_transform(basedep);
-                       basedep = expr_alloc_and(expr_copy(parentdep), basedep);
+                       basedep = expr_alloc_and(expr_copy(parent->dep), basedep);
                        basedep = expr_eliminate_dups(basedep);
                        menu->dep = basedep;
 
@@ -405,15 +387,12 @@ static void _menu_finalize(struct menu *parent, bool inside_choice)
                        }
                }
 
-               if (is_choice)
-                       expr_free(parentdep);
-
                /*
                 * Recursively process children in the same fashion before
                 * moving on
                 */
                for (menu = parent->list; menu; menu = menu->next)
-                       _menu_finalize(menu, is_choice);
+                       _menu_finalize(menu, sym && sym_is_choice(sym));
        } else if (!inside_choice && sym) {
                /*
                 * Automatic submenu creation. If sym is a symbol and A, B, C,
@@ -541,17 +520,6 @@ static void _menu_finalize(struct menu *parent, bool inside_choice)
                sym_check_prop(sym);
                sym->flags |= SYMBOL_WARNED;
        }
-
-       /*
-        * For choices, add a reverse dependency (corresponding to a select) of
-        * '<visibility> && y'. This prevents the user from setting the choice
-        * mode to 'n' when the choice is visible.
-        */
-       if (sym && sym_is_choice(sym) && parent->prompt) {
-               sym->rev_dep.expr = expr_alloc_or(sym->rev_dep.expr,
-                               expr_alloc_and(parent->prompt->visible.expr,
-                                       expr_alloc_symbol(&symbol_yes)));
-       }
 }
 
 void menu_finalize(void)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index e5441378c4b0dcfcdfac1072a7a468b3f4bebf03..1cb8b6a22c5a57e14afea8de2c01cdf853c886fe 100644 (file)
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -868,7 +868,7 @@ const char *sym_get_string_value(struct symbol *sym)
 
 bool sym_is_changeable(struct symbol *sym)
 {
-       return sym->visible > sym->rev_dep.tri;
+       return !sym_is_choice(sym) && sym->visible > sym->rev_dep.tri;
 }
 
 HASHTABLE_DEFINE(sym_hashtable, SYMBOL_HASHSIZE);