Page MenuHomePhabricator

[DebugInfo] Fix to ctor homing to ignore classes with trivial ctors.
ClosedPublic

Authored by akhuang on Jul 29 2020, 9:55 AM.

Details

Summary

Previously ctor homing was omitting debug info for classes if they
have both trival and nontrivial constructors, but we should only omit debug
info if the class doesn't have any trivial constructors.

Also change to use completeUnusedClass so the debug info is added to the
retained types list.

bug: https://bugs.llvm.org/show_bug.cgi?id=46537

Diff Detail

Event Timeline

akhuang created this revision.Jul 29 2020, 9:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 29 2020, 9:55 AM
akhuang requested review of this revision.Jul 29 2020, 9:55 AM

I think it'd be good to separate these two issues/patches.

In part because I'm curious whether, even with a trivial ctor - if we did the retained types thing - would that be enough?

I think it'd be good to separate these two issues/patches.

In part because I'm curious whether, even with a trivial ctor - if we did the retained types thing - would that be enough?

I think just the retained types thing wouldn't help in this case because the trivial ctor is not emitted? I tried it and the struct is still fwd declared. I could split out the retained types change, though.

akhuang updated this revision to Diff 281722.Jul 29 2020, 1:37 PM

remove change to add class types to retained types list,

also fix the test case to be a struct that previously didn't
get complete debug info.

dblaikie accepted this revision.Jul 29 2020, 2:30 PM

remove change to add class types to retained types list,

also fix the test case to be a struct that previously didn't
get complete debug info.

Ah, I see what you mean. Perhaps the implementation can/should be changed to trigger on source uses of the ctor rather than only DWARF uses? Though I suppose that'd have problems with the ctor being used in unused code. (like the "requires complete type" optimization does - the type can be required to be complete in unused code)

Sounds good!

This revision is now accepted and ready to land.Jul 29 2020, 2:30 PM