sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Great!
================
Comment at: clang-tools-extra/pseudo/include/clang-pseudo/GLR.h:117
struct ParseParams {
- // The grammar of the language we're going to parse.
- const Grammar &G;
- // The LR table which GLR uses to parse the input, should correspond to the
- // Grammar G.
- const LRTable &Table;
+ // The token stream being to parse.
+ const TokenStream &Code;
----------------
delete "being".
Or possibly the whole comment, this seems obvious?
================
Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:254
private:
+ bool canReduce(ExtensionID GuardID, SequenceRef RHS) const {
+ if (!GuardID)
----------------
nit: hadn't really intended SequenceRef to be a general vocabulary type, you
might just want ArrayRef to match the guard signature, up to you
================
Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:258
+ auto It = Lang.Guards.find(GuardID);
+ if (It == Lang.Guards.end()) {
+ llvm::dbgs() << " missing {0} guard implementation for rule {0}\n";
----------------
this should be guarded by LLVM_DEBUG
this whole thing might be clearer with lookup
```
if (auto Guard = Lang.Guards.lookup(GuardID))
return Guard(RHS.S, Params.Code);
LLVM_DEBUG(...);
return true;
```
================
Comment at: clang-tools-extra/pseudo/unittests/GLRTest.cpp:486
+ )bnf");
+ TestLang.Guards.insert(std::make_pair(
+ extensionID("TestOnly"),
----------------
insert(make_pair(...)) => emplace(...)?
================
Comment at: clang-tools-extra/pseudo/unittests/GLRTest.cpp:489
+ [&](llvm::ArrayRef<const ForestNode *> RHS, const TokenStream &Tokens) {
+ llvm::errs() << "callback!\n";
+ assert(RHS.size() == 1 &&
----------------
remove
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127448/new/
https://reviews.llvm.org/D127448
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
