__uuidof() and declspec(uuid("...")) should be allowed on enumeration types

[puetzk]

Summary

Although not specifically mentioned in the documentation, MSVC accepts __uuidof(…) and declspec(uuid("…")) attributes on enumeration types in addition to structs/classes. This is meaningful, as such types *do* have associated UUIDs in ActiveX typelibs, and such attributes are included by default in the wrappers generated by their #import construct, so they are not particularly unusual.

clang currently rejects the declspec with a –Wignored-attributes warning, and errors on uuidof() with “cannot call operator uuidof on a type with no GUID” (because it rejected the uuid attribute, and therefore finds no value). This is causing problems for us while trying to use clang-tidy on a codebase that makes heavy use of ActiveX.

I believe I have found the relevant places to add this functionality, this patch adds this case to clang’s implementation of these MS extensions.
patch is against r285994 (or actually the git mirror 80464680ce).

Both include an update to test/Parser/MicrosoftExtensions.cpp to exercise the new functionality.

This is my first time contributing to LLVM, so if I’ve missed anything else needed to prepare this for review just let me know!

__uuidof: https://msdn.microsoft.com/en-us/library/zaah6a61.aspx
declspec(uuid("…")): https://msdn.microsoft.com/en-us/library/3b6wkewa.aspx
#import: https://msdn.microsoft.com/en-us/library/8etzzkb6.aspx

[majnemer]

Do we have a testcase where the declspec is applied to something inappropriate like an int?

lib/Sema/SemaDeclAttr.cpp

4669–4673 ↗

(On Diff #78499)

I don't think you need this now that you've got this in Attr.td

lib/Sema/SemaExprCXX.cpp

523 ↗

(On Diff #78499)

Please renamed RD to something more appropriate like TD.

[aaron.ballman]

include/clang/Basic/Attr.td

1621 ↗

(On Diff #78499)

You can drop the WarnDiag and ExpectedEnumOrClass -- that should be handled automatically by the tablegen emitter once you change CalculateDiagnostic() in ClangAttrEmitter.cpp to handle it properly.

include/clang/Basic/DiagnosticSemaKinds.td

2637 ↗

(On Diff #78499)

This should read classes and enumerations.

lib/Sema/SemaDeclAttr.cpp

4669–4673 ↗

(On Diff #78499)

Correct, this entire block should go away.

[puetzk]

Address comments from @aaron.ballman and @majnemer

Add a test case wihich triggers "the 'uuid' attribute only applies to classes and enumerations" warning

[puetzk]

Also rebased against r287335/496a3f56c7

[majnemer]

This LGTM but Aaron should give the go ahead.

[rnk]

This looks good to me, I'll merge this today unless I hear otherwise.

[rnk]

FYI I had to make a few changes so that we still error out on __uuid in C mode.

[puetzk]

I see that you added a FIXME mentioning that it should be C++-only, but I don't see where you actually did anything that would make it so. What am I missing?

Also, did you mean to drop the changes to utils/TableGen/ClangAttrEmitter.cpp and bring back the WarnDiag, "ExpectedEnumOrClass" @aaron.ballman asked for in the first review?

cfe/trunk/include/clang/Basic/Attr.td

 def Used : InheritableAttr {
   let Spellings = [GCC<"used">];
   let Documentation = [Undocumented];
 }
 
 def Uuid : InheritableAttr {
   let Spellings = [Declspec<"uuid">, Microsoft<"uuid">];
   let Args = [StringArgument<"Guid">];
-//  let Subjects = SubjectList<[CXXRecord]>;
+  let Subjects = SubjectList<[Record, Enum], WarnDiag, "ExpectedEnumOrClass">;
+  // FIXME: Allow expressing logical AND for LangOpts. Our condition should be:
+  // CPlusPlus && (MicrosoftExt || Borland)
   let LangOpts = [MicrosoftExt, Borland];
   let Documentation = [Undocumented];
 }
 
 def VectorSize : TypeAttr {
   let Spellings = [GCC<"vector_size">];
   let Args = [ExprArgument<"NumBytes">];
   let Documentation = [Undocumented];

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def warn_attribute_wrong_decl_type : Warning<
   "|structs, unions, and typedefs"
   "|structs and typedefs"
   "|interface or protocol declarations"
   "|kernel functions"
   "|non-K&R-style functions"
   "|variables, enums, fields and typedefs"
   "|functions, methods, enums, and classes"
   "|structs, classes, variables, functions, and inline namespaces"
-  "|variables, functions, methods, types, enumerations, enumerators, labels, and non-static data members}1">,
+  "|variables, functions, methods, types, enumerations, enumerators, labels, and non-static data members"
+  "|classes and enumerations}1">,
   InGroup<IgnoredAttributes>;
 def err_attribute_wrong_decl_type : Error<warn_attribute_wrong_decl_type.Text>;
 def warn_type_attribute_wrong_type : Warning<
   "'%0' only applies to %select{function|pointer|"
   "Objective-C object or block pointer}1 types; type here is %2">,
   InGroup<IgnoredAttributes>;
 def warn_incomplete_encoded_type : Warning<
   "encoding of %0 type is incomplete because %1 component has unknown encoding">,

cfe/trunk/include/clang/Sema/AttributeList.h

enum AttributeDeclKind {
   ExpectedStructOrUnionOrTypedef,
   ExpectedStructOrTypedef,
   ExpectedObjectiveCInterfaceOrProtocol,
   ExpectedKernelFunction,
   ExpectedFunctionWithProtoType,
   ExpectedVariableEnumFieldOrTypedef,
   ExpectedFunctionMethodEnumOrClass,
   ExpectedStructClassVariableFunctionOrInlineNamespace,
-  ExpectedForMaybeUnused
+  ExpectedForMaybeUnused,
+  ExpectedEnumOrClass,
 };
 
 }  // end namespace clang
 
 #endif

cfe/trunk/lib/Sema/SemaDeclAttr.cpp

 
 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)
       << Attr.getName() << AttributeLangSupport::C;
     return;
   }
 
-  if (!isa<CXXRecordDecl>(D)) {
-    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
-      << Attr.getName() << ExpectedClass;
-    return;
-  }
-
   StringRef StrRef;
   SourceLocation LiteralLoc;
   if (!S.checkStringLiteralArgumentAttr(Attr, 0, StrRef, &LiteralLoc))
     return;
 
   // GUID format is "XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX" or
   // "{XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX}", normalize to the former.
   if (StrRef.size() == 38 && StrRef.front() == '{' && StrRef.back() == '}')

cfe/trunk/lib/Sema/SemaExprCXX.cpp

getUuidAttrOfType(Sema &SemaRef, QualType QT,
                   llvm::SmallSetVector<const UuidAttr *, 1> &UuidAttrs) {
   // Optionally remove one level of pointer, reference or array indirection.
   const Type *Ty = QT.getTypePtr();
   if (QT->isPointerType() || QT->isReferenceType())
     Ty = QT->getPointeeType().getTypePtr();
   else if (QT->isArrayType())
     Ty = Ty->getBaseElementTypeUnsafe();
 
-  const auto *RD = Ty->getAsCXXRecordDecl();
-  if (!RD)
+  const auto *TD = Ty->getAsTagDecl();
+  if (!TD)
     return;
 
-  if (const auto *Uuid = RD->getMostRecentDecl()->getAttr<UuidAttr>()) {
+  if (const auto *Uuid = TD->getMostRecentDecl()->getAttr<UuidAttr>()) {
     UuidAttrs.insert(Uuid);
     return;
   }
 
   // __uuidof can grab UUIDs from template arguments.
-  if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) {
+  if (const auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(TD)) {
     const TemplateArgumentList &TAL = CTSD->getTemplateArgs();
     for (const TemplateArgument &TA : TAL.asArray()) {
       const UuidAttr *UuidForTA = nullptr;
       if (TA.getKind() == TemplateArgument::Type)
         getUuidAttrOfType(SemaRef, TA.getAsType(), UuidAttrs);
       else if (TA.getKind() == TemplateArgument::Declaration)
         getUuidAttrOfType(SemaRef, TA.getAsDecl()->getType(), UuidAttrs);
 

cfe/trunk/test/Parser/MicrosoftExtensions.cpp

 struct __declspec(uuid("000000A0-0000-0000-C000-000000000049"))
 struct_with_uuid2;
 
 [uuid("000000A0-0000-0000-C000-000000000049")] struct struct_with_uuid3;
 
 struct
 struct_with_uuid2 {} ;
 
+enum __declspec(uuid("000000A0-0000-0000-C000-000000000046"))
+enum_with_uuid { };
+enum enum_without_uuid { };
+
+int __declspec(uuid("000000A0-0000-0000-C000-000000000046")) inappropriate_uuid; // expected-warning {{'uuid' attribute only applies to classes and enumerations}}
+
 int uuid_sema_test()
 {
    struct_with_uuid var_with_uuid[1];
    struct_without_uuid var_without_uuid[1];
 
    __uuidof(struct_with_uuid);
    __uuidof(struct_with_uuid2);
    __uuidof(struct_with_uuid3);
    __uuidof(struct_without_uuid); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
    __uuidof(struct_with_uuid*);
    __uuidof(struct_without_uuid*); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
    __uuidof(struct_with_uuid[1]);
    __uuidof(struct_with_uuid*[1]); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
    __uuidof(const struct_with_uuid[1][1]);
    __uuidof(const struct_with_uuid*[1][1]); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
 
+   __uuidof(enum_with_uuid);
+   __uuidof(enum_without_uuid); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
+   __uuidof(enum_with_uuid*);
+   __uuidof(enum_without_uuid*); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
+   __uuidof(enum_with_uuid[1]);
+   __uuidof(enum_with_uuid*[1]); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
+   __uuidof(const enum_with_uuid[1][1]);
+   __uuidof(const enum_with_uuid*[1][1]); // expected-error {{cannot call operator __uuidof on a type with no GUID}}
+
    __uuidof(var_with_uuid);
    __uuidof(var_without_uuid);// expected-error {{cannot call operator __uuidof on a type with no GUID}}
    __uuidof(var_with_uuid[1]);
    __uuidof(var_without_uuid[1]);// expected-error {{cannot call operator __uuidof on a type with no GUID}}
    __uuidof(&var_with_uuid[1]);
    __uuidof(&var_without_uuid[1]);// expected-error {{cannot call operator __uuidof on a type with no GUID}}
 
    __uuidof(0);