This is an archive of the discontinued LLVM Phabricator instance.

Revert "[DwarfDebug] Support emitting function-local declaration for a lexical block" & dependent patches
ClosedPublic

Authored by krisb on Dec 23 2021, 10:02 AM.

Details

Summary

Try to revert D113741 once again.

This also reverts 0ac75e82fff93a80ca401d3db3541e8d1d9098f9 (D114705)
as it causes LLDB's lldb-api.lang/cpp/nsimport.TestCppNsImport.py test
failure w/o D113741.

This reverts commit f9607d45f399e2afc39ec16222ea68b4e0831564.

Diff Detail

Event Timeline

krisb created this revision.Dec 23 2021, 10:02 AM
krisb requested review of this revision.Dec 23 2021, 10:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 23 2021, 10:02 AM

Thanks for giving this a go. Apparently there were some lingering failures from that precursor patch ( 0ac75e82fff93a80ca401d3db3541e8d1d9098f9 ) you're reverting here as well, so that's good. I'm happy to try applying/testing the next reapplication attempt once you/we've worked out the kinks.

Do you think this is ready to commit? I do see some auto-test failures, but if I'm reading them right, they're that generic failure about the go bindings I see all the time (at least locally - I guess I don't have some go pieces installed) and probably not related to this patch?

krisb added a comment.Dec 23 2021, 2:46 PM

Thanks for giving this a go. Apparently there were some lingering failures from that precursor patch ( 0ac75e82fff93a80ca401d3db3541e8d1d9098f9 ) you're reverting here as well, so that's good. I'm happy to try applying/testing the next reapplication attempt once you/we've worked out the kinks.

Do you think this is ready to commit? I do see some auto-test failures, but if I'm reading them right, they're that generic failure about the go bindings I see all the time (at least locally - I guess I don't have some go pieces installed) and probably not related to this patch?

I have local lldb build & tests just finished without issues, so, yes, this should be ready for land.

This revision was not accepted when it landed; it landed in state Needs Review.Dec 23 2021, 2:47 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.

Thanks for giving this a go. Apparently there were some lingering failures from that precursor patch ( 0ac75e82fff93a80ca401d3db3541e8d1d9098f9 ) you're reverting here as well, so that's good. I'm happy to try applying/testing the next reapplication attempt once you/we've worked out the kinks.

Do you think this is ready to commit? I do see some auto-test failures, but if I'm reading them right, they're that generic failure about the go bindings I see all the time (at least locally - I guess I don't have some go pieces installed) and probably not related to this patch?

I have local lldb build & tests just finished without issues, so, yes, this should be ready for land.

Thanks! (sorry about missing the formal approval earlier)

ellis added a subscriber: ellis.Jan 28 2022, 9:21 AM

Hi @krisb. Are their plans to give this fix another go? Do we need to first wait for D113741 to land again?

krisb added a comment.Feb 2 2022, 6:52 AM

Hi @krisb. Are their plans to give this fix another go? Do we need to first wait for D113741 to land again?

Yes, but the issues reported by David in D113741 should be fixed first. I still don't have a good solution for this puzzle.

llvm/test/DebugInfo/NVPTX/debug-addr-class.ll