Skip to content

Commit d33a5c1

Browse files
committed
[Sema] Explicit error for "extension GenericClass : ObjcProtocol".
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.
1 parent 141152c commit d33a5c1

File tree

8 files changed

+119
-31
lines changed

8 files changed

+119
-31
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1458,6 +1458,11 @@ ERROR(objc_protocol_cannot_have_conditional_conformance,none,
14581458
"type %0 cannot conditionally conform to @objc protocol %1 because "
14591459
"Objective-C does not support conditional conformances",
14601460
(Type, Type))
1461+
ERROR(objc_protocol_in_generic_extension,none,
1462+
"conformance of "
1463+
"%select{|subclass of a }2%select{class from generic context|generic class}3"
1464+
" %0 to @objc protocol %1 cannot be in an extension",
1465+
(Type, Type, bool, bool))
14611466
ERROR(conditional_conformances_cannot_imply_conformances,none,
14621467
"conditional conformance of type %0 to protocol %1 does not imply conformance to "
14631468
"inherited protocol %2",

include/swift/AST/Types.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,23 @@ class alignas(1 << TypeAlignInBits) TypeBase {
793793
/// `A<Int, NSObject>`.
794794
Type getSuperclassForDecl(const ClassDecl *classDecl);
795795

796+
/// \brief Whether this type or its superclasses has some form of generic
797+
/// context.
798+
///
799+
/// For example, given
800+
///
801+
/// class A<X> {}
802+
/// class B : A<Int> {}
803+
/// struct C<T> {
804+
/// struct Inner {}
805+
/// }
806+
/// class D {}
807+
/// class E: D {}
808+
///
809+
/// Calling hasGenericAncestry() on `B` returns `A<Int>`, on `C<T>.Inner`
810+
/// returns `C<T>.Inner`, but on `E` it returns null.
811+
Type getGenericAncestor();
812+
796813
/// \brief True if this type is the superclass of another type, or a generic
797814
/// type that could be bound to the superclass.
798815
///

lib/AST/Type.cpp

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3192,19 +3192,21 @@ const DependentMemberType *TypeBase::findUnresolvedDependentMemberType() {
31923192
return unresolvedDepMemTy;
31933193
}
31943194

3195-
3196-
Type TypeBase::getSuperclassForDecl(const ClassDecl *baseClass) {
3197-
Type t(this);
3198-
3195+
static Type getConcreteTypeForSuperclassTraversing(Type t) {
31993196
if (!t->getAnyNominal()) {
32003197
if (auto archetype = t->getAs<ArchetypeType>()) {
3201-
t = archetype->getSuperclass();
3198+
return archetype->getSuperclass();
32023199
} else if (auto dynamicSelfTy = t->getAs<DynamicSelfType>()) {
3203-
t = dynamicSelfTy->getSelfType();
3200+
return dynamicSelfTy->getSelfType();
32043201
} else if (auto compositionTy = t->getAs<ProtocolCompositionType>()) {
3205-
t = compositionTy->getExistentialLayout().superclass;
3202+
return compositionTy->getExistentialLayout().superclass;
32063203
}
32073204
}
3205+
return t;
3206+
}
3207+
3208+
Type TypeBase::getSuperclassForDecl(const ClassDecl *baseClass) {
3209+
Type t = getConcreteTypeForSuperclassTraversing(this);
32083210

32093211
while (t) {
32103212
// If we have a class-constrained archetype or class-constrained
@@ -3221,6 +3223,24 @@ Type TypeBase::getSuperclassForDecl(const ClassDecl *baseClass) {
32213223
llvm_unreachable("no inheritance relationship between given classes");
32223224
}
32233225

3226+
Type TypeBase::getGenericAncestor() {
3227+
Type t = getConcreteTypeForSuperclassTraversing(this);
3228+
3229+
while (t && !t->hasError()) {
3230+
auto NTD = t->getAnyNominal();
3231+
assert(NTD && "expected nominal type in NTD");
3232+
if (!NTD)
3233+
return Type();
3234+
3235+
if (NTD->isGenericContext())
3236+
return t;
3237+
3238+
t = t->getSuperclass();
3239+
}
3240+
3241+
return Type();
3242+
}
3243+
32243244
TypeSubstitutionMap
32253245
TypeBase::getContextSubstitutions(const DeclContext *dc,
32263246
GenericEnvironment *genericEnv) {

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1460,8 +1460,8 @@ checkIndividualConformance(NormalProtocolConformance *conformance,
14601460
return conformance;
14611461
}
14621462

1463-
// Foreign classes cannot conform to objc protocols.
14641463
if (Proto->isObjC()) {
1464+
// Foreign classes cannot conform to objc protocols.
14651465
if (auto clas = canT->getClassOrBoundGenericClass()) {
14661466
Optional<decltype(diag::cf_class_cannot_conform_to_objc_protocol)>
14671467
diagKind;
@@ -1481,6 +1481,31 @@ checkIndividualConformance(NormalProtocolConformance *conformance,
14811481
return conformance;
14821482
}
14831483
}
1484+
1485+
// @objc protocols can't be conditionally-conformed to. We could, in theory,
1486+
// front-load the requirement checking to generic-instantiation time (rather
1487+
// than conformance-lookup/construction time) and register the conformance
1488+
// with the Obj-C runtime when they're satisfied, but we'd still have solve
1489+
// the problem with extensions that we check for below.
1490+
if (!conformance->getConditionalRequirements().empty()) {
1491+
TC.diagnose(ComplainLoc,
1492+
diag::objc_protocol_cannot_have_conditional_conformance,
1493+
T, ProtoType);
1494+
conformance->setInvalid();
1495+
return conformance;
1496+
}
1497+
// And... even if it isn't conditional, we still don't currently support
1498+
// @objc protocols in extensions.
1499+
if (isa<ExtensionDecl>(DC)) {
1500+
if (auto genericT = T->getGenericAncestor()) {
1501+
auto isSubclass = !genericT->isEqual(T);
1502+
auto genericTIsGeneric = (bool)genericT->getAnyGeneric()->getGenericParams();
1503+
TC.diagnose(ComplainLoc, diag::objc_protocol_in_generic_extension, T,
1504+
ProtoType, isSubclass, genericTIsGeneric);
1505+
conformance->setInvalid();
1506+
return conformance;
1507+
}
1508+
}
14841509
}
14851510

14861511
// Not every protocol/type is compatible with conditional conformances.
@@ -1502,18 +1527,6 @@ checkIndividualConformance(NormalProtocolConformance *conformance,
15021527

15031528
nestedType = nestedType.getNominalParent();
15041529
}
1505-
1506-
// Also, the same applies for Obj-C protocols. We could, in theory,
1507-
// front-load the requirement checking to generic-instantiation time (rather
1508-
// than conformance-lookup/construction time) and register the conformance
1509-
// with the Obj-C runtime when they're satisfied.
1510-
if (Proto->isObjC()) {
1511-
TC.diagnose(ComplainLoc,
1512-
diag::objc_protocol_cannot_have_conditional_conformance,
1513-
T, ProtoType);
1514-
conformance->setInvalid();
1515-
return conformance;
1516-
}
15171530
}
15181531

15191532
// If the protocol contains missing requirements, it can't be conformed to

lib/Sema/TypeCheckType.cpp

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3400,24 +3400,18 @@ static bool checkObjCInExtensionContext(TypeChecker &tc,
34003400
return true;
34013401
}
34023402

3403-
// Check if any classes in the inheritance hierarchy have generic
3403+
// Check if any Swift classes in the inheritance hierarchy have generic
34043404
// parameters.
34053405
// FIXME: This is a current limitation, not inherent. We don't have
34063406
// a concrete class to attach Objective-C category metadata to.
3407-
Type extendedTy = ED->getDeclaredInterfaceType();
3408-
while (!extendedTy.isNull()) {
3409-
const ClassDecl *CD = extendedTy->getClassOrBoundGenericClass();
3410-
if (!CD)
3411-
break;
3412-
3413-
if (!CD->hasClangNode() && CD->getGenericParams()) {
3407+
if (auto generic = ED->getDeclaredInterfaceType()
3408+
->getGenericAncestor()) {
3409+
if (!generic->getClassOrBoundGenericClass()->hasClangNode()) {
34143410
if (diagnose) {
34153411
tc.diagnose(value, diag::objc_in_generic_extension);
34163412
}
34173413
return true;
34183414
}
3419-
3420-
extendedTy = CD->getSuperclass();
34213415
}
34223416
}
34233417

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// RUN: %target-swift-frontend -typecheck -verify %s
2+
3+
// REQUIRES: objc_interop
4+
5+
import Foundation
6+
7+
@objc protocol P {}
8+
9+
public class C1<T> {}
10+
extension C1: P {}
11+
// expected-error@-1 {{conformance of generic class 'C1<T>' to @objc protocol 'P' cannot be in an extension}}
12+
13+
public class C2<T> {}
14+
public class C3 : C2<Int> {}
15+
extension C3: P {}
16+
// expected-error@-1 {{conformance of subclass of a generic class 'C3' to @objc protocol 'P' cannot be in an extension}}
17+
18+
class Outer<T> {
19+
class Inner {}
20+
class Inner2 {}
21+
}
22+
23+
extension Outer.Inner: P {}
24+
// expected-error@-1 {{conformance of class from generic context 'Outer<T>.Inner' to @objc protocol 'P' cannot be in an extension}}
25+
26+
class SubInner: Outer<Int>.Inner2 {}
27+
28+
extension SubInner: P {}
29+
// expected-error@-1 {{conformance of subclass of a class from generic context 'SubInner' to @objc protocol 'P' cannot be in an extension}}

test/decl/ext/extension-generic-objc.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,12 @@ extension D {
4747
func d2() {}
4848
}
4949

50+
class Outer<T> {
51+
class Inner {}
52+
}
53+
54+
extension Outer.Inner {
55+
@objc func outerInner1() {}
56+
// expected-error@-1{{@objc is not supported within extensions of generic classes or classes that inherit from generic classes}}
57+
func outerInner2() {}
58+
}

validation-test/compiler_crashers_2/0122-rdar27383752.swift renamed to validation-test/compiler_crashers_2_fixed/0122-rdar27383752.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: not --crash %target-swift-frontend %s -emit-ir
1+
// RUN: %target-swift-frontend %s -typecheck -verify
22
// REQUIRES: OS=macosx
33

44
import Foundation
@@ -27,6 +27,7 @@ class Bar<T>: NSObject {
2727

2828
@available(macOS 10.12, *)
2929
extension Bar: NSFetchedResultsControllerDelegate {
30+
// expected-error@-1 {{conformance of generic class 'Bar<T>' to @objc protocol 'NSFetchedResultsControllerDelegate' cannot be in an extension}}
3031
@nonobjc func controllerWillChangeContent(_ controller: NSFetchedResultsController<NSFetchRequestResult>) {
3132
}
3233
}

0 commit comments

Comments
 (0)