Page MenuHomePhabricator

Add an opque payload field to lldb::Type (NFC).
ClosedPublic

Authored by aprantl on Mar 3 2020, 1:16 PM.

Details

Summary

This hides the language-specific flags and lets each language define its own fields in the opaquer storage.

Diff Detail

Event Timeline

xiaobai added a subscriber: xiaobai.Mar 3 2020, 1:26 PM

Thanks for working to make LLDB more language-agnostic!

aprantl updated this revision to Diff 248024.Mar 3 2020, 1:54 PM

Document layout.

I want to see how you end up resolving the comments on payload being a plain integer type in D75626 before looking at this again. Maybe it makes more sense to use a type, the use is pretty clever but perhaps makes for opaque code in some places.

aprantl updated this revision to Diff 250825.Mar 17 2020, 10:35 AM
aprantl updated this revision to Diff 250829.Mar 17 2020, 11:01 AM

Address feedback from Pavel and add the payload to the constructor.

labath added a subscriber: labath.Mar 20 2020, 3:55 AM
labath added inline comments.
lldb/include/lldb/Symbol/Type.h
201–202

It doesn't look like this setter is needed, as the single usage has now been changed to a constructor argument. And if this is immutable, then in turn some methods of TypePayloadClang become unneeded (SetIsCompleteObjCClass)

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
64

maybe make this explicit too (and augment it with a encode method to make the usage less weird)?

aprantl updated this revision to Diff 253224.Mar 27 2020, 2:40 PM

Rebase and update based on https://reviews.llvm.org/D75488. It no longer takes a reference, but is a value type instead.

aprantl updated this revision to Diff 253226.Mar 27 2020, 2:45 PM
aprantl marked an inline comment as done.

Remove the obsolete setter in lldb::Type.

teemperor accepted this revision.Mar 31 2020, 10:37 AM

Could you make the uint32_t a typedef? I'm fine with this not being type-safe, but a typedef would at least allow me to search for where this information is used. Grepping/searching for uint32_t isn't a great experience. It would also make this interface more self-documenting.

Beside that LGTM.

This revision is now accepted and ready to land.Mar 31 2020, 10:37 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2020, 11:28 AM