This is an archive of the discontinued LLVM Phabricator instance.

[lldb][NFCI] Refactor TypeSystemClang::GetBasicTypeEnumeration
ClosedPublic

Authored by bulbazord on Jun 6 2023, 3:42 PM.

Details

Summary

TypeSystemClang::GetBasicTypeEnumeration(ConstString) builds up a
giant UniqueCStringMap<lldb::BasicType> which is effectively a vector whose elements
are (ConstString, lldb::BasicType). Once sorted, we get to have fairly
quick lookups (O(log n) on average). This is fine, but we can do better
on average with an llvm::StringMap.

This also allows us to simplify the initialization code for the lookup,
no more once_flag!

Additionally, I removed unused declarations of related functions from
CompilerType and TypeSystemClang

Diff Detail

Event Timeline

bulbazord created this revision.Jun 6 2023, 3:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2023, 3:42 PM
bulbazord requested review of this revision.Jun 6 2023, 3:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2023, 3:42 PM
aprantl accepted this revision.Jun 6 2023, 3:48 PM
aprantl added inline comments.
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
868

Isn't this redundant?

This revision is now accepted and ready to land.Jun 6 2023, 3:48 PM
bulbazord added inline comments.Jun 6 2023, 3:51 PM
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
868

Technically yes. My reasoning is that it's faster to see if a StringRef is empty than to perform a lookup in a hash map.

aprantl added inline comments.Jun 6 2023, 4:17 PM
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
868

True, but this adds extra code to the binary that in practice will (presumably?) never get executed. Or are we often looking up empty types? (It's not a big deal either way, but since we are being pedantic :-)

bulbazord updated this revision to Diff 529081.Jun 6 2023, 4:25 PM

Minor pedantry

bulbazord marked 2 inline comments as done.Jun 6 2023, 4:25 PM
fdeazeve accepted this revision.Jun 8 2023, 3:36 AM

LGTM!