================
@@ -1471,6 +1485,10 @@ FormatToken *FormatTokenLexer::getNextToken() {
// the tok value seems to be needed. Not sure if there's a more elegant
// way.
FormatTok->Tok.setKind(tok::kw_if);
+ } else if (it->second == TT_TryMacro) {
+ FormatTok->Tok.setKind(tok::kw_try);
+ } else if (it->second == TT_CatchMacro) {
+ FormatTok->Tok.setKind(tok::kw_catch);
----------------
harrishancock wrote:
We could call `FormatTok->setFinalizedType(it->second)` for the `TT_TryMacro`
and `TT_CatchMacro` cases, and remove those TokenTypes from the `isNoneOf()`
check in TokenAnnotator.cpp, as you point out.
I imagine we could probably just call `setFinalizedType(it->second)`
unconditionally for all such user-configured macros, which would allow us to
remove several other `TT_*Macro` TokenTypes from the `isNoneOf()` check, too.
That would increase the blast radius of the PR, but when I try it locally, all
tests do pass.
Do you have a preference? That is, between these options:
- `setFinalizedType()` on only `TT_TryMacro` and `TT_CatchMacro`, remove only
them from TokenAnnotator's `isNoneOf()` check
- `setFinalizedType()` on the above, plus `TT_IfMacro`, which is the only
existing user-configured macro which gets its TokenKind overridden
- `setFinalizedType()` on all user-configured macros (e.g.
`TT_NamespaceMacro`), remove them all from TokenAnnotator's `isNoneOf()` check
As far as removing the `setKind()` calls, I _think_ that's an orthogonal change
to whether we call `setType()` versus `setFinalizedType()` here.
For the TryMacros / CatchMacros feature to work at all, we still need to set
the TokenKind to `tok::kw_try` / `kw_catch` for them to be treated as `try` and
`catch`. If I'm reading the code correctly, the alternative would be:
- add or modify ~6 branches to the `case tok::identifier:` paths in
UnwrappedLineParser (similar to how
[ForEachMacro](https://github.com/llvm/llvm-project/blob/9ec7b025cf1806c4d1a9ab7541c7695e7453558f/clang/lib/Format/UnwrappedLineParser.cpp#L1653),
[NamespaceMacro, and
StatementMacro](https://github.com/llvm/llvm-project/blob/9ec7b025cf1806c4d1a9ab7541c7695e7453558f/clang/lib/Format/UnwrappedLineParser.cpp#L1695-L1702)
are handled).
- add or modify ~3 branches in TokenAnnotator to expand `tok::kw_try` /
`tok::kw_catch` handling to also check for `tok::identifier` + `TT_TryMacro` /
`TT_CatchMacro`, e.g.
[here](https://github.com/llvm/llvm-project/blob/73dec8293b8624668965c5fcf80baa9456fbd0c6/clang/lib/Format/TokenAnnotator.cpp#L411).
(Note that the existing comment where we override `TT_IfMacro` to `tok::kw_if`
says that [its TokenKind is currently
`tok::kw_unknown`](https://github.com/llvm/llvm-project/blob/9ec7b025cf1806c4d1a9ab7541c7695e7453558f/clang/lib/Format/FormatTokenLexer.cpp#L1469)
at that point, but from reading the rest of the code, I think it must actually
be `tok::identifier`. I'll verify and update the comment.)
Both options -- overriding the TokenKind here versus adding special cases to
check both TokenKind and TokenType elsewhere -- seem differently hacky. The
precedent set by IfMacros of overriding the TokenKind seems a bit more
maintainable, though, as any new branches that check for the keyword (`if`,
`try`, `catch`) wouldn't have to remember that the `TT_*Macro` TokenTypes
exist. That said, it looks like there's plenty of precedent for that style in
the codebase.
https://github.com/llvm/llvm-project/pull/183352
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
