Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 2/3] checkkconfigsymbols.py: Fix Kconfig parsing to find
 'if' lines
To:     Masahiro Yamada <masahiroy@kernel.org>
Cc:     Michal Marek <michal.lkml@markovi.net>,
        Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Valentin Rothberg <valentinrothberg@gmail.com>
References: <20210822192205.43210-1-arielmarcovitch@gmail.com>
 <20210822192205.43210-3-arielmarcovitch@gmail.com>
 <CAK7LNATy17OQ900ThKJwHRy35+4Yg=9CRNg9Zp0tZ_O=uQ+kaw@mail.gmail.com>
From:   Ariel Marcovitch <arielmarcovitch@gmail.com>
Message-ID: <2bba4c0a-8639-1d3a-5dd5-8e2576f6ab77@gmail.com>
Date:   Sun, 29 Aug 2021 16:17:59 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <CAK7LNATy17OQ900ThKJwHRy35+4Yg=9CRNg9Zp0tZ_O=uQ+kaw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk

Hello again!

On 24/08/2021 16:30, Masahiro Yamada wrote:
 > On Mon, Aug 23, 2021 at 4:22 AM Ariel Marcovitch
 > <arielmarcovitch@gmail.com> wrote:
 >>
 >> When parsing Kconfig files to find symbol definitions and references,
 >> lines after a 'help' line are skipped until a new config definition
 >> starts.
 >>
 >> However, it is quite common to define a config and then make some other
 >> configs depend on it by adding an 'if' line. This kind of kconfig
 >> statement usually appears after a config definition which might contain
 >> a 'help' section. The 'if' line is skipped in parse_kconfig_file()
 >> because it is not a config definition.
 >>
 >> This means that symbols referenced in this kind of statements are
 >> ignored by this function and thus are not considered undefined
 >> references in case the symbol is not defined.
 >>
 >> The REGEX_KCONFIG_STMT regex can't be used because the other types of
 >> statements can't break help lines.
 >>
 >> Define a new regex for matching 'if' statements and stop the 'help'
 >> skipping in case it is encountered.
 >>
 >> Signed-off-by: Ariel Marcovitch <arielmarcovitch@gmail.com>
 >> ---
 >>  scripts/checkkconfigsymbols.py | 8 +++++++-
 >>  1 file changed, 7 insertions(+), 1 deletion(-)
 >>
 >> diff --git a/scripts/checkkconfigsymbols.py 
b/scripts/checkkconfigsymbols.py
 >> index b9b0f15e5880..875e9a2c14b2 100755
 >> --- a/scripts/checkkconfigsymbols.py
 >> +++ b/scripts/checkkconfigsymbols.py
 >> @@ -26,6 +26,7 @@ EXPR = r"(?:" + OPERATORS + r"|\s|" + SYMBOL + r")+"
 >>  DEFAULT = r"default\s+.*?(?:if\s.+){,1}"
 >>  STMT = r"^\s*(?:if|select|imply|depends\s+on|(?:" + DEFAULT + 
r"))\s+" + EXPR
 >>  SOURCE_SYMBOL = r"(?:\W|\b)+[D]{,1}CONFIG_(" + SYMBOL + r")"
 >> +IF_LINE = r"^\s*(?:if)\s+" + EXPR
 >
 >
 > Why is it enclosed by "(?: )"   ?
 >
 > "(?:if)"  seems to the same as "if"
Oh you are absolutely right.
I just mindlessly copied the STMT regex and removed the other types :)
 >
 >
 >
 >
 >
 >
 >>
 >>  # regex objects
 >>  REGEX_FILE_KCONFIG = re.compile(r".*Kconfig[\.\w+\-]*$")
 >> @@ -35,11 +36,11 @@ REGEX_KCONFIG_DEF = re.compile(DEF)
 >>  REGEX_KCONFIG_EXPR = re.compile(EXPR)
 >>  REGEX_KCONFIG_STMT = re.compile(STMT)
 >>  REGEX_KCONFIG_HELP = re.compile(r"^\s+help\s*$")
 >> +REGEX_KCONFIG_IF_LINE = re.compile(IF_LINE)
 >>  REGEX_FILTER_SYMBOLS = re.compile(r"[A-Za-z0-9]$")
 >>  REGEX_NUMERIC = re.compile(r"0[xX][0-9a-fA-F]+|[0-9]+")
 >>  REGEX_QUOTES = re.compile("(\"(.*?)\")")
 >>
 >> -
 >>  def parse_options():
 >>      """The user interface of this module."""
 >>      usage = "Run this tool to detect Kconfig symbols that are 
referenced but " \
 >> @@ -445,6 +446,11 @@ def parse_kconfig_file(kfile):
 >>          line = line.strip('\n')
 >>          line = line.split("#")[0]  # ignore comments
 >>
 >> +        # 'if EXPR' lines can be after help lines
 >> +        # The if line itself is handled later
 >> +        if REGEX_KCONFIG_IF_LINE.match(line):
 >> +            skip = False
 >> +
 >
 >
 > I do not think this is the right fix.
 > There are similar patterns where
 > config references are ignored.
 >
 > For example, FOO and BAR are ignored
 > in the following cases.
 >
 > ex1)
 >
 > choice
 >           prompt "foo"
 >           default FOO
 >
 >
 >
 > ex2)
 >
 > menu "bar"
 >            depends on BAR
 >
 >
 >
 >
 > The help block ends with shallower indentation.
So IIUC we need to measure the indentation when we encounter a help
statement and in the next lines look for a line with a different depth
(which is not an empty line because these are allowed).
 >
 >
 >
 >
 >>          if REGEX_KCONFIG_DEF.match(line):
 >>              symbol_def = REGEX_KCONFIG_DEF.findall(line)
 >>              defined.append(symbol_def[0])
 >> --
 >> 2.25.1
 >>
 >
 >
 > --
 > Best Regards
 > Masahiro Yamada

Thanks for your time!

Ariel Marcovitch