<div dir="ltr">Hi!<div>Thanks for the quick reply. I'm attaching a new patch which addresses the comments.</div><div><br></div><div>I have created a new function CodeGenModule::setAliasAttributes() which can be invoked by the alias creation functions instead of SetCommonAttributes. The new function includes a call to SetCommonAttributes just like its already existing sister setNonAliasAttributes() does.</div><div><br></div><div>With this patch, the test does cover all the code for the dllexport transferral, and the only thing it does not cover is whether TryEmitDefinitionAsAlias calls the new setAliasAttributes(). The MSVC ABI seems to be generating only one constructor though (at least in the simple test), so the problem with aliasing is hidden and I can't really cover that line of code.</div><div><br></div><div>Cheers,</div><div>    Dario Domizioli</div><div>    SN Systems - Sony Computer Entertainment Group</div><div><br></div><div><br></div><div><br></div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On 16 September 2014 19:09, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>At a high-level, can you factor this out into something like CodeGenModule::setAliasAttributes?</div><div><br></div><div>+// RUN: %clang -target x86_64-windows-gnu %s -S -emit-llvm -o - | FileCheck %s</div><div><br></div><div>Don't use the clang driver (%clang) in codegen tests. Use %clang_cc1. This means you will need to pass -mconstructor-aliases.</div></div><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Tue, Sep 16, 2014 at 10:42 AM, Dario Domizioli <span dir="ltr"><<a href="mailto:dario.domizioli@gmail.com" target="_blank">dario.domizioli@gmail.com</a>></span> wrote:<br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"><div dir="ltr"><div>Hello Clang,</div><div><br></div><div>I have noticed that when the compiler emits aliases for simple constructors and destructors it does not transfer the 'dllexport' storage class attribute to the alias.</div><div>Because in the source code there is only "the constructor" and the user cannot differentiate between the C1 and C2 portions of it, if one portion is dllexport'ed then the other must be too. At the moment however C1 is aliased to C2, and the alias does not have 'dllexport'. This can cause link errors. The same is true for the destructor (and its D1 and D2 portions).</div><div><br></div><div>The attached patch fixed the issue, although I am not 100% sure that my change is in the right place.<br></div><div>Rafael's refactor in r217847 means that the same code has to exist in two places: TryEmitDefinitionAsAlias() in CGCXX.cpp and emitConstructorDestructorAlias() in ItaniumCXXABI.cpp.</div><div>Maybe there's a better place where to put the change...</div><div><br></div><div>Also, my test assumes that C1 is aliased to C2 (and D1 to D2), and I have checked it's true with the provided triple which exercises the Itanium ABI code, but I cannot seem to get the "x86_64-windows-msvc" triple to create aliases for C1 and D1, so I don't have coverage for the code in TryEmitDefinitionAsAlias.</div><div>Can anybody provide advice on that? Or should I just fix the issue in the Itanium ABI and worry about the msvc ABI later? It would seem a shame not to fix both cases...</div><div><br></div><div>Cheers,</div><div>    Dario Domizioli</div><div>    SN Systems - Sony Computer Entertainment Group</div><div><br></div><div><br></div></div>
<br></div></div>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>
</blockquote></div><br></div>