Skip to content

[ParseableInterfaces] Support inheriting default arguments in module interfaces via '= super' #24073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1281,6 +1281,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", ())
Expand Down
3 changes: 3 additions & 0 deletions include/swift/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,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;
Expand Down
5 changes: 1 addition & 4 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5403,10 +5403,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";
Expand Down
42 changes: 34 additions & 8 deletions lib/Parse/ParsePattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -365,7 +384,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.
Expand Down Expand Up @@ -603,16 +624,21 @@ mapParsedParameters(Parser &parser,
}
}

assert (((!param.DefaultArg &&
!param.hasInheritedDefaultArg) ||
paramContext == Parser::ParameterContextKind::Function ||
paramContext == Parser::ParameterContextKind::Operator ||
paramContext == Parser::ParameterContextKind::Initializer ||
paramContext == Parser::ParameterContextKind::EnumElement ||
paramContext == Parser::ParameterContextKind::Subscript) &&
"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 ||
paramContext == Parser::ParameterContextKind::Subscript) &&
"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);
Expand Down
48 changes: 48 additions & 0 deletions lib/Sema/TypeCheckStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1737,10 +1737,58 @@ 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) {
for (auto *param : *params) {
checkInheritedDefaultValueRestrictions(*this, param);
if (!param->getDefaultValue() ||
!param->hasInterfaceType() ||
param->getInterfaceType()->hasError())
Expand Down
30 changes: 28 additions & 2 deletions test/ParseableInterface/default-args.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,34 @@
// 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.swiftinterface -enable-library-evolution %s
// RUN: %FileCheck %s < %t.swiftinterface
// 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")
Expand Down
65 changes: 65 additions & 0 deletions test/ParseableInterface/inherited-defaults-checks.swiftinterface
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// 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 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 {

// 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}}
}
47 changes: 47 additions & 0 deletions test/ParseableInterface/inherited-defaults-execution.swift
Original file line number Diff line number Diff line change
@@ -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 | %FileCheck --check-prefix=OUTPUT %s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, look like this broke device testing. In order to make sure libInherited.dylib gets copied to the device, you have to pass it as an argument to %t/main. (That's the hack we came up with, anyway: pretend it's an input.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you're way ahead of me. :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wasn't sure what I was missing, but @harlanhaskins helped me out!

//
// 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 {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<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><SubscriptDecl><DeclModifier>
public </DeclModifier>subscript<ParameterClause>(<FunctionParameter>k: <SimpleTypeIdentifier>Int </SimpleTypeIdentifier><InitializerClause>= <SuperRefExpr>super</SuperRefExpr></InitializerClause></FunctionParameter>) </ParameterClause><ReturnClause>-> <SimpleTypeIdentifier>Int </SimpleTypeIdentifier></ReturnClause><AccessorBlock>{ <AccessorDecl>get </AccessorDecl>}</AccessorBlock></SubscriptDecl></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>
24 changes: 24 additions & 0 deletions test/Syntax/round_trip_module_interface.swiftinterface
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// 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 subscript(k: Int = super) -> Int { get }
public func foo(x: Int = super)
public func foo(y: Int = super.init)
public func foo(z: Int = super[1])
}
2 changes: 2 additions & 0 deletions tools/swift-syntax-test/swift-syntax-test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand Down