Error on using dll attribute on members of class that already has a dll attribute

[hans]

Forgot to mention, this depends on http://reviews.llvm.org/D3972

[nrieck]

LGTM except that this check should not be done for MinGW (and on a related note, I wish MSVC would behave the same since I can't see the benefit of this diagnostic).

include/clang/Basic/DiagnosticSemaKinds.td

2455

Do we need a new note here? Wouldn't "previous attribute is here" suffice?

[hans]

LGTM except that this check should not be done for MinGW (and on a related note, I wish MSVC would behave the same since I can't see the benefit of this diagnostic).

Thanks for pointing this out. I've kept it as an error for now, but maybe we want to downgrade it to a warning since nothing is breaking here, it's just a compatibility issue with MSVC.

include/clang/Basic/DiagnosticSemaKinds.td

2455

You're right, let's just use the old note.

[hans]

Closed by commit rL209954 (authored by @hans).

include/clang/Basic/DiagnosticSemaKinds.td

 def err_attribute_dllimport_function_definition : Error<
   "dllimport cannot be applied to non-inline function definition">;
 def err_attribute_dll_deleted : Error<
   "attribute %q0 cannot be applied to a deleted function">;
 def err_attribute_dllimport_data_definition : Error<
   "definition of dllimport data">;
 def err_attribute_dllimport_static_field_definition : Error<
   "definition of dllimport static field not allowed">;
+def err_attribute_dll_member_of_dll_class : Error<
+  "attribute %q0 cannot be applied to member of %q1 class">;
 def err_attribute_weakref_not_static : Error<
   "weakref declaration must have internal linkage">;
 def err_attribute_weakref_not_global_context : Error<
   "weakref declaration of %0 must be in a global context">;
 def err_attribute_weakref_without_alias : Error<
   "weakref declaration of %0 must also have an alias attribute">;
 def err_alias_not_supported_on_darwin : Error <
   "only weak aliases are supported on darwin">;
def warn_attribute_type_not_supported : Warning<
   InGroup<IgnoredAttributes>;
 def warn_attribute_unknown_visibility : Warning<"unknown visibility %0">,
   InGroup<IgnoredAttributes>;
 def warn_attribute_protected_visibility :
   Warning<"target does not support 'protected' visibility; using 'default'">,
   InGroup<DiagGroup<"unsupported-visibility">>;
 def err_mismatched_visibility: Error<"visibility does not match previous declaration">;
 def note_previous_attribute : Note<"previous attribute is here">;
+def note_class_attribute : Note<"class attribute is here">;
 def note_attribute : Note<"attribute is here">;
 def err_mismatched_ms_inheritance : Error<
   "inheritance model does not match %select{definition|previous declaration}0">;
 def warn_ignored_ms_inheritance : Warning<
   "inheritance model ignored on %select{primary template|partial specialization}0">,
   InGroup<IgnoredAttributes>;
 def note_previous_ms_inheritance : Note<
   "previous inheritance model specified here">;

lib/Sema/SemaDeclCXX.cpp

static void checkDLLAttribute(Sema &S, CXXRecordDecl *Class) {
 
   // FIXME: MSVC's docs say all bases must be exportable, but this doesn't
   // seem to be true in practice?
 
   // FIXME: We also need to propagate the attribute upwards to class template
   // specialization bases.
 
   for (Decl *Member : Class->decls()) {
-    if (getDLLAttr(Member)) {
-      // FIXME: Error about importing/exporting individual members.
-    }
-
     if (!isa<CXXMethodDecl>(Member) && !isa<VarDecl>(Member))
       continue;
 
-    auto *NewAttr = cast<InheritableAttr>(ClassAttr->clone(S.getASTContext()));
+    if (InheritableAttr *MemberAttr = getDLLAttr(Member)) {
+      if (!MemberAttr->isInherited()) {
+        S.Diag(MemberAttr->getLocation(),
+               diag::err_attribute_dll_member_of_dll_class)
+            << MemberAttr << ClassAttr;
+        S.Diag(ClassAttr->getLocation(), diag::note_class_attribute);
+        Member->setInvalidDecl();
+        continue;
+      }
+    } else {
+      auto *NewAttr =
+          cast<InheritableAttr>(ClassAttr->clone(S.getASTContext()));
       NewAttr->setInherited(true);
       Member->addAttr(NewAttr);
+    }
 
     if (CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(Member)) {
       if (ClassExported) {
         if (MD->isDeleted())
           continue;
 
         if (MD->isUserProvided()) {
           // Instantiate non-default methods.

test/SemaCXX/dllimport.cpp

 
 //===----------------------------------------------------------------------===//
 // Classes
 //===----------------------------------------------------------------------===//
 
 class __declspec(dllimport) ClassDecl;
 
 class __declspec(dllimport) ClassDef { };
+
+class __declspec(dllimport) ImportClassWithDllMember { // expected-note2{{class attribute is here}}
+  void __declspec(dllexport) foo(); // expected-error{{attribute 'dllexport' cannot be applied to member of 'dllimport' class}}
+  void __declspec(dllimport) bar(); // expected-error{{attribute 'dllimport' cannot be applied to member of 'dllimport' class}}
+};
+
+class __declspec(dllexport) ExportClassWithDllMember { // expected-note2{{class attribute is here}}
+  void __declspec(dllimport) foo(); // expected-error{{attribute 'dllimport' cannot be applied to member of 'dllexport' class}}
+  void __declspec(dllexport) bar(); // expected-error{{attribute 'dllexport' cannot be applied to member of 'dllexport' class}}
+};