[PATCH] D24469: [clang-cl] Diagnose duplicate uuids.

Nico Weber via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 13 09:01:29 PDT 2016


thakis marked 2 inline comments as done.
thakis added a comment.

In https://reviews.llvm.org/D24469#540400, @majnemer wrote:

> Does __uuidof walk bases to find [uuid] in cl.exe?  They walk bases for the __declspec spelling.


As far as I can tell, they don't for either:

C:\src\chrome\src>type atltest.cc
#include <guiddef.h>

class __declspec(uuid("86759049-8B8E-47F4-81F1-AE07D3F876C8")) Foo {};
class Bar : public Foo {};

[uuid("06759049-8B8E-47F4-81F1-AE07D3F876C7")] class Foo2 {};
class Bar2 : public Foo2 {};

void f() {

  __uuidof(Bar);
  __uuidof(Bar2);

}

C:\src\chrome\src>cl /c atltest.cc /nologo
atltest.cc
atltest.cc(6): warning C4467: usage of ATL attributes is deprecated
atltest.cc(10): error C2787: 'Bar': no GUID has been associated with this object
atltest.cc(11): error C2787: 'Bar2': no GUID has been associated with this object


================
Comment at: lib/Parse/ParseDecl.cpp:1476
@@ +1475,3 @@
+  // Find end of type attributes Attrs and add NewTypeAttributes in the same
+  // order they were in originally.  (Remember, in AttributeList things earlier
+  // in source order are later in the list, since new attributes are added to
----------------
aaron.ballman wrote:
> Remove double space here.
`ack '\.  ' lib`

================
Comment at: lib/Sema/SemaDeclAttr.cpp:4609
@@ +4608,3 @@
+                              unsigned AttrSpellingListIndex, StringRef Uuid) {
+  if (UuidAttr *UA = D->getAttr<UuidAttr>()) {
+    if (UA->getGuid() == Uuid)
----------------
aaron.ballman wrote:
> Can use `const auto *` here.
> 
> Also, don't you need to iterate over all of the `UuidAttr` objects attached to the declaration to see if any of them match, rather than just the first?
>From what I understand, every time a new declaration is created, it copies attributes from the previous decl (see mergeDeclAttribute() in SemaDecl.cpp), so checking the latest should be enough. I added a test case for this.


https://reviews.llvm.org/D24469





More information about the cfe-commits mailing list