This is an archive of the discontinued LLVM Phabricator instance.

[pseudo] Introduce parse forest.
ClosedPublic

Authored by hokein on Mar 21 2022, 7:39 AM.

Details

Summary

Parse forest is the output of the GLR parser, it is a tree-like DAG
which presents all possible parse trees without duplicating subparse structures.

This is a patch split from https://reviews.llvm.org/D121150.

Diff Detail

Event Timeline

hokein created this revision.Mar 21 2022, 7:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2022, 7:39 AM
Herald added a subscriber: mgorny. · View Herald Transcript
hokein requested review of this revision.Mar 21 2022, 7:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2022, 7:39 AM
hokein updated this revision to Diff 416936.Mar 21 2022, 7:41 AM

small tweaks.

sammccall accepted this revision.Mar 21 2022, 9:46 AM
sammccall added inline comments.
clang-tools-extra/pseudo/include/clang-pseudo/Forest.h
36

I'd replace ", e.g." with a period. (There are in fact no other cases, and likely never will be, so "e.g." is a bit misleading)

40

maybe also "Nodes do not have parent pointers"?

144

nit: llvm::all_of

153

nit: either drop this blank line or add them above

clang-tools-extra/pseudo/unittests/ForestTest.cpp
30

I'd prefer id -> symbol, to disambiguate vs rule id

41

nit: I think this would be read more clearly as "ruleFor" or "rule" or at least "onlyRule" to emphasize "rule" more and "single" less.

Combining with previous:

createSequence(symbol("id-expression"), ruleFor("id-expression"), {&T.front()})

45

nit: point out the failure more actionably

"singleRule(" << NonTerminalName << ") but " << NonTerminalName << " has " << N << " rules!"

66

not sure what you intend by the l here, but unsigned long != size_t in general.
(I think in standard LLVM warning config 3u is fine)

This revision is now accepted and ready to land.Mar 21 2022, 9:46 AM
hokein updated this revision to Diff 417225.Mar 22 2022, 2:41 AM
hokein marked 7 inline comments as done.

address review comments.

This revision was automatically updated to reflect the committed changes.
RKSimon added a subscriber: RKSimon.Aug 9 2022, 9:13 AM
RKSimon added inline comments.
clang-tools-extra/pseudo/include/clang-pseudo/Forest.h
159

@hokein Static analysis is warning about sizeof(this) - should it be sizeof(*this) ?

hokein added inline comments.Aug 9 2022, 12:50 PM
clang-tools-extra/pseudo/include/clang-pseudo/Forest.h
159

Good catch! You're right, it should be sizeof(*this). Fixed in c2c5c39c401b39d2057ebb19b843f53deb950f6e.