Re: [PATCH v2 05/28] docs: kdoc_re: add a C tokenizer

Mauro Carvalho Chehab
2026-03-17T08:21:58Z

On Mon, 16 Mar 2026 17:40:22 -0600
Jonathan Corbet <corbet@lwn.net> wrote:

> Randy Dunlap <rdunlap@infradead.org> writes:
>
> > Uh, I find this review confusing.
> > Do your (Jon) comments refer to the code above them?
> > (more below)
>
> They do
>
> Or, at least, they did...but they clearly got mixed up in the sending
> somewhere. Below is the intended version...

Oh, I should have read this one before... Ignore my previous comment.
I'll move the answers to this reply, and answer the other ones.

> > tools/lib/python/kdoc/kdoc_re.py | 234 +++++++++++++++++++++++++++++++
> > 1 file changed, 234 insertions(+)
> >
> > diff --git a/tools/lib/python/kdoc/kdoc_re.py b/tools/lib/python/kdoc/kdoc_re.py
> > index 085b89a4547c..7bed4e9a8810 100644
> > --- a/tools/lib/python/kdoc/kdoc_re.py
> > +++ b/tools/lib/python/kdoc/kdoc_re.py
> > @@ -141,6 +141,240 @@ class KernRe:
> >
> > return self.last_match.groups()
> >
> > +class TokType():
> > +
> > + @staticmethod
> > + def __str__(val):
> > + ""Return the name of an enum value""
> > + return TokType._name_by_val.get(val, f"UNKNOWN({val})")
>
> What is this class supposed to do?

This __str__() method ensures that, when printing a CToken object,
the name will be displayed, instead of a number. This is really
useful when debugging.

See, if I add a print:

