[PATCH] D27775: [ThinLTO] Import composite types as declarations

[David Blaikie via llvm-commits]

On Tue, Jan 3, 2017 at 11:41 AM Teresa Johnson via Phabricator <
reviews at reviews.llvm.org> wrote:

> tejohnson added inline comments.
>
>
> ================
> Comment at: llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp:750
> +        TemplateParams = getMDOrNull(Record[14]);
> +      }
>        DICompositeType *CT = nullptr;
> ----------------
> aprantl wrote:
> > Sorry for bringing this up so late, but I just realized that this is
> only legal if the `Identifier` field is nonempty. The ODR (or an equivalent
> rule in the underlying source language) only applies to types with an
> identifier field. In C, for example, it is legal for multiple translation
> units to contain definitions of types with the same name, but a different
> layout. Using just the name it is impossible for the debugger to find the
> correct definition of a forward-declared type in the other translation
> units.
> Ok, I can make the decl optimization conditional on the Identifier being
> non-null. I assume that is not the common case anyway - in which case that
> change shouldn't hurt the overall impact much?
>

For C++ any external linkage type will have an identifier - which, yes, is
most of them. (types in anonymous namespaces, or local to static functions,
etc - do not have identifiers/do not have external linkage)

The biggest hole in this I've seen is the use of anonymous enums in headers
to define constants - technically these don't have external linkage, so we
don't merge them... (& technically code using these might be an ODR
violation... but it's really common, so would be great to have a language
fix).


>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D27775
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170103/f323e27c/attachment.html>