<div dir="ltr">On Tue, Aug 13, 2013 at 5:35 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="im">On Tue, Aug 13, 2013 at 1:59 PM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br>
</div><div class="gmail_extra"><div class="gmail_quote"><div class="im">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><br>
<br>
================<br>
Comment at: test/CodeGenCXX/microsoft-uuidof.cpp:8<br>
@@ +7,3 @@<br>
+#ifdef WRONG_GUID<br>
+    unsigned int SomethingWentWrong[4];<br>
+#else<br>
----------------<br>
</div><div>Reid Kleckner wrote:<br>
> I was expecting a diagnostic.  :)<br>
><br>
> Previously clang would reject this code:<br>
> struct __declspec(uuid("12345678-1234-1234-1234-1234567890aB")) S { };<br>
> typedef struct _GUID { int x; int y; } GUID;<br>
> GUID g = __uuidof(S);<br>
><br>
> cl.exe accepts and only copies the first 8 bytes, but I'd like to preserve the original clang behavior.  Can we at least check that the sizeof() the user's type is 16?<br>
</div>Are we going into the business of making sure system-types are well formed? It isn't really a user type, it starts with an underscore and a capital letter... We don't make sure the std::type_info type is proper.<br>


<br>
That said, consider it done if you guys feel strongly about it.</blockquote><div><br></div></div><div>I'll let Reid make the call here: I don't think it matters much, since _GUID is a system type, and if someone defines it themselves and gets it wrong, they'll get broken (but technically correct) code rather than an ICE.</div>
</div></div></div></blockquote><div><br></div><div>OK, how about a test to ensure that the memcpy doesn't overrun the type?  That's what cl does at least.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div class="im"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div>
================<br>
Comment at: lib/CodeGen/CodeGenModule.cpp:1063<br>
@@ -1074,3 +1062,3 @@<br>
       /*isConstant=*/true, llvm::GlobalValue::ExternalLinkage, Init, Name);<br>
   GV->setUnnamedAddr(true);<br>
   return GV;<br>
----------------<br>
</div><div>Richard Smith wrote:<br>
> Is unnamed_addr really correct for these things?<br>
</div>That was there when I got here. ;)<br>
<br>
LangRef makes it sound like appropriate for this case:<br>
> Global variables can be marked with unnamed_addr which indicates that the address is not significant, only the content. Constants marked like this can be merged with other constants if they have the same initializer. Note that a constant with significant address can be merged with a unnamed_addr constant, the result being a constant whose address is significant."<br>


<br>
What was your thinking?</blockquote><div><br></div></div><div>We expose the address of the __s_GUID object to user code, so it doesn't seem like it should be unnamed_addr. For instance:<br></div><div><br></div><div>const char my_stuff[16] = { the GUID };</div>

<div>assert(my_stuff != &__uuidof(type));</div><div><br></div><div>That assert should presumably not fire.</div></div></div></div>
</blockquote></div><br></div><div class="gmail_extra">I agree.  At least it's more conservative.</div></div>