<snip>
--- a/tools/lib/python/kdoc/kdoc_parser.py
+++ b/tools/lib/python/kdoc/kdoc_parser.py
@@ -87,6 +87,7 @@ def trim_private_members(text):
"""

tokens = CTokenizer(text)
+ print(tokens.tokens)
return str(tokens)

</snip>

the tokens will appear as names at the output:

$ ./scripts/kernel-doc -none er.c
[CToken(CToken.ENUM, "enum", 0, (0, 0, 0)), CToken(CToken.SPACE, " ", 4, (0, 0, 0)), CToken(CToken.NAME, "dmub_abm_ace_curve_type", 5, (0, 0, 0)), CToken(CToken.SPACE, " ", 28, (0, 0, 0)), CToken(CToken.BEGIN, "{", 29, (0, 0, 1)), CToken(CToken.SPACE, " ", 30, (0, 0, 1)), CToken(CToken.COMMENT, "/**
* ACE curve as defined by the SW layer. */", 31, (0, 0, 1)), CToken(CToken.SPACE, " ", 86, (0, 0, 1)), CToken(CToken.NAME, "ABM_ACE_CURVE_TYPE__SW", 87, (0, 0, 1)), CToken(CToken.SPACE, " ", 109, (0, 0, 1)), CToken(CToken.OP, "=", 110, (0, 0, 1)), CToken(CToken.SPACE, " ", 111, (0, 0, 1)), CToken(CToken.NUMBER, "0", 112, (0, 0, 1)), CToken(CToken.PUNC, ",", 113, (0, 0, 1)), CToken(CToken.SPACE, " ", 114, (0, 0, 1)), CToken(CToken.COMMENT, "/**
* ACE curve as defined by the SW to HW translation interface layer. */", 115, (0, 0, 1)), CToken(CToken.SPACE, " ", 198, (0, 0, 1)), CToken(CToken.NAME, "ABM_ACE_CURVE_TYPE__SW_IF", 199, (0, 0, 1)), CToken(CToken.SPACE, " ", 224, (0, 0, 1)), CToken(CToken.OP, "=", 225, (0, 0, 1)), CToken(CToken.SPACE, " ", 226, (0, 0, 1)), CToken(CToken.NUMBER, "1", 227, (0, 0, 1)), CToken(CToken.PUNC, ",", 228, (0, 0, 1)), CToken(CToken.SPACE, " ", 229, (0, 0, 1)), CToken(CToken.END, "}", 230, (0, 0, 0)), CToken(CToken.PUNC, ";", 231, (0, 0, 0))]

>
> > +
> > +class CToken():
> > + ""
> > + Data class to define a C token.
> > + ""
> > +
> > + # Tokens that can be used by the parser. Works like an C enum.
> > +
> > + COMMENT = 0 #: A standard C or C99 comment, including delimiter.
> > + STRING = 1 #: A string, including quotation marks.
> > + CHAR = 2 #: A character, including apostophes.
> > + NUMBER = 3 #: A number.
> > + PUNC = 4 #: A puntuation mark: ``;`` / ``,`` / ``.``.
> > + BEGIN = 5 #: A begin character: ``{`` / ``[`` / ``(``.
> > + END = 6 #: A end character: ``}`` / ``]`` / ``)``.
> > + CPP = 7 #: A preprocessor macro.
> > + HASH = 8 #: The hash character - useful to handle other macros.
> > + OP = 9 #: A C operator (add, subtract, ...).
> > + STRUCT = 10 #: A ``struct`` keyword.
> > + UNION = 11 #: An ``union`` keyword.
> > + ENUM = 12 #: A ``struct`` keyword.
> > + TYPEDEF = 13 #: A ``typedef`` keyword.
> > + NAME = 14 #: A name. Can be an ID or a type.
> > + SPACE = 15 #: Any space characters, including new lines
> > +
> > + MISMATCH = 255 #: an error indicator: should never happen in practice.
> > +
> > + # Dict to convert from an enum interger into a string.
> > + _name_by_val = {v: k for k, v in dict(vars()).items() if isinstance(v, int)}
> > +
> > + # Dict to convert from string to an enum-like integer value.
> > + _name_to_val = {k: v for v, k in _name_by_val.items()}
>
> This stuff strikes me as a bit overdone; _name_to_val is really just the
> variable list for the class, right?

Those two vars are a kind of magic: they create two dictionaries:

- _name_by_val converts a token integer into a string;
- _name_to_val converts a string to an integer.

I opted to use this approach for a couple of reasons:

1. using tok.kind == "BEGIN" (and similar) everywhere is harder to
maintain, as python won't check for typos. Now, if one writes:
CToken.BEGHIN, an error will be raised;

2. the cost to convert from string to int is O(1), so not much
a performance issue at the conversion;

3. using an integer on all checks should make the code faster as
it doesn't require a loop to check the string.

>
> > +
> > + @staticmethod
> > + def to_name(val):
> > + ""Convert from an integer value from CToken enum into a string""
> > +
> > + return CToken._name_by_val.get(val, f"UNKNOWN({val})")
> > +
> > + @staticmethod
> > + def from_name(name):
> > + ""Convert a string into a CToken enum value""
> > + if name in CToken._name_to_val:
> > + return CToken._name_to_val[name]
> > +
> > + return CToken.MISMATCH
> > +
> > + def __init__(self, kind, value, pos,
> > + brace_level, paren_level, bracket_level):
> > + self.kind = kind
> > + self.value = value
> > + self.pos = pos
> > + self.brace_level = brace_level
> > + self.paren_level = paren_level
> > + self.bracket_level = bracket_level
> > +
> > + def __repr__(self):
> > + name = self.to_name(self.kind)
> > + if isinstance(self.value, str):
> > + value = '"' + self.value + '"'
> > + else:
> > + value = self.value
> > +
> > + return f"CToken({name}, {value}, {self.pos}, " \
> > + f"{self.brace_level}, {self.paren_level}, {self.bracket_level})"
> > +
> > +#: Tokens to parse C code.
> > +TOKEN_LIST = [
>
> So these aren't "tokens", this is a list of regexes; how is it intended
> to be used?
>
> > + (CToken.COMMENT, r"//[^\n]*|/\*[\s\S]*?\*/"),
>
> How does "[\s\S]*" differ from plain old "*" ?

They are not identical, as "*" doesn't match "\n". As the tokenizer
also picks "\n" on several cases, like on comments, r"\s\S" works
better.

>
> > +
> > + (CToken.STRING, r'"(?:\\.|[^"\\])*"'),
> > + (CToken.CHAR, r"'(?:\\.|[^'\\])'"),
> > +
> > + (CToken.NUMBER, r"0[xX][0-9a-fA-F]+[uUlL]*|0[0-7]+[uUlL]*|"
> > + r"[0-9]+(\.[0-9]*)?([eE][+-]?[0-9]+)?[fFlL]*"),
> > +
> > + (CToken.PUNC, r"[;,\.]"),
> > +
> > + (CToken.BEGIN, r"[\[\(\{]"),
> > +
> > + (CToken.END, r"[\]\)\}]"),
> > +
> > + (CToken.CPP, r"#\s*(define|include|ifdef|ifndef|if|else|elif|endif|undef|pragma)\b"),
> > +
> > + (CToken.HASH, r"#"),
> > +
> > + (CToken.OP, r"\+\+|\-\-|\->|==|\!=|<=|>=|&&|\|\||<<|>>|\+=|\-=|\*=|/=|%="
> > + r"|&=|\|=|\^=|=|\+|\-|\*|/|%|<|>|&|\||\^|~|!|\?|\:"),
>
> "-" and "!" never need to be escaped.

"-" usually needs to be escaped, because it can be a range. I actually
tried without escaping it, but the regex failed. So I ended being
conservative.

>
> > +
> > + (CToken.STRUCT, r"\bstruct\b"),
> > + (CToken.UNION, r"\bunion\b"),
> > + (CToken.ENUM, r"\benum\b"),
> > + (CToken.TYPEDEF, r"\bkinddef\b"),
>
> "kinddef" ?

Should be "typedef".

This was due to a "sed s,type,kind," I applied to avoid using
"type" for the token type, as, when I started integrating it
with kdoc_re, it became confusing.

I'll fix at the next respin.

>
> > +
> > + (CToken.NAME, r"[A-Za-z_][A-Za-z0-9_]*"),
> > +
> > + (CToken.SPACE, r"[\s]+"),
>
> Don't need the [brackets] here

True. This was [ \t] and there as a separate token for new line.
I merged them, but forgot stripping the brackets.

Will cleanup at the next respin.

>
> > +
> > + (CToken.MISMATCH,r"."),
> > +]
> > +
> > +#: Handle C continuation lines.
> > +RE_CONT = KernRe(r"\\\n")
> > +
> > +RE_COMMENT_START = KernRe(r'/\*\s*')
> > +
> > +#: tokenizer regex. Will be filled at the first CTokenizer usage.
> > +re_scanner = None
>
> That seems weird, why don't you just initialize it here?

Yeah, I changed this one to:

def fill_re_scanner(token_list):
"""Ancillary routine to convert TOKEN_LIST into a finditer regex"""
re_tokens = []

for kind, pattern in token_list:
name = CToken.to_name(kind)
re_tokens.append(f"(?P<{name}>{pattern})")

return KernRe("|".join(re_tokens), re.MULTILINE | re.DOTALL)

RE_SCANNER = fill_re_scanner(TOKEN_LIST)

but I guess tis is on a patch later on.

>
> > +
> > +class CTokenizer():
> > + ""
> > + Scan C statements and definitions and produce tokens.
> > +
> > + When converted to string, it drops comments and handle public/private
> > + values, respecting depth.
> > + ""
> > +
> > + # This class is inspired and follows the basic concepts of:
> > + # https://docs.python.org/3/library/re.html#writing-a-tokenizer
> > +
> > + def _tokenize(self, source):
> > + ""
> > + Interactor that parses ``source``, splitting it into tokens, as defined
> > + at ``self.TOKEN_LIST``.
> > +
> > + The interactor returns a CToken class object.
> > + ""
>
> Do you mean "iterator" here?

Yes. will fix at the next respin.

>
> > +
> > + # Handle continuation lines. Note that kdoc_parser already has a
> > + # logic to do that. Still, let's keep it for completeness, as we might
> > + # end re-using this tokenizer outsize kernel-doc some day - or we may
> > + # eventually remove from there as a future cleanup.
> > + source = RE_CONT.sub(", source)
> > +
> > + brace_level = 0
> > + paren_level = 0
> > + bracket_level = 0
> > +
> > + for match in re_scanner.finditer(source):
> > + kind = CToken.from_name(match.lastgroup)
> > + pos = match.start()
> > + value = match.group()
> > +
> > + if kind == CToken.MISMATCH:
> > + raise RuntimeError(f"Unexpected token '{value}' on {pos}:\n\t{source}")
> > + elif kind == CToken.BEGIN:
> > + if value == '(':
> > + paren_level += 1
> > + elif value == '[':
> > + bracket_level += 1
> > + else: # value == '{'
> > + brace_level += 1
> > +
> > + elif kind == CToken.END:
> > + if value == ')' and paren_level > 0:
> > + paren_level -= 1
> > + elif value == ']' and bracket_level > 0:
> > + bracket_level -= 1
> > + elif brace_level > 0: # value == '}'
> > + brace_level -= 1
> > +
> > + yield CToken(kind, value, pos,
> > + brace_level, paren_level, bracket_level)
> > +
> > + def __init__(self, source):
>
> Putting __init__() first is fairly standard, methinks.

Yes, but __init__ calls _tokenize().

My personal preference is to have the caller methods before the methods
that actually call them, even inside a class, where the order doesn't
matter - or even in C, when we have an include with all prototypes.

But if you prefer, I can reorder it.

>
> > + ""
> > + Create a regular expression to handle TOKEN_LIST.
> > +
> > + While I generally don't like using regex group naming via:
> > + (?P<name>...)
> > +
> > + in this particular case, it makes sense, as we can pick the name
> > + when matching a code via re_scanner().
> > + ""
> > + global re_scanner
> > +
> > + if not re_scanner:
> > + re_tokens = []
> > +
> > + for kind, pattern in TOKEN_LIST:
> > + name = CToken.to_name(kind)
> > + re_tokens.append(f"(?P<{name}>{pattern})")
> > +
> > + re_scanner = KernRe("|".join(re_tokens), re.MULTILINE | re.DOTALL)
>
> I still don't understand why you do this here - this is all constant, right?

Yes. See above. I moved this logic to a function and called it during
module init time, for it to happen just once.

>
> > +
> > + self.tokens = []
> > + for tok in self._tokenize(source):
> > + self.tokens.append(tok)
>
> So you create a nice iterator structure, then just put it all together into a
> list anyway?

We could have used yield here, but what's the point? Due to C
transforms, we'll need to navigate on all tokens multiple times.

Having them on a list ends saving time, as we only need to
tokenize once per source code.

>
> > +
> > + def __str__(self):
> > + out="
> > + show_stack = [True]
> > +
> > + for tok in self.tokens:
> > + if tok.kind == CToken.BEGIN:
> > + show_stack.append(show_stack[-1])
> > +
> > + elif tok.kind == CToken.END:
> > + prev = show_stack[-1]
> > + if len(show_stack) > 1:
> > + show_stack.pop()
> > +
> > + if not prev and show_stack[-1]:
> > + #
> > + # Try to preserve indent
> > + #
> > + out += "\t" * (len(show_stack) - 1)
> > +
> > + out += str(tok.value)
> > + continue
> > +
> > + elif tok.kind == CToken.COMMENT:
> > + comment = RE_COMMENT_START.sub(", tok.value)
> > +
> > + if comment.startswith("private:"):
> > + show_stack[-1] = False
> > + show = False
> > + elif comment.startswith("public:"):
> > + show_stack[-1] = True
> > +
> > + continue
> > +
> > + if show_stack[-1]:
> > + out += str(tok.value)
> > +
> > + return out
> > +
> > +
> > #: Nested delimited pairs (brackets and parenthesis)
> > DELIMITER_PAIRS = {
> > '{': '}',
>
> Thanks,
>
> jon
>

Thanks,
Mauro