This is an archive of the discontinued LLVM Phabricator instance.

[pseudo] Generate an enum type for identifying grammar rules.
ClosedPublic

Authored by hokein on Jul 8 2022, 3:26 AM.

Details

Summary

The Rule enum type enables us to identify a grammar rule within C++'s
type system.

Diff Detail

Event Timeline

hokein created this revision.Jul 8 2022, 3:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 8 2022, 3:26 AM
hokein requested review of this revision.Jul 8 2022, 3:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 8 2022, 3:26 AM
sammccall accepted this revision.Jul 8 2022, 5:54 AM
sammccall added inline comments.
clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
172

I hope you mean kw_int? I'd like this to compile :-)

Can you also give the example for IDENTIFIER?

clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
48

I'm not sure exposing these from Grammar is an improvement, i think even in principle we'd only want to use these for codegen.

The need for testing is real, but i guess you can add a test that uses the CXX generated grammar, assert the elements of Rule::whatever, and the name of Symbol::whatever?

This revision is now accepted and ready to land.Jul 8 2022, 5:54 AM
hokein updated this revision to Diff 444956.Jul 15 2022, 6:05 AM
hokein marked 2 inline comments as done.

update.

clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
172

As discussed offline, we stick to the current version without kw_ prefix. Added some comments for this decision.

clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
48

Yeah, these functions are only used in the codegen. I'm inlined to the put it into the Grammar(it also has a symbolName definition, I'd prefer to have all naming functions in a single place).

This revision was landed with ongoing or failed builds.Jul 15 2022, 6:11 AM
This revision was automatically updated to reflect the committed changes.
sammccall added inline comments.Jul 15 2022, 7:07 AM
clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
48

That's exactly my point though: that symbolname is general purpose and should be used everywhere, which is why it belongs on grammar.

Vs the mangled name that literally nobody should be using for anything.