[PATCH] D32648: Remove line and file from DINamespace

Adrian Prantl via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 28 11:17:11 PDT 2017


aprantl added a comment.

In https://reviews.llvm.org/D32648#740966, @dblaikie wrote:

> I'm all for this, couple of questions though:
>
> - When would the use of "distinct" on an anonymous namespace matter? Since LLVM uses cross-CU references to refer to entities in different CUs, never importing/moving entities between CUs, the anonymous namespaces shouldn't collide, should they? (anymore than named namespaces would collide when there are different entities in different CUs but both in the same named namespace - LLVM still produces two separate namespaces, one in each CU, one with each of the respective entities)?


I believe the `distinct`  is necessary because anonymous namespaces whose parent scope is the CU have a `null` scope. (This is what DIBuilder does to every type node to allow for type uniquing.) If the anonymous namespace are not distinct, all top-level anonymous namespaces would be uniqued. Thus two types from different CU's that are in different anonymous namespaces would both have the same scope pointing to the uniqued anonymous namespace. It is not unthinkable that this could happen to work correctly because of how DwarfDebug is processing the data, but I'd rather not rely on this.

> 
> 
> - Which tests cover the LLVM codegen side of this change - several tests no longer look for the line/file, but do any tests ensure it is not present? (if this patch were reverted, which tests would fail (not a perfect question, since the change in the metadata format would cause parse failures - but not talking about those))

test/DebugInfo/X86/dwarf-public-names.ll
will fail, but not for a good reason: It hardcodes the size of the CU :-)
I'll add CHECK-NOTs to one of the tests!


https://reviews.llvm.org/D32648





More information about the llvm-commits mailing list