<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Apr 27, 2016 at 12:46 AM, Aboud, Amjad <span dir="ltr"><<a href="mailto:amjad.aboud@intel.com" target="_blank">amjad.aboud@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">I do not know if this is the only case where this happens, but consider the following:<u></u><u></u></span></p>
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> <u></u></span></b></p>
<p class="MsoNormal" style="margin-left:.5in"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">TestIM.cpp<u></u><u></u></span></b></p><span class="">
<p class="MsoNormal" style="margin-left:.5in"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">#include "TestIM.h"<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:.5in"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">#include "TestIM.h"<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:.5in"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal" style="margin-left:.5in"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">TestIM.h<u></u><u></u></span></b></p>
</span><p class="MsoNormal" style="margin-left:.5in"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">namespace X {<u></u><u></u></span></p><span class="">
<p class="MsoNormal" style="margin-left:.5in"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> typedef int MyINT_t;<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:.5in"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">}<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:.5in"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal" style="margin-left:.5in"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">namespace Y {<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:.5in"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> using X::MyINT_t;<u></u><u></u></span></p>
<p class="MsoNormal" style="margin-left:.5in"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">}<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> <u></u></span></p>
</span><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 3.8.0 (trunk 257325)", isOptimized: false, runtimeVersion: 0, emissionKind:
1, enums: !2, imports: !3)<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">!3 = !{!4, !4}<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">!4 = !DIImportedEntity(tag: DW_TAG_imported_declaration, scope: !5, entity: !7, line: 6)<u></u><u></u></span></p>
<p class="MsoNormal"><a name="m_-3343800049759544007__MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> <u></u></span></a></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Currently we just emit the imported module twice, but once I added my fix to Lexical scope, I had an assumption that “imports” list is unique.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">I could fix that in backend of course, but I think there is no added value to have the duplication in the IR.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Do you agree?</span></p></div></div></blockquote><div><br></div><div>I'm not sure I'm terribly fussed about the duplication here.<br><br>If it shows up a lot or is otherwise really problematic, sure, it may be worth a fix of some kind.<br><br>Ultimately the way using decls are handled is sub-optimal (& I implemented them this way, so I'm not pointing fingers or anything) because even unused ones end up emitted which is kind of a pain for debug info size, readability, etc. At some point we'd want to try to reverse the association (this has been speculated about on the threads regarding the subprogram link reversals, global variable link reversal speculation, etc) so that unused ones go away. That way duplicates wouldn't be much of an issue, they'd just disappear that way.<br><br>But yes, it doesn't seem sufficiently compelling to include these or emit them or require a frontend to deduplicate them as they are distinct entities at that level. I just didn't want to add another level of set deduplication if there was an obvious mechanism to avoid creating redundancy in the first place and it seems there isn't. Thanks for bearing with me.<br><br>- Dave</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Regards,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Amjad<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"><u></u> <u></u></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #e1e1e1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><a name="m_-3343800049759544007______replyseparator"></a><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> David Blaikie [mailto:<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>]
<br>
<b>Sent:</b> Tuesday, April 26, 2016 21:40<br>
<b>To:</b> Aboud, Amjad <<a href="mailto:amjad.aboud@intel.com" target="_blank">amjad.aboud@intel.com</a>><br>
<b>Cc:</b> <a href="mailto:reviews%2BD19468%2Bpublic%2Bd141c7248d054ca3@reviews.llvm.org" target="_blank">reviews+D19468+public+d141c7248d054ca3@reviews.llvm.org</a>; Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>>; Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>>; llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span></p><div><div class="h5"><br>
<b>Subject:</b> Re: [PATCH] D19468: Disallow duplication of imported entities (improved implementation)<u></u><u></u></div></div><p></p>
</div>
</div><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On Tue, Apr 26, 2016 at 1:01 AM, Aboud, Amjad <<a href="mailto:amjad.aboud@intel.com" target="_blank">amjad.aboud@intel.com</a>> wrote:<u></u><u></u></p>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">You cannot simply skip a redeclaration unless it is declared at same location.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">For example:</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">namespace X { int a; }</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">// some other code</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">namespace X { int b; }</span> <u></u><u></u></p>
</div>
</div>
</blockquote>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">You cannot eliminate the second namespace, can you?</span><u></u><u></u></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">It's not the declarations of the namespace that I'm referring to - it's the using declarations<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">void foo () { using X:a; }</span><u></u><u></u></p>
<p class="MsoNormal"><a name="m_-3343800049759544007_m_6718369539073311638_m_2552145902333175"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">void bar () { using X:a; }</span><u></u><u></u></a></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Also here, you cannot eliminate the second using-decl can you?</span><u></u><u></u></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">These wouldn't be redeclarations of each other, I don't think. They represent different decls in each scope (that resolve to the same entity, but are themselves distinct declarations).<br>
<br>
But no, it seems like two identical using declarations in the same scope aren't considered to be redeclarations of each other, so my suggestion doesn't apply in any case.<br>
<br>
Could you remind me why this comes up? If the user wrote two of the same using decls, it doesn't seem like the worst thing if we produce equally redundant output. If I recall correctly, we had some correctness problem/some case where we were emitting two using
decls even though the user only wrote one?<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Looking into the code, it is not simple to introduce these checks, as there is no one place that will
affect all Decls, and we will need to handle each Decl separately.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Can you think on a place where we can add this in AST?</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Thanks,</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d">Amjad</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1f497d"> </span><u></u><u></u></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #e1e1e1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> David Blaikie [mailto:<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>]
<br>
<b>Sent:</b> Tuesday, April 26, 2016 03:12<br>
<b>To:</b> <a href="mailto:reviews%2BD19468%2Bpublic%2Bd141c7248d054ca3@reviews.llvm.org" target="_blank">
reviews+D19468+public+d141c7248d054ca3@reviews.llvm.org</a>; Aboud, Amjad <<a href="mailto:amjad.aboud@intel.com" target="_blank">amjad.aboud@intel.com</a>><br>
<b>Cc:</b> Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>>; Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>>; llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>><br>
<b>Subject:</b> Re: [PATCH] D19468: Disallow duplication of imported entities (improved implementation)</span><u></u><u></u></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<p class="MsoNormal">On Mon, Apr 25, 2016 at 2:25 PM, Amjad Aboud via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<u></u><u></u></p>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal">aaboud added a comment.<br>
<br>
I am not much familiar with the AST part, but I assume there is reuse of previous generated DCL, even if it has same data.<u></u><u></u></p>
</blockquote>
<div>
<p class="MsoNormal"><br>
What if we filter by cases where the decl == the canonical decl, and skip any decl that's a redeclaration?<br>
<u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal"><br>
Here is a small example that leads into duplication in AST DCLs.<br>
You may run the following command line:<br>
<br>
clang -cc1 -ast-dump -o - -O0 TestIM.cpp<br>
<br>
TestIM.cpp<br>
----------<br>
<br>
#include "TestIM.h"<br>
#include "TestIM.h"<br>
<br>
<br>
<br>
TestIM.h<br>
--------<br>
<br>
namespace X {<br>
typedef int MyINT_t;<br>
}<br>
<br>
namespace Y {<br>
using X::MyINT_t;<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"> }<br>
<br>
<br>
<a href="http://reviews.llvm.org/D19468" target="_blank">http://reviews.llvm.org/D19468</a><br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><u></u><u></u></p>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
</div>
</div>
</div>
</div>
</div>
<p>---------------------------------------------------------------------<br>
Intel Israel (74) Limited<u></u><u></u></p>
<p>This e-mail and any attachments may contain confidential material for<br>
the sole use of the intended recipient(s). Any review or distribution<br>
by others is strictly prohibited. If you are not the intended<br>
recipient, please contact the sender and delete all copies.<u></u><u></u></p>
</div>
</blockquote>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div></div></div>
</div><div><div class="h5">
<p>---------------------------------------------------------------------<br>
Intel Israel (74) Limited</p>
<p>This e-mail and any attachments may contain confidential material for<br>
the sole use of the intended recipient(s). Any review or distribution<br>
by others is strictly prohibited. If you are not the intended<br>
recipient, please contact the sender and delete all copies.</p></div></div></div>
</blockquote></div><br></div></div>