Without this patch clang warns on

struct __attribute__((visibility("hidden"))) zed {
};
struct __attribute__((visibility("hidden"))) zed;

Which is a bit silly and got a lot noisier now that we correctly handle
visibility pragmas. This patch fixes that and also has some extra quality
improvements:

* We now produce an error instead of a warning for

struct __attribute__((visibility("hidden"))) zed {
};
struct __attribute__((visibility("default"))) zed;

* The "after definition" warning now points to the new attribute that is
  ignored instead of pointing to the declaration.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@160227 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Sema/SemaDecl.cpp

@@ -1767,16 +1767,46 @@ static const Decl *getDefinition(const Decl *D) {
   return NULL;
 }
 
+static bool hasAttribute(const Decl *D, attr::Kind Kind) {
+  for (Decl::attr_iterator I = D->attr_begin(), E = D->attr_end();
+       I != E; ++I) {
+    Attr *Attribute = *I;
+    if (Attribute->getKind() == Kind)
+      return true;
+  }
+  return false;
+}
+
+/// checkNewAttributesAfterDef - If we already have a definition, check that
+/// there are no new attributes in this declaration.
+static void checkNewAttributesAfterDef(Sema &S, Decl *New, const Decl *Old) {
+  if (!New->hasAttrs())
+    return;
+
+  const Decl *Def = getDefinition(Old);
+  if (!Def || Def == New)
+    return;
+
+  AttrVec &NewAttributes = New->getAttrs();
+  for (unsigned I = 0, E = NewAttributes.size(); I != E;) {
+    const Attr *NewAttribute = NewAttributes[I];
+    if (hasAttribute(Def, NewAttribute->getKind())) {
+      ++I;
+      continue; // regular attr merging will take care of validating this.
+    }
+    S.Diag(NewAttribute->getLocation(),
+           diag::warn_attribute_precede_definition);
+    S.Diag(Def->getLocation(), diag::note_previous_definition);
+    NewAttributes.erase(NewAttributes.begin() + I);
+    --E;
+  }
+}
+
 /// mergeDeclAttributes - Copy attributes from the Old decl to the New one.
 void Sema::mergeDeclAttributes(Decl *New, Decl *Old,
                                bool MergeDeprecation) {
   // attributes declared post-definition are currently ignored
-  const Decl *Def = getDefinition(Old);
+  checkNewAttributesAfterDef(*this, New, Old);
-  if (Def && Def != New && New->hasAttrs()) {
-    Diag(New->getLocation(), diag::warn_attribute_precede_definition);
-    Diag(Def->getLocation(), diag::note_previous_definition);
-    New->dropAttrs();
-  }
 
   if (!Old->hasAttrs())
     return;

test/Sema/attr-decl-after-definition.c

@@ -14,9 +14,26 @@ int bar __attribute__((weak));
 int bar __attribute__((used));
 extern int bar __attribute__((weak));
 int bar = 0; // expected-note {{previous definition is here}}
-int bar __attribute__((weak)); // expected-warning {{must precede definition}}
+int bar __attribute__((weak)); // no warning as it matches the existing
+                               // attribute.
+int bar __attribute__((used,
+                       visibility("hidden"))); // expected-warning {{must precede definition}}
 int bar;
 
 struct zed {  // expected-note {{previous definition is here}}
 };
 struct __attribute__((visibility("hidden"))) zed; // expected-warning {{must precede definition}}
+
+struct __attribute__((visibility("hidden"))) zed2 {
+};
+struct __attribute__((visibility("hidden"))) zed2;
+
+struct __attribute__((visibility("hidden"))) zed3 {  // expected-note {{previous definition is here}}
+};
+struct __attribute__((visibility("hidden"),
+                     packed  // expected-warning {{must precede definition}}
+                     )) zed3;
+
+struct __attribute__((visibility("hidden"))) zed4 {  // expected-note {{previous attribute is here}}
+};
+struct __attribute__((visibility("default"))) zed4; // expected-error {{visibility does not match previous declaration}}