This is an archive of the discontinued LLVM Phabricator instance.

[demangler][NFC] Tabularize operator name parsing
ClosedPublic

Authored by urnathan on Feb 10 2022, 11:59 AM.

Details

Summary

We need to parse operator encodings in 3 places -- expressions, names & fold expressions. Currently we have 3 separate pieces to do this, and a FIXME.

The operator name and expression parsing are implemented as handwritten two-character nested switches, the fold expression is a sequence of string comparisons.

This adds a new OperatorInfo class to encode the operator info (encoding, kind, name), and has a table that it can binary search. From that each of the above 3 uses are altered to use the new scheme.

Existing tests cover parsing operator encodings, but as you can see there's a self-check for table ordering.

Diff Detail

Event Timeline

urnathan requested review of this revision.Feb 10 2022, 11:59 AM
urnathan created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2022, 11:59 AM
urnathan updated this revision to Diff 409375.Feb 16 2022, 12:58 PM

Fix CI test fails:

  • Lambda parm shadows outer variable
  • auto return type is C++14
iains added inline comments.Feb 18 2022, 8:11 AM
libcxxabi/src/demangle/ItaniumDemangle.h
2879

I was expecting to see "aw" here - is that handled somewhere else?

urnathan added inline comments.Feb 18 2022, 8:54 AM
libcxxabi/src/demangle/ItaniumDemangle.h
2879

the demangler lacks support for co_await. Maybe file a bug?

urnathan updated this revision to Diff 409960.Feb 18 2022, 9:26 AM

formatting

urnathan updated this revision to Diff 409962.Feb 18 2022, 9:36 AM

fix format

urnathan updated this revision to Diff 411847.Feb 28 2022, 11:00 AM

rebase, ping?

ChuanqiXu accepted this revision.Mar 1 2022, 12:33 AM

The scale of the patch shocked me at the first sight. But after I tried to visit it actually. I find the new structure seems much better. Only style comment remained and feel free to address them.

libcxxabi/src/demangle/ItaniumDemangle.h
2936

Maybe it is possible to use container here:

static constexpr ArrayRef<OperatorInfo> OpArr(Ops);

Then we could use C++ style here.

2941

static bool Done = false;

2944–2945

Could we use std::is_sorted here?

3006

Is this not equal to isdigit() ?

4298

I feel the original if-else is more simpler.

This revision is now accepted and ready to land.Mar 1 2022, 12:33 AM
This revision was landed with ongoing or failed builds.Mar 1 2022, 4:45 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2022, 4:45 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript