thakis created this revision.
thakis added a reviewer: rnk.
thakis added subscribers: cfe-commits, aaron.ballman.
This mostly behaves cl.exe's behavior, even though clang-cl is stricter in some
corner cases and more lenient in others (see the included test).
To make the `uuid declared previously here` diagnostic work correctly, tweak
stripTypeAttributesOffDeclSpec() to keep attributes in the right order.
https://reviews.llvm.org/D24469
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/AttributeList.h
include/clang/Sema/Sema.h
lib/Parse/ParseDecl.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclAttr.cpp
test/SemaCXX/ms-uuid.cpp
Index: test/SemaCXX/ms-uuid.cpp
===================================================================
--- test/SemaCXX/ms-uuid.cpp
+++ test/SemaCXX/ms-uuid.cpp
@@ -0,0 +1,92 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions %s
+
+typedef struct _GUID {
+ unsigned long Data1;
+ unsigned short Data2;
+ unsigned short Data3;
+ unsigned char Data4[8];
+} GUID;
+
+namespace {
+
+// cl.exe's behavior with merging uuid attributes is a bit erratic:
+// * In []-style attributes, a single [] list must not list a duplicate uuid
+// (even if it's the same uuid), and only a single declaration of a class
+// must have a uuid else the compiler errors out (even if two declarations of
+// a class have the same uuid).
+// * For __declspec(uuid(...)), it's ok if several declarations of a class have
+// an uuid, as long as it's the same uuid each time. If uuids on declarations
+// don't match, the compiler errors out.
+// * If there are several __declspec(uuid(...))s on one declaration, the
+// compiler only warns about this and uses the last uuid. It even warns if
+// the uuids are the same.
+
+// clang-cl implements the following simpler (but largely compatible) behavior
+// instead:
+// * [] and __declspec uuids have the same behavior.
+// * If there are several uuids on a a class (no matter if on the same decl or
+// on several decls), it is an error if they don't match.
+// * Having several uuids that match is ok.
+
+// Both cl and clang-cl accept this:
+class __declspec(uuid("000000A0-0000-0000-C000-000000000049")) C1;
+class __declspec(uuid("000000A0-0000-0000-C000-000000000049")) C1;
+class __declspec(uuid("000000A0-0000-0000-C000-000000000049")) C1 {};
+
+// Both cl and clang-cl error out on this:
+// expected-note@+1 {{previous uuid specified here}}
+class __declspec(uuid("000000A0-0000-0000-C000-000000000049")) C2;
+// expected-error@+1 {{uiid does not match previous declaration}} expected-note@+1 {{previous uuid specified here}}
+class __declspec(uuid("110000A0-0000-0000-C000-000000000049")) C2;
+// expected-error@+1 {{uiid does not match previous declaration}}
+class __declspec(uuid("220000A0-0000-0000-C000-000000000049")) C2 {};
+
+// clang-cl accepts this, but cl errors out:
+[uuid("000000A0-0000-0000-C000-000000000049")] class C3;
+[uuid("000000A0-0000-0000-C000-000000000049")] class C3;
+[uuid("000000A0-0000-0000-C000-000000000049")] class C3 {};
+
+// Both cl and clang-cl error out on this (but for different reasons):
+// expected-note@+1 {{previous uuid specified here}}
+[uuid("000000A0-0000-0000-C000-000000000049")] class C4;
+// expected-error@+1 {{uiid does not match previous declaration}} expected-note@+1 {{previous uuid specified here}}
+[uuid("110000A0-0000-0000-C000-000000000049")] class C4;
+// expected-error@+1 {{uiid does not match previous declaration}}
+[uuid("220000A0-0000-0000-C000-000000000049")] class C4 {};
+
+// Both cl and clang-cl error out on this:
+// expected-note@+1 {{previous uuid specified here}}
+class __declspec(uuid("000000A0-0000-0000-C000-000000000049"))
+// expected-error@+1 {{uiid does not match previous declaration}}
+ __declspec(uuid("110000A0-0000-0000-C000-000000000049")) C5;
+
+// XXX diag is the wrong way round here
+// expected-note@+1 {{previous uuid specified here}}
+[uuid("000000A0-0000-0000-C000-000000000049"),
+// expected-error@+1 {{uiid does not match previous declaration}}
+ uuid("110000A0-0000-0000-C000-000000000049")] class C6;
+
+// cl doesn't diagnose having one uuid each as []-style attributes and as
+// __declspec, even if the uuids differ. clang-cl errors if they differ.
+[uuid("000000A0-0000-0000-C000-000000000049")]
+class __declspec(uuid("000000A0-0000-0000-C000-000000000049")) C7;
+
+// XXX diag order is kind of weird here
+// expected-note@+1 {{previous uuid specified here}}
+[uuid("000000A0-0000-0000-C000-000000000049")]
+// expected-error@+1 {{uiid does not match previous declaration}}
+class __declspec(uuid("110000A0-0000-0000-C000-000000000049")) C8;
+
+
+// cl warns on this, but clang-cl is fine with it (which is consistent with
+// e.g. specifying __multiple_inheritance several times, which cl accepts
+// without warning too).
+class __declspec(uuid("000000A0-0000-0000-C000-000000000049"))
+ __declspec(uuid("000000A0-0000-0000-C000-000000000049")) C9;
+
+// cl errors out on this, but clang-cl is fine with it (to be consistent with
+// the previous case).
+[uuid("000000A0-0000-0000-C000-000000000049"),
+ uuid("000000A0-0000-0000-C000-000000000049")] class C10;
+
+}
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -4604,6 +4604,19 @@
// Microsoft specific attribute handlers.
//===----------------------------------------------------------------------===//
+UuidAttr *Sema::mergeUuidAttr(Decl *D, SourceRange Range,
+ unsigned AttrSpellingListIndex, StringRef Uuid) {
+ if (UuidAttr *UA = D->getAttr<UuidAttr>()) {
+ if (UA->getGuid() == Uuid)
+ return nullptr;
+ Diag(UA->getLocation(), diag::err_mismatched_uuid);
+ Diag(Range.getBegin(), diag::note_previous_uuid);
+ return nullptr;
+ }
+
+ return ::new (Context) UuidAttr(Range, Context, Uuid, AttrSpellingListIndex);
+}
+
static void handleUuidAttr(Sema &S, Decl *D, const AttributeList &Attr) {
if (!S.LangOpts.CPlusPlus) {
S.Diag(Attr.getLoc(), diag::err_attribute_not_supported_in_lang)
@@ -4645,8 +4658,10 @@
}
}
- D->addAttr(::new (S.Context) UuidAttr(Attr.getRange(), S.Context, StrRef,
- Attr.getAttributeSpellingListIndex()));
+ UuidAttr *UA = S.mergeUuidAttr(D, Attr.getRange(),
+ Attr.getAttributeSpellingListIndex(), StrRef);
+ if (UA)
+ D->addAttr(UA);
}
static void handleMSInheritanceAttr(Sema &S, Decl *D, const AttributeList &Attr) {
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -2246,6 +2246,13 @@
static bool mergeDeclAttribute(Sema &S, NamedDecl *D,
const InheritableAttr *Attr,
Sema::AvailabilityMergeKind AMK) {
+ // This function copies an attribute Attr from a previous declaration to the
+ // new declaration D if the new declaration doesn't itself have that attribute
+ // yet or if that attribute allows duplicates.
+ // If you're adding a new attribute that requires logic different from
+ // "use explicit attribute on decl if present, else use attribute from
+ // previous decl", for example if the attribute needs to be consistent
+ // between redeclarations, you need to call a custom merge function here.
InheritableAttr *NewAttr = nullptr;
unsigned AttrSpellingListIndex = Attr->getSpellingListIndex();
if (const auto *AA = dyn_cast<AvailabilityAttr>(Attr))
@@ -2304,6 +2311,9 @@
(AMK == Sema::AMK_Override ||
AMK == Sema::AMK_ProtocolImplementation))
NewAttr = nullptr;
+ else if (const auto *UA = dyn_cast<UuidAttr>(Attr))
+ NewAttr = S.mergeUuidAttr(D, UA->getRange(), AttrSpellingListIndex,
+ UA->getGuid());
else if (Attr->duplicatesAllowed() || !DeclHasAttr(D, Attr))
NewAttr = cast<InheritableAttr>(Attr->clone(S.Context));
Index: lib/Parse/ParseDecl.cpp
===================================================================
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -1439,15 +1439,21 @@
ParsedAttributes &PA = DS.getAttributes();
AttributeList *AL = PA.getList();
AttributeList *Prev = nullptr;
+ AttributeList *TypeAttrHead = nullptr;
+ AttributeList *TypeAttrTail = nullptr;
while (AL) {
AttributeList *Next = AL->getNext();
if ((AL->getKind() == AttributeList::AT_Aligned &&
AL->isDeclspecAttribute()) ||
AL->isMicrosoftAttribute()) {
// Stitch the attribute into the tag's attribute list.
- AL->setNext(nullptr);
- Attrs.add(AL);
+ if (TypeAttrTail)
+ TypeAttrTail->setNext(AL);
+ else
+ TypeAttrHead = AL;
+ TypeAttrTail = AL;
+ TypeAttrTail->setNext(nullptr);
// Remove the attribute from the variable's attribute list.
if (Prev) {
@@ -1465,6 +1471,12 @@
AL = Next;
}
+
+ // 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
+ // the front of the list.)
+ Attrs.addAllAtEnd(TypeAttrHead);
}
/// ParseDeclaration - Parse a full 'declaration', which consists of
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -2203,6 +2203,8 @@
VisibilityAttr *mergeVisibilityAttr(Decl *D, SourceRange Range,
VisibilityAttr::VisibilityType Vis,
unsigned AttrSpellingListIndex);
+ UuidAttr *mergeUuidAttr(Decl *D, SourceRange Range,
+ unsigned AttrSpellingListIndex, StringRef Uuid);
DLLImportAttr *mergeDLLImportAttr(Decl *D, SourceRange Range,
unsigned AttrSpellingListIndex);
DLLExportAttr *mergeDLLExportAttr(Decl *D, SourceRange Range,
Index: include/clang/Sema/AttributeList.h
===================================================================
--- include/clang/Sema/AttributeList.h
+++ include/clang/Sema/AttributeList.h
@@ -748,6 +748,19 @@
list = newList;
}
+ void addAllAtEnd(AttributeList *newList) {
+ if (!list) {
+ list = newList;
+ return;
+ }
+
+ AttributeList *lastInList = list;
+ while (AttributeList *next = lastInList->getNext())
+ lastInList = next;
+
+ lastInList->setNext(newList);
+ }
+
void set(AttributeList *newList) {
list = newList;
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2255,6 +2255,10 @@
"%0 attribute can only be applied once per parameter">;
def err_attribute_uuid_malformed_guid : Error<
"uuid attribute contains a malformed GUID">;
+def err_mismatched_uuid : Error<
+ "uiid does not match previous declaration">;
+def note_previous_uuid : Note<
+ "previous uuid specified here">;
def warn_attribute_pointers_only : Warning<
"%0 attribute only applies to%select{| constant}1 pointer arguments">,
InGroup<IgnoredAttributes>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
