From: Masahiro Yamada <masahiroy@kernel.org>
To: linux-kbuild@vger.kernel.org
Cc: linux-kernel@vger.kernel.org,
Matt Porter <mporter@kernel.crashing.org>,
Alexandre Bounine <alex.bou9@gmail.com>,
Masahiro Yamada <masahiroy@kernel.org>,
Jonathan Corbet <corbet@lwn.net>,
Nathan Chancellor <nathan@kernel.org>,
Nicolas Schier <nicolas@fjasle.eu>,
linux-doc@vger.kernel.org
Subject: [PATCH 2/2] kconfig: do not imply the type of choice from the first entry
Date: Sat, 27 Apr 2024 19:42:31 +0900 [thread overview]
Message-ID: <20240427104231.2728905-3-masahiroy@kernel.org> (raw)
In-Reply-To: <20240427104231.2728905-1-masahiroy@kernel.org>
The followng two test cases are very similar, but the latter does not
work.
[test case 1]
choice
prompt "choose"
config A
bool "A"
if y
config B
bool "B"
endif
endchoice
[test case 2]
choice
prompt "choose"
if y
config A
bool "A"
config B
bool "B"
endif
endchoice
Since 'if y' is always true, both of them should be equivalent to:
choice
prompt "choose"
config A
bool "A"
config B
bool "B"
endchoice
However, the test case 2 warns:
Kconfig:1:warning: config symbol defined without type
If the type of choice is not specified, it is implied from the first
entry within the choice block.
When inferring the choice type, menu_finalize() checks only direct
children of the choice. At this point, the menu entries still exist
under the 'if' entry:
choice
\-- if y
|-- A
\-- B
Later, menu_finalize() re-parents the menu, so A and B will be lifted up
right under the choice:
choice
|-- if y
|-- A
\-- B
This is complex because menu_finalize() sets attributes, restructures
the menu tree, and checks the sanity at the same time, leading to some
bugs.
It would be possible to resolve it by setting the choice type after
re-parenting, but the current mechanism looks questionable to me.
Let's default all choices to 'bool' unless the type is specified.
This change makes sense because 99% of choice use cases are bool.
There exists only one 'tristate' choice in drivers/rapidio/Kconfig.
Another (much cleaner) approach would be to remove the tristate choice
support entirely, but I have not yet made up my mind.
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
Documentation/kbuild/kconfig-language.rst | 4 +---
| 11 -----------
scripts/kconfig/parser.y | 3 +++
scripts/kconfig/tests/choice/Kconfig | 2 +-
scripts/kconfig/tests/choice_value_with_m_dep/Kconfig | 2 +-
5 files changed, 6 insertions(+), 16 deletions(-)
diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
index 555c2f839969..42b975b8e0cf 100644
--- a/Documentation/kbuild/kconfig-language.rst
+++ b/Documentation/kbuild/kconfig-language.rst
@@ -400,9 +400,7 @@ choices::
This defines a choice group and accepts any of the above attributes as
options. A choice can only be of type bool or tristate. If no type is
-specified for a choice, its type will be determined by the type of
-the first choice element in the group or remain unknown if none of the
-choice elements have a type specified, as well.
+specified for a choice, its type will be the default 'bool'.
While a boolean choice only allows a single config entry to be
selected, a tristate choice also allows any number of config entries
--git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index e01b9ee87c05..134ef120ad08 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -323,17 +323,6 @@ static void _menu_finalize(struct menu *parent, bool inside_choice)
is_choice = true;
if (is_choice) {
- if (sym->type == S_UNKNOWN) {
- /* find the first choice value to find out choice type */
- current_entry = parent;
- for (menu = parent->list; menu; menu = menu->next) {
- if (menu->sym && menu->sym->type != S_UNKNOWN) {
- menu_set_type(menu->sym->type);
- break;
- }
- }
- }
-
/*
* Use the choice itself as the parent dependency of
* the contained items. This turns the mode of the
diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
index 613fa8c9c2d0..70ea3152d9b8 100644
--- a/scripts/kconfig/parser.y
+++ b/scripts/kconfig/parser.y
@@ -230,6 +230,9 @@ choice: T_CHOICE T_EOL
choice_entry: choice choice_option_list
{
+ if (current_entry->sym->type == S_UNKNOWN)
+ menu_set_type(S_BOOLEAN);
+
if (!current_entry->prompt) {
fprintf(stderr, "%s:%d: error: choice must have a prompt\n",
current_entry->filename, current_entry->lineno);
diff --git a/scripts/kconfig/tests/choice/Kconfig b/scripts/kconfig/tests/choice/Kconfig
index 8cdda40868a1..4dc0d3a1e089 100644
--- a/scripts/kconfig/tests/choice/Kconfig
+++ b/scripts/kconfig/tests/choice/Kconfig
@@ -18,7 +18,7 @@ config BOOL_CHOICE1
endchoice
choice
- prompt "tristate choice"
+ tristate "tristate choice"
default TRI_CHOICE1
config TRI_CHOICE0
diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig b/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
index bd970cec07d6..3e600c83279a 100644
--- a/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
+++ b/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
@@ -9,7 +9,7 @@ config DEP
default m
choice
- prompt "Tristate Choice"
+ tristate "Tristate Choice"
config CHOICE0
tristate "Choice 0"
--
2.40.1
next prev parent reply other threads:[~2024-04-27 10:43 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-27 10:42 [PATCH 0/2] kconfig: stop implification of choice type Masahiro Yamada
2024-04-27 10:42 ` [PATCH 1/2] rapidio: specify the type for tristate choice explicitly Masahiro Yamada
2024-05-07 8:30 ` Nicolas Schier
2024-04-27 10:42 ` Masahiro Yamada [this message]
2024-05-07 8:45 ` [PATCH 2/2] kconfig: do not imply the type of choice from the first entry Nicolas Schier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240427104231.2728905-3-masahiroy@kernel.org \
--to=masahiroy@kernel.org \
--cc=alex.bou9@gmail.com \
--cc=corbet@lwn.net \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mporter@kernel.crashing.org \
--cc=nathan@kernel.org \
--cc=nicolas@fjasle.eu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox