Skip to content

Fix ParameterList::clone to handle deserialized defaults arguments. #7156

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.

Sign up for GitHub

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 1 commit into from
Feb 1, 2017
Merged

Fix ParameterList::clone to handle deserialized defaults arguments. #7156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 8 additions & 3 deletions lib/AST/Parameter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ ParameterList *ParameterList::clone(const ASTContext &C,

// Remap the ParamDecls inside of the ParameterList.
for (auto &decl : params) {
bool hadDefaultArgument =decl->getDefaultValue() != nullptr;
bool hadDefaultArgument =
decl->getDefaultArgumentKind() == DefaultArgumentKind::Normal;

decl = new (C) ParamDecl(decl);
if (options & Implicit)
Expand All @@ -102,8 +103,12 @@ ParameterList *ParameterList::clone(const ASTContext &C,
decl->setName(C.getIdentifier("argument"));

// If we're inheriting a default argument, mark it as such.
if (hadDefaultArgument && (options & Inherited)) {
decl->setDefaultArgumentKind(DefaultArgumentKind::Inherited);
// FIXME: Figure out how to clone default arguments as well.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just assert here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had that at first and then had a wave of paranoia that someone would be depending on the current behavior. Not copying over the expression is nearly the same as setting the kind to None, but not quite.

if (hadDefaultArgument) {
if (options & Inherited)
decl->setDefaultArgumentKind(DefaultArgumentKind::Inherited);
else
decl->setDefaultArgumentKind(DefaultArgumentKind::None);
}
}

Expand Down
10 changes: 10 additions & 0 deletions test/Serialization/Inputs/inherited-initializer-base.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
open class Base {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you test the generic case also just in case?

public init(_ value: Int = 0) {}
}

public protocol Initializable {
init()
}
open class GenericBase<T: Initializable> {
public init(_ value: T = T()) {}
}
54 changes: 54 additions & 0 deletions test/Serialization/inherited-initializer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// RUN: rm -rf %t && mkdir -p %t
// RUN: %target-swift-frontend -emit-module -o %t -module-name InheritedInitializerBase %S/Inputs/inherited-initializer-base.swift
// RUN: %target-swift-frontend -emit-silgen -I %t %s | %FileCheck %s

import InheritedInitializerBase

class InheritsInit : Base {}

// CHECK-LABEL: sil hidden @_TF4main10testSimpleFT_T_
func testSimple() {
// CHECK: [[INIT:%.+]] = function_ref @_TFC4main12InheritsInitCfSiS0_
// CHECK: [[DEFAULT:%.+]] = function_ref @_TIFC24InheritedInitializerBase4BasecFSiS0_A_
// CHECK: [[ARG:%.+]] = apply [[DEFAULT]]()
// CHECK: apply [[INIT]]([[ARG]], {{%.+}})
_ = InheritsInit()

// CHECK: [[INIT:%.+]] = function_ref @_TFC4main12InheritsInitCfSiS0_
// CHECK: [[VALUE:%.+]] = integer_literal $Builtin.Int2048, 5
// CHECK: [[ARG:%.+]] = apply {{%.+}}([[VALUE]], {{%.+}}) : $@convention(method) (Builtin.Int2048, @thin Int.Type) -> Int
// CHECK: apply [[INIT]]([[ARG]], {{%.+}})
_ = InheritsInit(5)
} // CHECK: end sil function '_TF4main10testSimpleFT_T_'

struct Reinitializable<T>: Initializable {
init() {}
}

class GenericSub<T: Initializable> : GenericBase<T> {}
class ModifiedGenericSub<U> : GenericBase<Reinitializable<U>> {}
class NonGenericSub : GenericBase<Reinitializable<Int>> {}

// CHECK-LABEL: sil hidden @_TF4main11testGenericFT_T_
func testGeneric() {
// CHECK: [[INIT:%.+]] = function_ref @_TFC4main10GenericSubCfxGS0_x_
// CHECK: [[TYPE:%.+]] = metatype $@thick GenericSub<Reinitializable<Int8>>.Type
// CHECK: [[DEFAULT:%.+]] = function_ref @_TIFC24InheritedInitializerBase11GenericBasecFxGS0_x_A_
// CHECK: apply [[DEFAULT]]<Reinitializable<Int8>>({{%.+}})
// CHECK: apply [[INIT]]<Reinitializable<Int8>>({{%.+}}, [[TYPE]])
_ = GenericSub<Reinitializable<Int8>>.init() // works around SR-3806

// CHECK: [[INIT:%.+]] = function_ref @_TFC4main18ModifiedGenericSubCfGVS_15Reinitializablex_GS0_x_
// CHECK: [[TYPE:%.+]] = metatype $@thick ModifiedGenericSub<Int16>.Type
// CHECK: [[DEFAULT:%.+]] = function_ref @_TIFC24InheritedInitializerBase11GenericBasecFxGS0_x_A_
// CHECK: apply [[DEFAULT]]<Reinitializable<Int16>>({{%.+}})
// CHECK: apply [[INIT]]<Int16>({{%.+}}, [[TYPE]])
_ = ModifiedGenericSub<Int16>()

// CHECK: [[INIT:%.+]] = function_ref @_TFC4main13NonGenericSubCfGVS_15ReinitializableSi_S0_
// CHECK: [[TYPE:%.+]] = metatype $@thick NonGenericSub.Type
// CHECK: [[DEFAULT:%.+]] = function_ref @_TIFC24InheritedInitializerBase11GenericBasecFxGS0_x_A_
// CHECK: apply [[DEFAULT]]<Reinitializable<Int>>({{%.+}})
// CHECK: apply [[INIT]]({{%.+}}, [[TYPE]])
_ = NonGenericSub()
} // CHECK: end sil function '_TF4main11testGenericFT_T_'