Page MenuHomePhabricator

[LLDB][Clang] add AccessSpecDecl for methods and fields in RecordType
ClosedPublic

Authored by zequanwu on Dec 3 2021, 11:50 AM.

Details

Summary

This allows access type be printed when running lldb-test -dump-ast and
lldb-test -dump-clang-ast.

Diff Detail

Event Timeline

zequanwu requested review of this revision.Dec 3 2021, 11:50 AM
zequanwu created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptDec 3 2021, 11:50 AM

I would like to see more extensive test cases, for example alternating public and private specifiers for both class and struct case.

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
1356

This feels a bit expensive, we have to go through all the fields every time we add a field.

zequanwu updated this revision to Diff 391734.Dec 3 2021, 2:05 PM

Add tests for alternating access.

zequanwu updated this revision to Diff 391740.Dec 3 2021, 2:26 PM

Use a map to keep CXXRecordDecl to last added AccessSpecifier.

zequanwu marked an inline comment as done.Dec 3 2021, 2:27 PM
zequanwu added inline comments.
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
1356

Use a map now.

labath added a comment.Dec 6 2021, 2:24 AM

I like this feature, but I gotta say that a map feels a bit wasteful as well, given that the last access specifier is only needed for the short duration of time while the class definition is being parsed.

Maybe if you remove the relevant entry when CompleteTagDeclarationDefinition gets called, then we wouldn't at least have an unboundedly growing map on our hands?

zequanwu updated this revision to Diff 392126.Dec 6 2021, 10:48 AM
zequanwu marked an inline comment as done.

Erase the cxx_record_decl entry from map after completing it.

This dropped off my radar, but i think it generally looks good. Shafik, do you have anything to add?

shafik accepted this revision.Tue, Jan 4, 12:36 PM

LGTM

This revision is now accepted and ready to land.Tue, Jan 4, 12:36 PM