This patch might cover too many things (at least we could split the guide/glr implementation bit), but want to give you an overview of the picture first.

Thanks for the fix!

Thanks!

rebase, and emit string chunks rather than a long raw string literal (to make msvc happy)

Thanks, this looks a good start version!

In D125311#3524190, @sammccall wrote:

In D125311#3523388, @hokein wrote:

In D125311#3503452, @sammccall wrote:

This looks good, but if that's the idiom we should add it to cook() also.

cook is more tricky, it takes a TokenStream, and returns a new one with an allocator payload:

• the input TokenStream doesn't have a payload, this is OK
• the input TokenStream has a payload, but what if the type of payload is not an allocator?

Payload is opaque, it can be a pair of {new payload, inner payload}. e.g.

void TokenStream::addPayload(shared_ptr<void> P) {
  Payload = make_shared<pair<shared_ptr<void>, shared_ptr<void>>>(std::move(P), std::move(Payload));
}

add addPayload method, and share payload in cook as well

In D125863#3522384, @kadircet wrote:

In D125863#3522025, @hokein wrote:

sorry, I might be lack of the context, where is the user complaint? I'm not sure which cases are improved with this patch.

so the complaint was from an example with an active branch, eg:

#if 1
void foo();
#else
#endif

The user was complaining that #endif was inactive depending on first or second branch being active. Hence my initial thoughts were also around cases which had at least one "active" branch, but you're right this actually is confusing when there are no active branches.

format fix

Address comments:

In D125311#3503452, @sammccall wrote:

This looks good, but if that's the idiom we should add it to cook() also.

sorry, I might be lack of the context, where is the user complaint? I'm not sure which cases are improved with this patch.

move the findNonterminal to Grammar.

This is a revised version of D125231, based on our discussion. It has a narrow scope: only nonterminals of the grammar are compiled; and it mainly focuses on interfaces;

Would including *every* nonterminal as a start symbol would blow the size up a bit?
This would eliminate some complexity in the interface.

add a comment in cxx.bnf explaining why not making all nonterminals as start symbol by default.

• adress comments
• rename TargetSymbol -> StartSymbol
• don't use StartSymbol for "_", rename and update a few occurrences
• move findNonterminal to Grammar.h since it is used in various places

Thanks!

Example: https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/pseudo/tool/ClangPseudo.cpp#L88

In D125226#3500691, @sammccall wrote:

address comments

This is just a prototype, wanting some early feedback before making further progress:

• compiling the generated Cxx.cpp is very slow (took minutes, mostly due to the LRTable::Actions);
• layering (location of the generated header file) is not super clear;

This looks a reasonable split to me!

Nice!

I think "// FOO_BAR_H" in your message should be "// FOO_BAR_H" (two space).

Thanks!

would be nice to land it in some of form.

rebase

fix a dangling reference of the source text in clang-pseudo.

address remaining comments:

• return GSS::node in the NewHeadCallback, rather the fields of GSS::Node;
• remove the unncessary NewHeadResult structure in unittest;

Fix the bad format from lint.

• a derived version of D122408 and D121368;
• refine the APIs, getting rid of the GLR parser, and providing fine-grained pieces to allow writing tests easier;
• add unittests for the algorithm and a simple smoke lit test;
• when we fail to parse the input, we return an opaque forest node rather than a nullptr;
• rebase to the main branch;

Fix the broken test.

In D124384#3471917, @sammccall wrote:

This is a reasonable immediate fix, but creating null-typed expressions that live *outside* the recoveryexpr seems scary, and likely to violate expected invariants elsewhere too...

address comments