[PATCH] D130160: [pseudo] Eliminate the dangling-else syntax ambiguity.
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 20 12:58:58 PDT 2022
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Language.h:25
+ const TokenStream &Tokens;
+ SymbolID Lookahead;
+};
----------------
passing the position in the stream would be more general, there's no reason in particular that the amount of lookahead the guard needs is the same as what we have in GLR
OTOH as long as it *is* the same this is probably faster
================
Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:25
-bool guardOverride(llvm::ArrayRef<const ForestNode *> RHS,
- const TokenStream &Tokens) {
- assert(RHS.size() == 1 &&
- RHS.front()->symbol() == tokenSymbol(clang::tok::identifier));
- return Tokens.tokens()[RHS.front()->startTokenIndex()].text() == "override";
+bool guardOverride(const GuardParams &Params) {
+ assert(Params.RHS.size() == 1 &&
----------------
nit: I would suggest calling this conventionally P instead of Params, the bodies of these are really verbose
================
Comment at: clang-tools-extra/pseudo/test/cxx/dangling-else.cpp:3
+
+if (true)
+ if (true) {
----------------
nit: place this all on one line so it's clear there's no indentation-based heuristics at play?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130160/new/
https://reviews.llvm.org/D130160
More information about the cfe-commits
mailing list