hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
it looks good.
================
Comment at: clang-tools-extra/pseudo/gen/Main.cpp:102
llvm::StringRef Name = G.table().AttributeValues[EID];
- assert(!Name.empty());
Out.os() << llvm::formatv("EXTENSION({0}, {1})\n", Name, EID);
----------------
I think this invariant is still true even in this patch, ExtensionID for empty
attribute value is 0, and we start from 1 in the loop.
================
Comment at: clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h:31
//
// contextual-override := IDENTIFIER [guard=Override]
//
----------------
nit: update the stale comment.
================
Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:27
-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 guardFinal(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() == "final";
-}
-bool guardModule(llvm::ArrayRef<const ForestNode *> RHS,
- const TokenStream &Tokens) {
- return Tokens.tokens()[RHS.front()->startTokenIndex()].text() == "module";
+bool isStringUD(const Token &Tok) { return !Tok.text().endswith("\""); }
+bool isCharUD(const Token &Tok) { return !Tok.text().endswith("'"); }
----------------
nit: not sure the abbreviation of UD is a clearer, took me a while to
understand it is for UserDefined, add a comment?
================
Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:156
llvm::DenseMap<ExtensionID, RuleGuard> buildGuards() {
+#define TOKEN_GUARD(kind, cond)
\
----------------
we might probably to consider to split it out from the CXX.cpp at some point in
the future. I think currently it is fine.
================
Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:171
+ {(RuleID)Rule::non_function_declarator_0declarator,
+ SYMBOL_GUARD(declarator, !isFunctionDeclarator(&N))},
+ {(RuleID)Rule::contextual_override_0identifier,
----------------
nit: add a blank line, I think function-declarator guard is different than the
contextual-sensitive guard.
================
Comment at: clang-tools-extra/pseudo/tool/ClangPseudo.cpp:48
static opt<bool> PrintForest("print-forest", desc("Print parse forest"));
+static opt<bool> ForestAbbrev("forest-abbrev", desc("Abbreviate parse forest"),
+ init(true));
----------------
I like this flag -- I have been wanted for several times.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130066/new/
https://reviews.llvm.org/D130066
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
