Merge pull request #24155 from nathawes/module-interface-inherit-default-args-via-super

swift-ci · web-flow · commit 8ea57598b0d0 · 2019-04-19T15:34:26.000-07:00
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -1288,6 +1288,19 @@ WARNING(expr_dynamic_lookup_swift3_objc_inference,none,
         "deprecated in Swift 4",
         (DescriptiveDeclKind, DeclName, Identifier))
 
+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 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,
       "alignment value must be a power of two", ())
diff --git a/include/swift/Parse/Parser.h b/include/swift/Parse/Parser.h
@@ -1144,6 +1144,9 @@ class Parser {
 
     /// The default argument for this parameter.
     Expr *DefaultArg = nullptr;
+
+    /// True if this parameter inherits a default argument via '= super'
+    bool hasInheritedDefaultArg = false;
     
     /// True if we emitted a parse error about this parameter.
     bool isInvalid = false;
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -5361,10 +5361,7 @@ ParamDecl::getDefaultValueStringRepresentation(
                                 var->getParentInitializer(),
                                 scratch);
   }
-  case DefaultArgumentKind::Inherited:
-    // FIXME: This needs /some/ kind of textual representation, but this isn't
-    // a great one.
-    return "super";
+  case DefaultArgumentKind::Inherited: return "super";
   case DefaultArgumentKind::File: return "#file";
   case DefaultArgumentKind::Line: return "#line";
   case DefaultArgumentKind::Column: return "#column";
diff --git a/lib/Parse/ParsePattern.cpp b/lib/Parse/ParsePattern.cpp
@@ -66,11 +66,30 @@ void Parser::DefaultArgumentInfo::setFunctionContext(
 
 static ParserStatus parseDefaultArgument(
     Parser &P, Parser::DefaultArgumentInfo *defaultArgs, unsigned argIndex,
-    Expr *&init, Parser::ParameterContextKind paramContext) {
+    Expr *&init, bool &hasInheritedDefaultArg,
+    Parser::ParameterContextKind paramContext) {
   SyntaxParsingContext DefaultArgContext(P.SyntaxContext,
                                          SyntaxKind::InitializerClause);
   SourceLoc equalLoc = P.consumeToken(tok::equal);
 
+  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);
+      P.SyntaxContext->createNodeInPlace(SyntaxKind::SuperRefExpr);
+      defaultArgs->HasDefaultArgument = true;
+      return ParserStatus();
+    }
+  }
+
   // Enter a fresh default-argument context with a meaningless parent.
   // We'll change the parent to the function later after we've created
   // that declaration.
@@ -367,7 +386,9 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
     if (Tok.is(tok::equal)) {
       SourceLoc EqualLoc = Tok.getLoc();
       status |= parseDefaultArgument(*this, defaultArgs, defaultArgIndex,
-                                     param.DefaultArg, paramContext);
+                                     param.DefaultArg,
+                                     param.hasInheritedDefaultArg,
+                                     paramContext);
 
       if (param.EllipsisLoc.isValid() && param.DefaultArg) {
         // The range of the complete default argument.
@@ -605,15 +626,20 @@ mapParsedParameters(Parser &parser,
       }
     }
 
+    assert (((!param.DefaultArg &&
+              !param.hasInheritedDefaultArg) ||
+             paramContext == Parser::ParameterContextKind::Function ||
+             paramContext == Parser::ParameterContextKind::Operator ||
+             paramContext == Parser::ParameterContextKind::Initializer ||
+             paramContext == Parser::ParameterContextKind::EnumElement) &&
+            "Default arguments are only permitted on the first param clause");
+
     if (param.DefaultArg) {
-      assert((paramContext == Parser::ParameterContextKind::Function ||
-              paramContext == Parser::ParameterContextKind::Operator ||
-              paramContext == Parser::ParameterContextKind::Initializer ||
-              paramContext == Parser::ParameterContextKind::EnumElement) &&
-             "Default arguments are only permitted on the first param clause");
       DefaultArgumentKind kind = getDefaultArgKind(param.DefaultArg);
       result->setDefaultArgumentKind(kind);
       result->setDefaultValue(param.DefaultArg);
+    } else if (param.hasInheritedDefaultArg) {
+      result->setDefaultArgumentKind(DefaultArgumentKind::Inherited);
     }
 
     elements.push_back(result);
diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp
@@ -1731,6 +1731,53 @@ Stmt *StmtChecker::visitBraceStmt(BraceStmt *BS) {
   return BS;
 }
 
