I think putting token kind first might be more readable at the call site -- going top-down, first the kind (general), then spelling (specific).
+assert(spelling != nullptr); ?
Could we make a combined function that does not require the user to make a distinction between punctuation and keywords?
We should also allow creating tokens that have a user-specified spelling, for example, identifiers and string literals.
So maybe define two functions:
// Uses the provided spelling. syntax::Leaf *createLeaf(syntax::Arena &A, tok::TokenKind K, StringRef Spelling); // Infers spelling if possible. syntax::Leaf *createLeaf(syntax::Arena &A, tok::TokenKind K);
Could you extract the part that I highlighted into a function, and deduplicate it across all tests?
First, thanks for the great comments!
That exists already! It is createLeaf as is!
I don't think the user cares very much about whether their token is a keyword or a punctuation. Furthermore, most callsites will have the token kind as an argument, so saying that it is a keyword or punctuation is redundant:
createKeyword(Arena, tok::kw_if); // Yeah, 'if' is a keyword.
The semantics of the combined function I'm proposing is "create a token that has one fixed spelling". The other function is "create a token where the spelling is user-provided".
I think the user cares about whether their token is a keyword.
It's is somewhat redundant yes, but I don't think that hurts, we are still specifying that the keyword being used is if.
I see your points, and I don't have a strong opinion so I'm updating the patch.