Addressing discussion feedback

Keeping the original duplicate message if the access levels match
Changing the note message
Avoid extra attribute list traversals
Also avoiding extra variable declarations

Author: Visckmart

include/swift/AST/DiagnosticsParse.def

@@ -1488,7 +1488,7 @@ NOTE(previous_attribute,none,
 ERROR(multiple_access_level_modifiers,none,
       "multiple incompatible access-level modifiers specified", ())
 NOTE(previous_access_level_modifier,none,
-     "incompatible modifier specified here", (StringRef))
+     "previous modifier specified here", (StringRef))
 ERROR(mutually_exclusive_attrs,none,
       "'%0' contradicts previous %select{attribute|modifier}2 '%1'", (StringRef, StringRef, bool))
 

lib/Parse/ParseDecl.cpp

@@ -2987,20 +2987,21 @@ ParserStatus Parser::parseNewDeclAttribute(DeclAttributes &Attributes,
       // Normal access control attribute.
       AttrRange = Loc;
 
-      // Try to find a previously parsed access control attribute.
-      auto PrevAccessControlAttr = Attributes.getAttribute<AccessControlAttr>();
 
-      if (!PrevAccessControlAttr) {
+      if (!DiscardAttribute) {
         Attributes.add(new (Context) AccessControlAttr(AtLoc, Loc, access));
-      } else {
+        break;
+      }
+      
+      if (access != cast<AccessControlAttr>(*DuplicateAttribute).getAccess()) {
         // Diagnose if there is already an access control attribute on
-        // this declaration.
+        // this declaration with a different access level.
         diagnose(Loc, diag::multiple_access_level_modifiers)
           .highlight(AttrRange);
-        diagnose(PrevAccessControlAttr->getLocation(),
+        diagnose(DuplicateAttribute->getLocation(),
                  diag::previous_access_level_modifier,
-                 PrevAccessControlAttr->getAttrName())
-          .highlight(PrevAccessControlAttr->getRange());
+                 DuplicateAttribute->getAttrName())
+          .highlight(DuplicateAttribute->getRange());
 
         // Remove the reference to the duplicate attribute
         // to avoid the extra diagnostic.
@@ -3037,20 +3038,20 @@ ParserStatus Parser::parseNewDeclAttribute(DeclAttributes &Attributes,
 
     DuplicateAttribute = Attributes.getAttribute<SetterAccessAttr>();
 
-    // Try to find a previously parsed setter access control attribute.
-    auto PrevSetterAccessControlAttr = Attributes.getAttribute<SetterAccessAttr>();
-
-    if (!PrevSetterAccessControlAttr) {
+    if (!DuplicateAttribute) {
       Attributes.add(new (Context) SetterAccessAttr(AtLoc, AttrRange, access));
-    } else {
-      // Diagnose if there is already a setter access control attribute on
-      // this declaration.
+      break;
+    }
+    
+    if (access != cast<SetterAccessAttr>(*DuplicateAttribute).getAccess()) {
+      // Diagnose if there is already an access control attribute on
+      // this declaration with a different access level.
       diagnose(Loc, diag::multiple_access_level_modifiers)
         .highlight(AttrRange);
-      diagnose(PrevSetterAccessControlAttr->getLocation(),
+      diagnose(DuplicateAttribute->getLocation(),
                diag::previous_access_level_modifier,
-               PrevSetterAccessControlAttr->getAttrName())
-      .highlight(PrevSetterAccessControlAttr->getRange());
+               DuplicateAttribute->getAttrName())
+        .highlight(DuplicateAttribute->getRange());
 
       // Remove the reference to the duplicate attribute
       // to avoid the extra diagnostic.

test/attr/accessibility.swift

@@ -1,26 +1,26 @@
 // RUN: %target-typecheck-verify-swift -package-name myPkg
 
 // CHECK PARSING
-private // expected-note {{incompatible modifier specified here}}
-private // expected-error {{multiple incompatible access-level modifiers specified}}
+private // expected-note {{modifier already specified here}}
+private // expected-error {{duplicate modifier}}
 func duplicateAttr() {}
 
-private // expected-note {{incompatible modifier specified here}}
+private // expected-note {{previous modifier specified here}}
 public // expected-error {{multiple incompatible access-level modifiers specified}}
 func duplicateAttrChanged() {}
 
-private // expected-note 2 {{incompatible modifier specified here}}
+private // expected-note 2 {{previous modifier specified here}}
 public // expected-error {{multiple incompatible access-level modifiers specified}}
 internal // expected-error {{multiple incompatible access-level modifiers specified}}
 func triplicateAttrChanged() {}
 
-private // expected-note 3 {{incompatible modifier specified here}}
+private // expected-note 3 {{previous modifier specified here}}
 public // expected-error {{multiple incompatible access-level modifiers specified}}
 package // expected-error {{multiple incompatible access-level modifiers specified}}
 internal // expected-error {{multiple incompatible access-level modifiers specified}}
 func quadruplicateAttrChanged() {}
 
-private // expected-note 4 {{incompatible modifier specified here}}
+private // expected-note 4 {{previous modifier specified here}}
 public // expected-error {{multiple incompatible access-level modifiers specified}}
 package // expected-error {{multiple incompatible access-level modifiers specified}}
 internal // expected-error {{multiple incompatible access-level modifiers specified}}
@@ -51,21 +51,21 @@ internal(set)
 package
 var customSetter6 = 0
 
-private(set) // expected-note {{incompatible modifier specified here}}
+private(set) // expected-note {{previous modifier specified here}}
 public(set) // expected-error {{multiple incompatible access-level modifiers specified}}
 var customSetterDuplicateAttr = 0
 
-private(set) // expected-note {{incompatible modifier specified here}}
-public // expected-note {{incompatible modifier specified here}}
+private(set) // expected-note {{previous modifier specified here}}
+public // expected-note {{previous modifier specified here}}
 public(set) // expected-error {{multiple incompatible access-level modifiers specified}}
 private // expected-error {{multiple incompatible access-level modifiers specified}}
 var customSetterDuplicateAttrsAllAround = 0
 
-private(set) // expected-note {{incompatible modifier specified here}}
+private(set) // expected-note {{previous modifier specified here}}
 package(set) // expected-error {{multiple incompatible access-level modifiers specified}}
 var customSetterDuplicateAttr2 = 0
 
-package(set) // expected-note {{incompatible modifier specified here}}
+package(set) // expected-note {{previous modifier specified here}}
 public(set) // expected-error {{multiple incompatible access-level modifiers specified}}
 public var customSetterDuplicateAttr3 = 0
 

test/attr/open.swift

@@ -6,11 +6,11 @@ import OpenHelpers
 
 /**** General structural limitations on open. ****/
 
-open private class OpenIsNotCompatibleWithPrivate {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{incompatible modifier specified here}}
-open fileprivate class OpenIsNotCompatibleWithFilePrivate {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{incompatible modifier specified here}}
-open internal class OpenIsNotCompatibleWithInternal {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{incompatible modifier specified here}}
-open public class OpenIsNotCompatibleWithPublic {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{incompatible modifier specified here}}
-open open class OpenIsNotCompatibleWithOpen {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{incompatible modifier specified here}}
+open private class OpenIsNotCompatibleWithPrivate {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{previous modifier specified here}}
+open fileprivate class OpenIsNotCompatibleWithFilePrivate {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{previous modifier specified here}}
+open internal class OpenIsNotCompatibleWithInternal {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{previous modifier specified here}}
+open public class OpenIsNotCompatibleWithPublic {} // expected-error {{multiple incompatible access-level modifiers specified}} expected-note{{previous modifier specified here}}
+open open class OpenIsNotCompatibleWithOpen {} // expected-error {{duplicate modifier}} expected-note{{modifier already specified here}}
 
 open typealias OpenIsNotAllowedOnTypeAliases = Int // expected-error {{only classes and overridable class members can be declared 'open'; use 'public'}}
 open struct OpenIsNotAllowedOnStructs {} // expected-error {{only classes and overridable class members can be declared 'open'; use 'public'}}