[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