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
[Sema] Explicit error for "extension GenericClass : ObjcProtocol". #15842
[Sema] Explicit error for "extension GenericClass : ObjcProtocol". #15842
Conversation
@swift-ci please smoke test |
include/swift/AST/Types.h
Outdated
/// | ||
/// \returns The closest superclass of this type that has generic parameters, | ||
/// including the type itself, or null if none exists. | ||
Type getLowestGenericClassInHierarchy(); |
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.
I like the term "closest" that you used in comments better than "lowest", because I've seen people write inheritance in both directions and wouldn't necessarily know which "lower" we're talking about.
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.
I ended up changing it to "getGenericAncestor" on @slavapestov's suggestion, since it covers slightly more than generic classes:
class Outer<T> { class Inner {} }
class ThisShouldAlsoBeFlagged: Outer<Int>.Inner {}
d8f7e7d
to
df2db66
Compare
@swift-ci please smoke test |
@swift-ci smoke test. |
@swift-ci please smoke test |
@swift-ci please smoke test. |
df2db66
to
5da5eb0
Compare
@swift-ci please smoke test. |
@swift-ci please smoke test |
1 similar comment
@swift-ci please smoke test |
lib/AST/Type.cpp
Outdated
|
||
while (t) { | ||
auto NTD = t->getAnyNominal(); | ||
assert(NTD && "expected nominal type in NTD"); |
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.
Should we be tolerant of error types here, e.g., assert(NTD || t->hasError())
?
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.
I changed the loop to while (t && !t->hasError())
.
If there was any requirements in the @objc protocol, the user got an error in some form (a complaint about those requirements), but giving the direct error is better, and handles @objc protocol with no requirements. Also, fix a hole in that existing @objc method check: in `class Outer<T> { class Inner {} }`, Inner should be considered generic, but the loop that was checking for this didn't consider it. Fixes https://bugs.swift.org/browse/SR-7370 and rdar://problem/39239512.
5da5eb0
to
d33a5c1
Compare
@swift-ci please smoke test |
1 similar comment
@swift-ci please smoke test |
If there was any requirements in the @objc protocol, the user got an error in
some form (a complaint about those requirements), but giving the direct error is
better, and handles @objc protocol with no requirements.
Fixes https://bugs.swift.org/browse/SR-7370 and rdar://problem/39239512.