Improve access level mismatch diagnostics.

Use a single consistent error message stating "derivative function must have
same access level as original function".

Add access level fix-it on the derivative function.

Author: dan-zheng

include/swift/AST/DiagnosticsSema.def

@@ -3054,22 +3054,18 @@ ERROR(derivative_attr_original_already_has_derivative,none,
       "a derivative already exists for %0", (DeclName))
 NOTE(derivative_attr_duplicate_note,none,
      "other attribute declared here", ())
-ERROR(derivative_attr_access_level_lower_than_original,none,
-      "derivative functions for "
-      "%select{private|fileprivate|internal|public or '@usableFromInline'|open}1 "
-      "original function %0 must also be "
-      "%select{private|fileprivate|internal|public or '@usableFromInline'|open}1, "
-      "but %2 is %select{private|fileprivate|internal|public|open}3",
-      (/*original*/ DeclName, /*original*/ AccessLevel,
-       /*derivative*/ DeclName, /*derivative*/ AccessLevel))
-ERROR(derivative_attr_access_level_higher_than_original,none,
-      "the original function of "
-      "%select{a private|a fileprivate|an internal|a public or '@usableFromInline'|an open}3 "
-      "derivative function must also be "
-      "%select{private|fileprivate|internal|public or '@usableFromInline'|open}3, "
-      "but %0 is %select{private|fileprivate|internal|public|open}1",
+ERROR(derivative_attr_access_level_mismatch,none,
+      "derivative function must have same access level as original function; "
+      "derivative function %2 is "
+      "%select{private|fileprivate|internal|public|open}3, "
+      "but original function %0 is "
+      "%select{private|fileprivate|internal|public|open}1",
       (/*original*/ DeclName, /*original*/ AccessLevel,
        /*derivative*/ DeclName, /*derivative*/ AccessLevel))
+NOTE(derivative_attr_fix_access,none,
+     "mark the derivative function as "
+     "'%select{private|fileprivate|internal|public|open}0' to match the "
+     "original function", (AccessLevel))
 
 // @transpose
 ERROR(transpose_attr_invalid_linearity_parameter_or_result,none,

lib/Sema/TypeCheckAttr.cpp

@@ -4541,7 +4541,7 @@ static bool typeCheckDerivativeAttr(ASTContext &Ctx, Decl *D,
   attr->setOriginalFunction(originalAFD);
 
   // Returns true if:
-  // - Original function and derivative function has the same access level.
+  // - Original function and derivative function have the same access level.
   // - Original function is public and derivative function is internal
   //   `@usableFromInline`. This is the only special case.
   auto compatibleAccessLevels = [&]() {
@@ -4553,21 +4553,16 @@ static bool typeCheckDerivativeAttr(ASTContext &Ctx, Decl *D,
 
   // Check access level compatibility for original and derivative functions.
   if (!compatibleAccessLevels()) {
-    auto diagID = diag::derivative_attr_access_level_higher_than_original;
-    AccessLevel originalAccess;
-    AccessLevel derivativeAccess;
-    if (originalAFD->getEffectiveAccess() < derivative->getEffectiveAccess()) {
-      originalAccess =
-          originalAFD->getFormalAccessScope().accessLevelForDiagnostics();
-      derivativeAccess = derivative->getEffectiveAccess();
-    } else {
-      diagID = diag::derivative_attr_access_level_lower_than_original;
-      originalAccess = originalAFD->getEffectiveAccess();
-      derivativeAccess =
-          derivative->getFormalAccessScope().accessLevelForDiagnostics();
-    }
-    diags.diagnose(originalName.Loc, diagID, originalAFD->getName(),
-                   originalAccess, derivative->getName(), derivativeAccess);
+    auto originalAccess = originalAFD->getFormalAccess();
+    auto derivativeAccess =
+        derivative->getFormalAccessScope().accessLevelForDiagnostics();
+    diags.diagnose(originalName.Loc,
+                   diag::derivative_attr_access_level_mismatch,
+                   originalAFD->getName(), originalAccess,
+                   derivative->getName(), derivativeAccess);
+    auto fixItDiag =
+        derivative->diagnose(diag::derivative_attr_fix_access, originalAccess);
+    fixItAccess(fixItDiag, derivative, originalAccess);
     return true;
   }
 

test/AutoDiff/Sema/derivative_attr_type_checking.swift

@@ -819,14 +819,6 @@ func _public_original_usablefrominline_derivative(_ x: Float) -> (value: Float,
   fatalError()
 }
 
-@usableFromInline
-func usablefrominline_original_public_derivative(_ x: Float) -> Float { x }
-// expected-error @+1 {{derivative functions for public or '@usableFromInline' original function 'usablefrominline_original_public_derivative' must also be public or '@usableFromInline', but '_usablefrominline_original_public_derivative' is public}}
-@derivative(of: usablefrominline_original_public_derivative)
-public func _usablefrominline_original_public_derivative(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
-  fatalError()
-}
-
 func internal_original_internal_derivative(_ x: Float) -> Float { x }
 @derivative(of: internal_original_internal_derivative)
 func _internal_original_internal_derivative(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
@@ -846,69 +838,87 @@ fileprivate func _fileprivate_original_fileprivate_derivative(_ x: Float) -> (va
 }
 
 fileprivate func fileprivate_original_private_derivative(_ x: Float) -> Float { x }
-// expected-error @+1 {{derivative functions for fileprivate original function 'fileprivate_original_private_derivative' must also be fileprivate, but '_fileprivate_original_private_derivative' is private}}
+// expected-error @+1 {{derivative function must have same access level as original function; derivative function '_fileprivate_original_private_derivative' is private, but original function 'fileprivate_original_private_derivative' is fileprivate}}
 @derivative(of: fileprivate_original_private_derivative)
+// expected-note @+1 {{mark the derivative function as 'fileprivate' to match the original function}}
 private func _fileprivate_original_private_derivative(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
   fatalError()
 }
 
 private func private_original_fileprivate_derivative(_ x: Float) -> Float { x }
-// expected-error @+1 {{derivative functions for fileprivate original function 'private_original_fileprivate_derivative' must also be fileprivate, but '_private_original_fileprivate_derivative' is fileprivate}}
+// expected-error @+1 {{derivative function must have same access level as original function; derivative function '_private_original_fileprivate_derivative' is fileprivate, but original function 'private_original_fileprivate_derivative' is private}}
 @derivative(of: private_original_fileprivate_derivative)
+// expected-note @+1 {{mark the derivative function as 'private' to match the original function}}
 fileprivate func _private_original_fileprivate_derivative(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
   fatalError()
 }
 
 // MARK: - Original function visibility < derivative function visibility
 
+@usableFromInline
+func usablefrominline_original_public_derivative(_ x: Float) -> Float { x }
+// expected-error @+1 {{derivative function must have same access level as original function; derivative function '_usablefrominline_original_public_derivative' is public, but original function 'usablefrominline_original_public_derivative' is internal}}
+@derivative(of: usablefrominline_original_public_derivative)
+// expected-note @+1 {{mark the derivative function as 'internal' to match the original function}}
+public func _usablefrominline_original_public_derivative(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
+  fatalError()
+}
+
 func internal_original_public_derivative(_ x: Float) -> Float { x }
-// expected-error @+1 {{the original function of a public or '@usableFromInline' derivative function must also be public or '@usableFromInline', but 'internal_original_public_derivative' is internal}}
+// expected-error @+1 {{derivative function must have same access level as original function; derivative function '_internal_original_public_derivative' is public, but original function 'internal_original_public_derivative' is internal}}
 @derivative(of: internal_original_public_derivative)
+// expected-note @+1 {{mark the derivative function as 'internal' to match the original function}}
 public func _internal_original_public_derivative(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
   fatalError()
 }
 
 private func private_original_usablefrominline_derivative(_ x: Float) -> Float { x }
-// expected-error @+1 {{the original function of a public or '@usableFromInline' derivative function must also be public or '@usableFromInline', but 'private_original_usablefrominline_derivative' is private}}
+// expected-error @+1 {{derivative function must have same access level as original function; derivative function '_private_original_usablefrominline_derivative' is internal, but original function 'private_original_usablefrominline_derivative' is private}}
 @derivative(of: private_original_usablefrominline_derivative)
 @usableFromInline
+// expected-note @+1 {{mark the derivative function as 'private' to match the original function}}
 func _private_original_usablefrominline_derivative(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
   fatalError()
 }
 
 private func private_original_public_derivative(_ x: Float) -> Float { x }
-// expected-error @+1 {{the original function of a public or '@usableFromInline' derivative function must also be public or '@usableFromInline', but 'private_original_public_derivative' is private}}
+// expected-error @+1 {{derivative function must have same access level as original function; derivative function '_private_original_public_derivative' is public, but original function 'private_original_public_derivative' is private}}
 @derivative(of: private_original_public_derivative)
+// expected-note @+1 {{mark the derivative function as 'private' to match the original function}}
 public func _private_original_public_derivative(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
   fatalError()
 }
 
 private func private_original_internal_derivative(_ x: Float) -> Float { x }
-// expected-error @+1 {{the original function of an internal derivative function must also be internal, but 'private_original_internal_derivative' is private}}
+// expected-error @+1 {{derivative function must have same access level as original function; derivative function '_private_original_internal_derivative' is internal, but original function 'private_original_internal_derivative' is private}}
 @derivative(of: private_original_internal_derivative)
+// expected-note @+1 {{mark the derivative function as 'private' to match the original function}}
 func _private_original_internal_derivative(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
   fatalError()
 }
 
 // MARK: - Original function visibility > derivative function visibility
 
 public func public_original_private_derivative(_ x: Float) -> Float { x }
-// expected-error @+1 {{derivative functions for public or '@usableFromInline' original function 'public_original_private_derivative' must also be public or '@usableFromInline', but '_public_original_private_derivative' is fileprivate}}
+// expected-error @+1 {{derivative function must have same access level as original function; derivative function '_public_original_private_derivative' is fileprivate, but original function 'public_original_private_derivative' is public}}
 @derivative(of: public_original_private_derivative)
+// expected-note @+1 {{mark the derivative function as 'public' to match the original function}}
 fileprivate func _public_original_private_derivative(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
   fatalError()
 }
 
 public func public_original_internal_derivative(_ x: Float) -> Float { x }
-// expected-error @+1 {{derivative functions for public or '@usableFromInline' original function 'public_original_internal_derivative' must also be public or '@usableFromInline', but '_public_original_internal_derivative' is internal}}
+// expected-error @+1 {{derivative function must have same access level as original function; derivative function '_public_original_internal_derivative' is internal, but original function 'public_original_internal_derivative' is public}}
 @derivative(of: public_original_internal_derivative)
+// expected-note @+1 {{mark the derivative function as 'public' to match the original function}}
 func _public_original_internal_derivative(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
   fatalError()
 }
 
 func internal_original_fileprivate_derivative(_ x: Float) -> Float { x }
-// expected-error @+1 {{derivative functions for internal original function 'internal_original_fileprivate_derivative' must also be internal, but '_internal_original_fileprivate_derivative' is fileprivate}}
+// expected-error @+1 {{derivative function must have same access level as original function; derivative function '_internal_original_fileprivate_derivative' is fileprivate, but original function 'internal_original_fileprivate_derivative' is internal}}
 @derivative(of: internal_original_fileprivate_derivative)
+// expected-note @+1 {{mark the derivative function as 'internal' to match the original function}}
 fileprivate func _internal_original_fileprivate_derivative(_ x: Float) -> (value: Float, pullback: (Float) -> Float) {
   fatalError()
 }