hokein added a comment.
In D122303#3402375 <https://reviews.llvm.org/D122303#3402375>, @sammccall wrote:
> The extraction of TestGrammar seems unrelated to the rest of the patch and it
> *isn't* actually shared between tests yet, which makes it a bit hard to
> review.
It is used in our previous Forest patch (not landed yet), but you're probably
right. I have removed the extraction code in this patch, will send out a
followup patch for the extraction afterwards.
================
Comment at: clang-tools-extra/pseudo/lib/GrammarBNF.cpp:55
};
for (const auto &Spec : Specs) {
Consider(Spec.Target);
----------------
sammccall wrote:
> You could identify dependencies here, and then use them to sort the
> uniquenonterminals before allocating SymbolIDs
not sure I get the your point -- we could move the topological sort here, but
it doesn't seem to be significantly better compared with the current solution.
================
Comment at: clang-tools-extra/pseudo/lib/GrammarBNF.cpp:135
+ };
+ std::vector<SymbolID> VisitStates(T->Nonterminals.size(), NotVisited);
+ std::function<void(SymbolID)> DFS = [&](SymbolID SID) -> void {
----------------
sammccall wrote:
> (is a vector of symbol id, or of states?)
oops, it should be the enum State
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122303/new/
https://reviews.llvm.org/D122303
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
