<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Jan 3, 2017 at 11:41 AM Teresa Johnson via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">tejohnson added inline comments.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: llvm/trunk/lib/Bitcode/Reader/MetadataLoader.cpp:750<br class="gmail_msg">
+        TemplateParams = getMDOrNull(Record[14]);<br class="gmail_msg">
+      }<br class="gmail_msg">
       DICompositeType *CT = nullptr;<br class="gmail_msg">
----------------<br class="gmail_msg">
aprantl wrote:<br class="gmail_msg">
> 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.<br class="gmail_msg">
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?<br class="gmail_msg"></blockquote><div><br>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)<br><br>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).<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="gmail_msg">
<br class="gmail_msg">
Repository:<br class="gmail_msg">
  rL LLVM<br class="gmail_msg">
<br class="gmail_msg">
<a href="https://reviews.llvm.org/D27775" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D27775</a><br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div></div>