Skip to content

Swift 4 compatibility hack for redeclaration of properties in generic type and extension #17392

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 2 commits into from
Jun 22, 2018
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
2 changes: 1 addition & 1 deletion include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -635,7 +635,7 @@ ERROR(reserved_member_name,none,

ERROR(invalid_redecl,none,"invalid redeclaration of %0", (DeclName))
WARNING(invalid_redecl_swift5_warning,none,
"redeclaration of %0 is deprecated and will be illegal in Swift 5",
"redeclaration of %0 is deprecated and will be an error in Swift 5",
(DeclName))

NOTE(invalid_redecl_prev,none,
Expand Down
25 changes: 24 additions & 1 deletion lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1747,7 +1747,30 @@ bool swift::conflicting(ASTContext &ctx,
}

// Otherwise, the declarations conflict if the overload types are the same.
return sig1Type == sig2Type;
if (sig1Type != sig2Type)
return false;

// The Swift 5 overload types are the same, but similar to the above, prior to
// Swift 5, a variable not in an extension of a generic type got a null
// overload type instead of a function type as it does now, so we really
// follow that behaviour and warn if there's going to be a conflict in future.
if (!ctx.isSwiftVersionAtLeast(5)) {
auto swift4Sig1Type = sig1.IsVariable && !sig1.InExtensionOfGenericType
? CanType()
: sig1Type;
auto swift4Sig2Type = sig1.IsVariable && !sig2.InExtensionOfGenericType
? CanType()
: sig1Type;
if (swift4Sig1Type != swift4Sig2Type) {
// Old was different to the new behaviour!
if (wouldConflictInSwift5)
*wouldConflictInSwift5 = true;

return false;
}
}

return true;
}

static Type mapSignatureFunctionType(ASTContext &ctx, Type type,
Expand Down
11 changes: 11 additions & 0 deletions lib/Sema/TypeCheckDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,17 @@ static void checkRedeclaration(TypeChecker &tc, ValueDecl *current) {
continue;
}

// If both are VarDecls, and both have exactly the same type, then
// matching the Swift 4 behaviour (i.e. just emitting the future-compat
// warning) will result in SILGen crashes due to both properties mangling
// the same, so it's better to just follow the Swift 5 behaviour and emit
// the actual error.
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 add a test case for this?

Copy link
Contributor Author

@huonw huonw Jun 21, 2018

Choose a reason for hiding this comment

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

The z property in the diff for the overload.swift is this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, right, thanks.

if (wouldBeSwift5Redeclaration && isa<VarDecl>(current) &&
isa<VarDecl>(other) &&
current->getInterfaceType()->isEqual(other->getInterfaceType())) {
wouldBeSwift5Redeclaration = false;
}

// If this isn't a redeclaration in the current version of Swift, but
// would be in Swift 5 mode, emit a warning instead of an error.
if (wouldBeSwift5Redeclaration) {
Expand Down
4 changes: 2 additions & 2 deletions test/IDE/complete_value_expr.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1855,7 +1855,7 @@ func testDeDuped(_ x: dedupS) {
}
func testDeDuped2(_ x: dedupP) {
x#^PROTOCOL_EXT_DEDUP_2^#
// PROTOCOL_EXT_DEDUP_2: Begin completions, 4 items
// PROTOCOL_EXT_DEDUP_2: Begin completions, 5 items
// PROTOCOL_EXT_DEDUP_2: Decl[InstanceMethod]/CurrNominal: .foo()[#dedupP.T#]; name=foo()
// PROTOCOL_EXT_DEDUP_2: Decl[InstanceVar]/CurrNominal: .bar[#dedupP.T#]; name=bar
// PROTOCOL_EXT_DEDUP_2: Decl[Subscript]/CurrNominal: [{#Self.T#}][#Self.T#]; name=[Self.T]
Expand All @@ -1864,7 +1864,7 @@ func testDeDuped2(_ x: dedupP) {
}
func testDeDuped3<T : dedupP where T.T == Int>(_ x: T) {
x#^PROTOCOL_EXT_DEDUP_3^#
// PROTOCOL_EXT_DEDUP_3: Begin completions, 4 items
// PROTOCOL_EXT_DEDUP_3: Begin completions, 5 items
// PROTOCOL_EXT_DEDUP_3: Decl[InstanceMethod]/Super: .foo()[#Int#]; name=foo()
// PROTOCOL_EXT_DEDUP_3: Decl[InstanceVar]/Super: .bar[#Int#]; name=bar
// PROTOCOL_EXT_DEDUP_3: Decl[Subscript]/Super: [{#Self.T#}][#Self.T#]; name=[Self.T]
Expand Down
4 changes: 2 additions & 2 deletions test/decl/overload.swift
Original file line number Diff line number Diff line change
Expand Up @@ -448,8 +448,8 @@ struct SR7249<T> {
}

extension SR7249 {
var x: Int { fatalError() } // expected-error{{invalid redeclaration of 'x'}}
var y: T { fatalError() } // expected-error{{invalid redeclaration of 'y'}}
var x: Int { fatalError() } // expected-warning{{redeclaration of 'x' is deprecated and will be an error in Swift 5}}
var y: T { fatalError() } // expected-warning{{redeclaration of 'y' is deprecated and will be an error in Swift 5}}
var z: Int { fatalError() } // expected-error{{invalid redeclaration of 'z'}}
}

Expand Down
6 changes: 3 additions & 3 deletions test/decl/overload_swift4.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ extension SR7251 {
// expected-note@-1 {{previously declared here}}
// expected-note@-2 {{previously declared here}}

struct i {} // expected-warning {{redeclaration of 'i' is deprecated and will be illegal in Swift 5}}
typealias i = Int // expected-warning {{redeclaration of 'i' is deprecated and will be illegal in Swift 5}}
struct i {} // expected-warning {{redeclaration of 'i' is deprecated and will be an error in Swift 5}}
typealias i = Int // expected-warning {{redeclaration of 'i' is deprecated and will be an error in Swift 5}}

static var j: Int { return 0 } // expected-warning {{redeclaration of 'j' is deprecated and will be illegal in Swift 5}}
static var j: Int { return 0 } // expected-warning {{redeclaration of 'j' is deprecated and will be an error in Swift 5}}

struct k {} // expected-error{{invalid redeclaration of 'k'}}
}
Expand Down
11 changes: 11 additions & 0 deletions test/decl/overload_swift5.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,14 @@ extension SR7251 {
struct k {} // expected-error{{invalid redeclaration of 'k'}}
}

struct SR7249<T> {
var x: T { fatalError() } // expected-note {{previously declared}}
var y: Int // expected-note {{previously declared}}
var z: Int // expected-note {{previously declared}}
}

extension SR7249 {
var x: Int { fatalError() } // expected-error{{invalid redeclaration of 'x'}}
var y: T { fatalError() } // expected-error{{invalid redeclaration of 'y'}}
var z: Int { fatalError() } // expected-error{{invalid redeclaration of 'z'}}
}