-
Notifications
You must be signed in to change notification settings - Fork 10.5k
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
Conversation
"illegal" is an overloaded word and can distress people who aren't familiar with the usage as being illegal with respect to the language definition, not the actual law.
@swift-ci please smoke test |
e5ef10d
to
76924b9
Compare
@swift-ci please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable, if unfortunate.
// 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. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, right, thanks.
lib/AST/Decl.cpp
Outdated
@@ -1747,7 +1747,31 @@ bool swift::conflicting(ASTContext &ctx, | |||
} | |||
|
|||
// Otherwise, the declarations conflict if the overload types are the same. | |||
return sig1Type == sig2Type; | |||
if (sig1Type == sig2Type) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: early return.
…tensions of generic types. The patch that nailed down our semantics here missed an additional case that required a compatibility hack: a property on a generic type and a same-named one in an (unconstrained) extension: struct Foo<T> { var x: Int { return 0 } } extension Foo { var x: Bool { return false } } Fixes rdar://problem/40685642.
76924b9
to
b96aeda
Compare
@swift-ci please smoke test |
The patch that nailed down our semantics here missed an additional case that
required a compatibility hack: a property on a generic type and a same-named one
in an (unconstrained) extension:
Fixes rdar://problem/40685642.