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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to