This is an archive of the discontinued LLVM Phabricator instance.

[NFCI] Refactor FormatterContainerPair into TieredFormatterContainer.
ClosedPublic

Authored by jgorbe on Sep 14 2022, 6:08 PM.

Details

Summary

FormatterContainerPair is (as its name indicates) a very thin wrapper
over two formatter containers, one for exact matches and another one for
regex matches. The logic to decide which subcontainer to access is
replicated everywhere FormatterContainerPairs are used.

So, for example, when we look for a formatter there's some adhoc code
that does a lookup in the exact match formatter container, and if it
fails it does a lookup in the regex match formatter container. The same
logic is then copied and pasted for summaries, filters, and synthetic
child providers.

This change introduces a new TieredFormatterContainer that has two
main characteristics:

  • It generalizes FormatterContainerPair from 2 to any number of subcontainers, that are looked up in priority order.
  • It centralizes all the logic to choose which subcontainer to use for lookups, add/delete, and indexing.

This allows us to have a single copy of the same logic, templatized for
each kind of formatter. It also simplifies the upcoming addition of a
new tier of callback-based matches. See
https://discourse.llvm.org/t/rfc-python-callback-for-data-formatters-type-matching/64204
for more details about this.

The rest of the change is mostly replacing copy-pasted code with calls
to methods of the relevant TieredFormatterContainer, and adding some
methods to the TypeCategoryImpl class so we can remove some of this
copy-pasted code from SBTypeCategory.

Diff Detail

Event Timeline

jgorbe created this revision.Sep 14 2022, 6:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 14 2022, 6:08 PM
jgorbe requested review of this revision.Sep 14 2022, 6:08 PM
labath added inline comments.Sep 20 2022, 12:48 AM
lldb/include/lldb/DataFormatters/TypeCategory.h
75
88

Is this reachable from the SB API (perhaps via SBTypeCategory::GetXXXAtIndex). If so, we'd want to do something different here (return a default/empty value), or add a check somewhere in the SB layer.

130

same question.

jgorbe updated this revision to Diff 461775.Sep 20 2022, 6:31 PM

Address review feedback:

  • Removed asserts on GetXXXAtIndex methods when the index is out of range. Return a default-initialized shared_ptr of the right type instead (which will result in an invalid value when returned back to the SB API).
  • Removed unnecessary braces to follow the style guide.
jgorbe marked 3 inline comments as done.Sep 20 2022, 6:32 PM

Thanks for the review! Please take another look.

lldb/include/lldb/DataFormatters/TypeCategory.h
75

Removed them (and also another similar set of braces in the constructor). Thanks!

88

Yes, it's reachable. Changed it to a default-initialized return value (which is a null shared_ptr).

130

same answer :)

labath accepted this revision.Sep 27 2022, 11:16 AM
This revision is now accepted and ready to land.Sep 27 2022, 11:16 AM
This revision was landed with ongoing or failed builds.Sep 27 2022, 2:29 PM
This revision was automatically updated to reflect the committed changes.
jgorbe marked 3 inline comments as done.