This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Check Decl kind when completing -flimit-debug-info types
ClosedPublic

Authored by labath on Aug 13 2020, 6:11 AM.

Details

Summary

The search for the complete class definition can also produce entries
which are not of the expected type. This can happen for instance when
there is a function with the same name as the class we're looking up
(which means that the class needs to be disambiguated with the
struct/class tag in most contexts).

Previously we were just picking the first Decl that the lookup returned,
which later caused crashes or assertion failures if it was not of the
correct type. This patch changes that to search for an entry of the
correct type.

Diff Detail

Event Timeline

labath created this revision.Aug 13 2020, 6:11 AM
Herald added a project: Restricted Project. · View Herald Transcript
labath requested review of this revision.Aug 13 2020, 6:11 AM
teemperor accepted this revision.Aug 13 2020, 7:08 AM

Is it too late to claim that I did this on purpose to see if anyone noticed the bug? It probably is :/

lldb/test/API/functionalities/limit-debug-info/main.cpp
31

Nit: the public isn't necessary and you also omitted above.

32

I'm probably missing something obvious here, but why give every class in the test a defaulted constexpr constructor?

This revision is now accepted and ready to land.Aug 13 2020, 7:08 AM
shafik accepted this revision.Aug 13 2020, 2:07 PM

Minor comment but LGTM, nice fix!

lldb/test/API/functionalities/limit-debug-info/main.cpp
34

Just to clarify we func_shadow::One is being shadowed here. Maybe a comment would help clarify the what is going on.

labath marked 2 inline comments as done.Aug 14 2020, 3:29 AM

Is it too late to claim that I did this on purpose to see if anyone noticed the bug? It probably is :/

:P

lldb/test/API/functionalities/limit-debug-info/main.cpp
32

I was trying to guarantee that these variables get constant-initialized because earlier versions of this test just inspected a static binary without running it. But then one of the cases actually required running code so it no longer serves any purpose. I'll just delete them...

This revision was landed with ongoing or failed builds.Aug 14 2020, 3:32 AM
This revision was automatically updated to reflect the committed changes.