sammccall marked an inline comment as done. sammccall added inline comments.
================ Comment at: clang-tools-extra/pseudo/unittests/ForestTest.cpp:138 + + const auto *B = &Arena.createSequence(symbol("B"), ruleFor("B"), {Star}); + const auto *A1 = &Arena.createSequence(symbol("A"), ruleFor("A"), {B}); ---------------- hokein wrote: > I was confused by the forest here -- the fact that it doesn't reflect the > grammar (I was trying to figure out why there is an ambiguous node for A by > reading the grammar above) > > I think there should be a comment clarifying the gap between the grammar and > forest -- this is an artificial forest which is only for testing the dump > purpose -- we only need the symbol bits from the grammar (I actual think > `GLRTest::buildGrammar` is a better fit for the tests in this file). This forest does matches the grammar, unless I'm missing something. It has imperfect sharing (two nodes with the same rule+range), but strictly it's our GLR parser that guarantees perfect sharing, not the forest itself. Added a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128805/new/ https://reviews.llvm.org/D128805 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits