Simplify diagnostics for inherited default argument values in module interfaces

Also:
- additionally require the containing and overridden initializers are
  designated, as that's the only case in which we should produce the '= super'
  syntax in module interfaces
- Add notes to point out the locations of the overriden initializer when it's
  not designated, and the corresponding parameter in that initializer when it
  doesn't have a default argument to inherit.

Author: Nathan Hawes

include/swift/AST/DiagnosticsSema.def

@@ -1281,15 +1281,18 @@ WARNING(expr_dynamic_lookup_swift3_objc_inference,none,
         "deprecated in Swift 4",
         (DescriptiveDeclKind, DeclName, Identifier))
 
-ERROR(inherited_default_value_not_on_constructor_param,none,
-      "default value inheritance via 'super' is only valid for initializer "
-      "parameters", ())
+ERROR(inherited_default_value_not_in_designated_constructor,none,
+      "default value inheritance via 'super' is only valid on the parameters of "
+      "designated initializers", ())
+ERROR(inherited_default_value_used_in_non_overriding_constructor,none,
+      "default value inheritance via 'super' can only be used when "
+      "overriding a designated initializer", ())
 ERROR(corresponding_param_not_defaulted,none,
       "default value inheritance via 'super' requires that the corresponding "
-      "parameter of the overridden initializer has a default value", ())
-ERROR(inherited_default_value_used_in_non_overriding_constructor,none,
-      "default value inheritance via 'super' can only be used on the "
-      "parameters of overriding initializers", ())
+      "parameter of the overridden designated initializer has a default value",
+      ())
+NOTE(inherited_default_param_here,none,
+     "corresponding parameter declared here", ())
 
 // Alignment attribute
 ERROR(alignment_not_power_of_two,none,

lib/Parse/ParsePattern.cpp

@@ -71,9 +71,16 @@ static ParserStatus parseDefaultArgument(
   SyntaxParsingContext DefaultArgContext(P.SyntaxContext,
                                          SyntaxKind::InitializerClause);
   SourceLoc equalLoc = P.consumeToken(tok::equal);
-  // Handle the special '= super' syntax for inherited default values in module
-  // interface files
+
   if (P.SF.Kind == SourceFileKind::Interface) {
+    // Swift module interfaces don't synthesize inherited intializers and
+    // instead include them explicitly in subclasses. Since the
+    // \c DefaultArgumentKind of these initializers is \c Inherited, this is
+    // represented textually as `= super` in the interface.
+
+    // If we're in a module interface and the default argument is exactly
+    // `super` (i.e. the token after that is `,` or `)` which end a parameter)
+    // report an inherited default argument to the caller and return.
     if (P.Tok.is(tok::kw_super) && P.peekToken().isAny(tok::comma, tok::r_paren)) {
       hasInheritedDefaultArg = true;
       P.consumeToken(tok::kw_super);

lib/Sema/TypeCheckStmt.cpp

@@ -1753,51 +1753,37 @@ checkInheritedDefaultValueRestrictions(TypeChecker &TC, ParamDecl *PD) {
 
   auto *DC = PD->getInnermostDeclContext();
   const SourceFile *SF = DC->getParentSourceFile();
-  if (!SF || SF->Kind != SourceFileKind::Interface)
+  assert((SF && SF->Kind == SourceFileKind::Interface || PD->isImplicit()) &&
+         "explicit inherited default argument outside of a module interface?");
+
+  // The containing decl should be a designated initializer.
+  auto ctor = dyn_cast<ConstructorDecl>(DC);
+  if (!ctor || ctor->isConvenienceInit()) {
+    TC.diagnose(
+        PD->getLoc(),
+        diag::inherited_default_value_not_in_designated_constructor);
     return;
+  }
 
-  if (!isa<ConstructorDecl>(DC)) {
-    TC.diagnose(PD->getLoc(),
-                diag::inherited_default_value_not_on_constructor_param);
+  // The decl it overrides should also be a designated initializer.
+  auto overridden = ctor->getOverriddenDecl();
+  if (!overridden || overridden->isConvenienceInit()) {
+    TC.diagnose(
+        PD->getLoc(),
+        diag::inherited_default_value_used_in_non_overriding_constructor);
+    if (overridden)
+      TC.diagnose(overridden->getLoc(), diag::overridden_here);
     return;
   }
 
-  // Check the matching parameter of an overriden decl has a default value
-  auto ctor = cast<ConstructorDecl>(DC);
+  // The corresponding parameter should have a default value.
   Optional<unsigned> idx = getParamIndex(ctor->getParameters(), PD);
-  bool foundOverriden = false;
   assert(idx && "containing decl does not contain param?");
-  for (auto overridden = ctor->getOverriddenDecl();
-       overridden != nullptr;
-       overridden = overridden->getOverriddenDecl()) {
-    foundOverriden = true;
-    auto equivalentParam = overridden->getParameters()->get(*idx);
-    assert(equivalentParam && "inherited decl mismatched arguments?");
-
-    // If the corresponding overridden param is also inherited, keep looking
-    if (equivalentParam->getDefaultArgumentKind() ==
-        DefaultArgumentKind::Inherited)
-      continue;
-
-    // If it doesn't have a default value, diagnose it
-    if (equivalentParam->getDefaultArgumentKind() == DefaultArgumentKind::None) {
-      TC.diagnose(PD->getLoc(),
-                  diag::corresponding_param_not_defaulted);
-    }
-    // We found a match: it's valid
-    return;
+  ParamDecl *equivalentParam = overridden->getParameters()->get(*idx);
+  if (equivalentParam->getDefaultArgumentKind() == DefaultArgumentKind::None) {
+    TC.diagnose(PD->getLoc(), diag::corresponding_param_not_defaulted);
+    TC.diagnose(equivalentParam->getLoc(), diag::inherited_default_param_here);
   }
-
-  // Used with a param of a non-overriding initializer, or one where the
-  // corresponding param in the overridden decl was also inherited but *it*
-  // didn't override anything. Diagnose the first case and ignore the second.
-  // The second will be diagnosed when checking the default args of the
-  // overridden declaration.
-  if (foundOverriden)
-    return;
-
-  TC.diagnose(PD->getLoc(),
-              diag::inherited_default_value_used_in_non_overriding_constructor);
 }
 
 /// Check the default arguments that occur within this pattern.

test/ParseableInterface/default-args.swift

@@ -12,19 +12,24 @@
 // CHECK: class Base {
 public class Base {
   // CHECK: init(x: Int = 3)
-	public init(x: Int = 3) {}
-	// CHECK: foo(y: Int = 42)
-	public func foo(y: Int = 42) {}
+  public init(x: Int = 3) {}
+  public convenience init(convInit: Int) {
+    self.init(x: convInit)
+  }
+  // CHECK: foo(y: Int = 42)
+  public func foo(y: Int = 42) {}
 }
 
 // CHECK: class Derived : Base {
 public class Derived: Base {
-	// CHECK: init(y: Int)
-	public convenience init(y: Int) {
-		self.init()
-	}
-
-	// CHECK: override {{(public )?}}init(x: Int = super)
+  // CHECK: init(y: Int)
+  public convenience init(y: Int) {
+    self.init()
+  }
+
+  // CHECK-NOT: init(convInit: Int = super)
+  // CHECK: override {{(public )?}}init(x: Int = super)
+  // CHECK-NOT: init(convInit: Int = super)
 }
 
 public enum Enum {

test/ParseableInterface/inherited-defaults-checks.swiftinterface

@@ -3,36 +3,63 @@
 
 // RUN: %empty-directory(%t)
 // RUN: %target-typecheck-verify-swift
-// BUN: %target-swift-frontend -typecheck -verify -module-cache-path %t/MCP -o %t/InheritedDefaults.swiftmodule -module-name InheritedDefaults %s
 
 import Swift
 
 public class Bar {
-  public init(x: Int = 24, y: Int)
+  // note associated with the expected error in (F) below
+  public init(x: Int = 24, y: Int, z: Int = 42) // expected-note {{corresponding parameter declared here}}
+
+  public init(a: Int, b: Int = 99)
+  public convenience init(convInit: Int = 45) {}
+
+  // note associated with the expected error in (D) below
+  public convenience init(first: Int, second: Int = 88, third: Int, fourth: Int) // expected-note {{overridden declaration is here}}
 }
 
 public class Foo: Bar {
-  public override init(x: Int = super, y: Int)
-  public subscript(k: Int = super) -> Int { get } // expected-error {{default value inheritance via 'super' is only valid for initializer parameters}}
-  public func foo(z: Int = super) // expected-error {{default value inheritance via 'super' is only valid for initializer parameters}}
+
+  // A) designated overriding designated - valid
+  public override init(x: Int = super, y: Int, z: Int = super)
+
+  // B) convenience shadowing convenience
+  public convenience init(convInit: Int = super) // expected-error {{default value inheritance via 'super' is only valid on the parameters of designated initializers}}
+
+  // C) convenience overriding designated
+  public override convenience init(a: Int, b: Int = super) // expected-error {{default value inheritance via 'super' is only valid on the parameters of designated initializers}}
+
+  // D) designated shadowing convenience
+  public init(first: Int, second: Int = super, third: Int, fourth: Int) // expected-error {{default value inheritance via 'super' can only be used when overriding a designated initializer}}
+
+  // E) not in initializer
+  public subscript(k: Int = super) -> Int { get } // expected-error {{default value inheritance via 'super' is only valid on the parameters of designated initializers}}
+  public func foo(z: Int = super) // expected-error {{default value inheritance via 'super' is only valid on the parameters of designated initializers}}
 }
 
 public class Baz: Bar {
-  public override init(x: Int = super, y: Int = super) // expected-error {{default value inheritance via 'super' requires that the corresponding parameter of the overridden initializer has a default value}}
+
+  // F) Matching param not defaulted
+  public override init(x: Int = super, y: Int = super, z: Int = super) // expected-error {{default value inheritance via 'super' requires that the corresponding parameter of the overridden designated initializer has a default value}}
 }
 
 public class Direct: Bar {
-  public override init(x: Int = super, y: Int)
-  public override init(other: Int = super, value: Int) // expected-error {{argument labels for initializer 'init(other:value:)' do not match those of overridden initializer 'init(x:y:)'}}
-  // expected-error@-1 {{default value inheritance via 'super' can only be used on the parameters of overriding initializers}}
+  public override init(x: Int = super, y: Int, z: Int = super)
+
+  // G) Doesn't override anything
+  public override init(other: Int = super, value: Int) // expected-error {{argument labels for initializer 'init(other:value:)' do not match those of overridden initializer 'init(a:b:)'}}
+  // expected-error@-1 {{default value inheritance via 'super' can only be used when overriding a designated initializer}}
 }
 
 public class Indirect: Direct {
-  public override init(x: Int = super, y: Int)
+
+  // H) Chain of inherited defaults - valid all the way down
+  public override init(x: Int = super, y: Int, z: Int = super)
+
+  // I) Chain of inherited defaults - invalid further down (and diagnosed there)
   public override init(other: Int = super, value: Int)
 }
 
 public enum Bob {
   case bar(p: Int)
-  public init(foo: String = super /*comment*/) // expected-error {{default value inheritance via 'super' can only be used on the parameters of overriding initializers}}
+  public init(foo: String = super /*comment*/) // expected-error {{default value inheritance via 'super' can only be used when overriding a designated initializer}}
 }