On Tue, Aug 13, 2013 at 5:35 PM, Richard Smith <[email protected]>wrote:
> On Tue, Aug 13, 2013 at 1:59 PM, David Majnemer > <[email protected]>wrote: > >> >> >> ================ >> Comment at: test/CodeGenCXX/microsoft-uuidof.cpp:8 >> @@ +7,3 @@ >> +#ifdef WRONG_GUID >> + unsigned int SomethingWentWrong[4]; >> +#else >> ---------------- >> Reid Kleckner wrote: >> > I was expecting a diagnostic. :) >> > >> > Previously clang would reject this code: >> > struct __declspec(uuid("12345678-1234-1234-1234-1234567890aB")) S { }; >> > typedef struct _GUID { int x; int y; } GUID; >> > GUID g = __uuidof(S); >> > >> > 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? >> 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. >> >> That said, consider it done if you guys feel strongly about it. > > > 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. > OK, how about a test to ensure that the memcpy doesn't overrun the type? That's what cl does at least. > ================ >> Comment at: lib/CodeGen/CodeGenModule.cpp:1063 >> @@ -1074,3 +1062,3 @@ >> /*isConstant=*/true, llvm::GlobalValue::ExternalLinkage, Init, >> Name); >> GV->setUnnamedAddr(true); >> return GV; >> ---------------- >> Richard Smith wrote: >> > Is unnamed_addr really correct for these things? >> That was there when I got here. ;) >> >> LangRef makes it sound like appropriate for this case: >> > 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." >> >> What was your thinking? > > > 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: > > const char my_stuff[16] = { the GUID }; > assert(my_stuff != &__uuidof(type)); > > That assert should presumably not fire. > I agree. At least it's more conservative.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
