This is an archive of the discontinued LLVM Phabricator instance.

Debug Info: Add a file: field to DIImportedEntity
ClosedPublic

Authored by aprantl on Jul 18 2017, 2:13 PM.

Details

Summary

DIImportedEntity has a line number, but not a file field. To determine the decl_line/decl_file we combine the line number from the DIImportedEntity with the file from the DIImportedEntity's scope. This does not work correctly when the parent scope is a DINamespace or a DIModule, both of which do not have a source file.

This patch adds a file field to DIImportedEntity to unambiguously identify the source location of the using/import declaration.
Most testcase updates are mechanical, the interesting one is the removal of the FIXME in test/DebugInfo/Generic/namespace.ll.

This fixes PR33822. See https://bugs.llvm.org/show_bug.cgi?id=33822 for more context.

Diff Detail

Repository
rL LLVM

Event Timeline

aprantl created this revision.Jul 18 2017, 2:13 PM

Note that this review contains also the corresponding clang changes in the tools/ directory.

aprantl updated this revision to Diff 107180.Jul 18 2017, 2:18 PM

Forgot to git-add new clang test case input files.

probinson edited edge metadata.Jul 18 2017, 3:42 PM

Sweet. I don't have the chops to understand the Objective-C test but otherwise LGTM.

lib/Bitcode/Reader/MetadataLoader.cpp
1684 ↗(On Diff #107180)

That's a nice touch.

aprantl added inline comments.Jul 18 2017, 3:49 PM
lib/Bitcode/Reader/MetadataLoader.cpp
1684 ↗(On Diff #107180)

That was the best "upgrade" I could come up with for reliably dealing with the old format. There is an assertion in DIBuilder that discourages creating new nodes with a location with a line but no file. We could upgrade that to a verifier check if we want to.

This revision was automatically updated to reflect the committed changes.