+static Optional<unsigned>
+getParamIndex(const ParameterList *paramList, const ParamDecl *decl) {
+  ArrayRef<ParamDecl *> params = paramList->getArray();
+  for (unsigned i = 0; i < params.size(); ++i) {
+    if (params[i] == decl) return i;
+  }
+  return None;
+}
+
+static void
+checkInheritedDefaultValueRestrictions(TypeChecker &TC, ParamDecl *PD) {
+  if (PD->getDefaultArgumentKind() != DefaultArgumentKind::Inherited)
+    return;
+
+  auto *DC = PD->getInnermostDeclContext();
+  const SourceFile *SF = DC->getParentSourceFile();
+  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, diag::inherited_default_value_not_in_designated_constructor);
+    return;
+  }
+
+  // The decl it overrides should also be a designated initializer.
+  auto overridden = ctor->getOverriddenDecl();
+  if (!overridden || overridden->isConvenienceInit()) {
+    TC.diagnose(
+        PD, diag::inherited_default_value_used_in_non_overriding_constructor);
+    if (overridden)
+      TC.diagnose(overridden, diag::overridden_here);
+    return;
+  }
+
+  // The corresponding parameter should have a default value.
+  Optional<unsigned> idx = getParamIndex(ctor->getParameters(), PD);
+  assert(idx && "containing decl does not contain param?");
+  ParamDecl *equivalentParam = overridden->getParameters()->get(*idx);
+  if (equivalentParam->getDefaultArgumentKind() == DefaultArgumentKind::None) {
+    TC.diagnose(PD, diag::corresponding_param_not_defaulted);
+    TC.diagnose(equivalentParam, diag::inherited_default_param_here);
+  }
+}
+
 /// Check the default arguments that occur within this pattern.
 void TypeChecker::checkDefaultArguments(ParameterList *params,
                                         ValueDecl *VD) {
@@ -1756,6 +1803,7 @@ void TypeChecker::checkDefaultArguments(ParameterList *params,
   }
 
   for (auto *param : *params) {
+    checkInheritedDefaultValueRestrictions(*this, param);
     if (!param->getDefaultValue() ||
         !param->hasInterfaceType() ||
         param->getInterfaceType()->hasError())
diff --git a/test/ParseableInterface/default-args.swift b/test/ParseableInterface/default-args.swift
@@ -0,0 +1,51 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module -o %t/Test~partial.swiftmodule -module-name Test -primary-file %s
+// RUN: %target-swift-frontend -merge-modules -emit-module -o %t/Test.swiftmodule %t/Test~partial.swiftmodule
+// RUN: %target-swift-ide-test -print-module -module-to-print=Test -source-filename=x -I %t | %FileCheck %s
+
+// RUN: %target-swift-frontend -typecheck -emit-parseable-module-interface-path %t/Test.swiftinterface -enable-library-evolution %s
+// RUN: rm %t/Test.swiftmodule
+// RUN: echo "import Test" > %t/test-client.swift
+// RUN: %target-swift-frontend -typecheck -I%t %t/test-client.swift
+// RUN: %FileCheck %s < %t/Test.swiftinterface
+
+// CHECK: class Base {
+public class Base {
+  // CHECK: init(x: Int = 3)
+  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-NOT: init(convInit: Int = super)
+  // CHECK: override {{(public )?}}init(x: Int = super)
+  // CHECK-NOT: init(convInit: Int = super)
+}
+
+public enum Enum {
+  // CHECK: case pie(filling: String = "apple")
+  case pie(filling: String = "apple")
+}
+
+// CHECK: func hasClosureDefaultArg(_ x: () -> Void = {
+// CHECK-NEXT: })
+public func hasClosureDefaultArg(_ x: () -> Void = {
+}) {
+}
+
+// CHECK: func hasMagicDefaultArgs(_ f: String = #file, _ fu: String = #function, _ l: Int = #line)
+public func hasMagicDefaultArgs(_ f: String = #file, _ fu: String = #function, _ l: Int = #line) {}
+
+// CHECK: func hasSimpleDefaultArgs(_ x: Int = 0, b: Int = 1)
+public func hasSimpleDefaultArgs(_ x: Int = 0, b: Int = 1) {
+}
diff --git a/test/ParseableInterface/inherited-defaults-checks.swiftinterface b/test/ParseableInterface/inherited-defaults-checks.swiftinterface
@@ -0,0 +1,64 @@
+// swift-interface-format-version: 1.0
+// swift-module-flags: -module-name InheritedDefaults
+
+// RUN: %empty-directory(%t)
+// RUN: %target-typecheck-verify-swift
+
+import Swift
+
+public class Bar {
+  // 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 {
+
+  // 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 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 {
+
+  // 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, 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 {
+
+  // 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 when overriding a designated initializer}}
+}
diff --git a/test/ParseableInterface/inherited-defaults-execution.swift b/test/ParseableInterface/inherited-defaults-execution.swift
@@ -0,0 +1,47 @@
+// REQUIRES: executable_test
+// RUN: %empty-directory(%t)
+
+// 1) Build the 'Inherited' library and its interface from this file
+//
+// RUN: %target-build-swift-dylib(%t/%target-library-name(Inherited)) -emit-module-path %t/Inherited.swiftmodule -emit-parseable-module-interface-path %t/Inherited.swiftinterface -module-name Inherited %s
+// RUN: rm %t/Inherited.swiftmodule
+
+// 2) Check the interface includes the synthesized initializers of the base
+//    class in the derived class explicitly and uses the '= super' syntax to
+//    inherit its default arguments.
+//
+// RUN: cat %t/Inherited.swiftinterface | %FileCheck --check-prefix=INTERFACE %s
+//
+// INTERFACE: public class Base {
+// INTERFACE:   public init(x: Swift.Int = 45, y: Swift.Int = 98)
+// INTERFACE: }
+// INTERFACE: public class Derived : Inherited.Base {
+// INTERFACE:   override public init(x: Swift.Int = super, y: Swift.Int = super)
+// INTERFACE: }
+
+// 4) Generate a main.swift file that uses the 'Inherited' library and makes use
+//    of the inherited default arguments
+//
+// RUN: echo "import Inherited" > %t/main.swift
+// RUN: echo "print(Derived().x)" >> %t/main.swift
+// RUN: echo "print(Derived().y)" >> %t/main.swift
+
+// 5) Build and run the executable, checking the defaulted arguments resulted in
+//    the correct values being stored
+//
+// RUN: %target-build-swift -I%t -L%t -lInherited -o %t/main %target-rpath(%t) %t/main.swift -swift-version 5
+// RUN: %target-codesign %t/main %t/%target-library-name(Inherited)
+// RUN: %target-run %t/main %t/%target-library-name(Inherited) | %FileCheck --check-prefix=OUTPUT %s
+//
+// OUTPUT: 45
+// OUTPUT-NEXT: 98
+
+public class Base {
+  public let x: Int
+  public let y: Int
+  public init(x: Int = 45, y: Int = 98) {
+    self.x = x
+    self.y = y
+  }
+}
+public class Derived: Base {}
diff --git a/test/Syntax/Outputs/round_trip_module_interface.swiftinterface.withkinds b/test/Syntax/Outputs/round_trip_module_interface.swiftinterface.withkinds
@@ -0,0 +1,23 @@
+<ImportDecl>// RUN: rm -rf %t
+// RUN: %swift-syntax-test -input-source-filename %s -parse-gen > %t
+// RUN: diff -u %s %t
+// RUN: %swift-syntax-test -input-source-filename %s -parse-gen -print-node-kind > %t.withkinds
+// RUN: diff -u %S/Outputs/round_trip_module_interface.swiftinterface.withkinds %t.withkinds
+// RUN: %swift-syntax-test -input-source-filename %s -eof > %t
+// RUN: diff -u %s %t
+// RUN: %swift-syntax-test -serialize-raw-tree -input-source-filename %s > %t.dump
+// RUN: %swift-syntax-test -deserialize-raw-tree -input-source-filename %t.dump -output-filename %t
+// RUN: diff -u %s %t
+
+import <AccessPathComponent>Swift</AccessPathComponent></ImportDecl><ClassDecl><DeclModifier>
+
+public </DeclModifier>class Bar <MemberDeclBlock>{<MemberDeclListItem><InitializerDecl><DeclModifier>
+  public </DeclModifier>init<ParameterClause>(<FunctionParameter>x: <SimpleTypeIdentifier>Int </SimpleTypeIdentifier><InitializerClause>= <IntegerLiteralExpr>24</IntegerLiteralExpr></InitializerClause>, </FunctionParameter><FunctionParameter>y: <SimpleTypeIdentifier>Int</SimpleTypeIdentifier></FunctionParameter>)</ParameterClause></InitializerDecl></MemberDeclListItem>
+}</MemberDeclBlock></ClassDecl><ClassDecl><DeclModifier>
+
+public </DeclModifier>class Foo<TypeInheritanceClause>: <InheritedType><SimpleTypeIdentifier>Bar </SimpleTypeIdentifier></InheritedType></TypeInheritanceClause><MemberDeclBlock>{<MemberDeclListItem><InitializerDecl><DeclModifier>
+  public </DeclModifier><DeclModifier>override </DeclModifier>init<ParameterClause>(<FunctionParameter>x: <SimpleTypeIdentifier>Int </SimpleTypeIdentifier><InitializerClause>= <SuperRefExpr>super</SuperRefExpr></InitializerClause>, </FunctionParameter><FunctionParameter>y: <SimpleTypeIdentifier>Int</SimpleTypeIdentifier></FunctionParameter>)</ParameterClause></InitializerDecl></MemberDeclListItem><MemberDeclListItem><FunctionDecl><DeclModifier>
+  public </DeclModifier>func foo<FunctionSignature><ParameterClause>(<FunctionParameter>x: <SimpleTypeIdentifier>Int </SimpleTypeIdentifier><InitializerClause>= <SuperRefExpr>super</SuperRefExpr></InitializerClause></FunctionParameter>)</ParameterClause></FunctionSignature></FunctionDecl></MemberDeclListItem><MemberDeclListItem><FunctionDecl><DeclModifier>
+  public </DeclModifier>func foo<FunctionSignature><ParameterClause>(<FunctionParameter>y: <SimpleTypeIdentifier>Int </SimpleTypeIdentifier><InitializerClause>= <MemberAccessExpr><SuperRefExpr>super</SuperRefExpr>.init</MemberAccessExpr></InitializerClause></FunctionParameter>)</ParameterClause></FunctionSignature></FunctionDecl></MemberDeclListItem><MemberDeclListItem><FunctionDecl><DeclModifier>
+  public </DeclModifier>func foo<FunctionSignature><ParameterClause>(<FunctionParameter>z: <SimpleTypeIdentifier>Int </SimpleTypeIdentifier><InitializerClause>= <SubscriptExpr><SuperRefExpr>super</SuperRefExpr>[<FunctionCallArgument><IntegerLiteralExpr>1</IntegerLiteralExpr></FunctionCallArgument>]</SubscriptExpr></InitializerClause></FunctionParameter>)</ParameterClause></FunctionSignature></FunctionDecl></MemberDeclListItem>
+}</MemberDeclBlock></ClassDecl>
diff --git a/test/Syntax/round_trip_module_interface.swiftinterface b/test/Syntax/round_trip_module_interface.swiftinterface
@@ -0,0 +1,23 @@
+// RUN: rm -rf %t
+// RUN: %swift-syntax-test -input-source-filename %s -parse-gen > %t
+// RUN: diff -u %s %t
+// RUN: %swift-syntax-test -input-source-filename %s -parse-gen -print-node-kind > %t.withkinds
+// RUN: diff -u %S/Outputs/round_trip_module_interface.swiftinterface.withkinds %t.withkinds
+// RUN: %swift-syntax-test -input-source-filename %s -eof > %t
+// RUN: diff -u %s %t
+// RUN: %swift-syntax-test -serialize-raw-tree -input-source-filename %s > %t.dump
+// RUN: %swift-syntax-test -deserialize-raw-tree -input-source-filename %t.dump -output-filename %t
+// RUN: diff -u %s %t
+
+import Swift
+
+public class Bar {
+  public init(x: Int = 24, y: Int)
+}
+
+public class Foo: Bar {
+  public override init(x: Int = super, y: Int)
+  public func foo(x: Int = super)
+  public func foo(y: Int = super.init)
+  public func foo(z: Int = super[1])
+}
diff --git a/tools/swift-syntax-test/swift-syntax-test.cpp b/tools/swift-syntax-test/swift-syntax-test.cpp
@@ -600,6 +600,8 @@ int parseFile(
   Invocation.getLangOptions().VerifySyntaxTree = options::VerifySyntaxTree;
   Invocation.getLangOptions().RequestEvaluatorGraphVizPath = options::GraphVisPath;
   Invocation.getFrontendOptions().InputsAndOutputs.addInputFile(InputFileName);
+  if (InputFileName.endswith(".swiftinterface"))
+    Invocation.setInputKind(InputFileKind::SwiftModuleInterface);
   Invocation.setMainExecutablePath(
     llvm::sys::fs::getMainExecutable(MainExecutablePath,
       reinterpret_cast<void *>(&anchorForGetMainExecutable)));
