Skip to content

Commit af47110

Browse files
varungandhi-applevarungandhi-apple
authored
Merge pull request #26690 from varungandhi-apple/vg-refactor-validateExtension
Avoid setting the extendedType before and after computing the generic signature.
2 parents 134b8ca + 7f75ac1 commit af47110

File tree

12 files changed

+107
-78
lines changed

12 files changed

+107
-78
lines changed

‎include/swift/Serialization/ModuleFormat.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
5252
/// describe what change you made. The content of this comment isn't important;
5353
/// it just ensures a conflict if two people change the module format.
5454
/// Don't worry about adhering to the 80-column limit for this line.
55-
const uint16_t SWIFTMODULE_VERSION_MINOR = 511; // ctor failability change
55+
const uint16_t SWIFTMODULE_VERSION_MINOR = 512; // extended types may be left as unbound generic types
5656

5757
using DeclIDField = BCFixed<31>;
5858

‎lib/Sema/TypeCheckDecl.cpp

Lines changed: 40 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -4451,7 +4451,7 @@ static Type formExtensionInterfaceType(
44514451

44524452
/// Check the generic parameters of an extension, recursively handling all of
44534453
/// the parameter lists within the extension.
4454-
static std::pair<GenericEnvironment *, Type>
4454+
static GenericEnvironment *
44554455
checkExtensionGenericParams(TypeChecker &tc, ExtensionDecl *ext, Type type,
44564456
GenericParamList *genericParams) {
44574457
assert(!ext->getGenericEnvironment());
@@ -4489,7 +4489,7 @@ checkExtensionGenericParams(TypeChecker &tc, ExtensionDecl *ext, Type type,
44894489
(mustInferRequirements ||
44904490
!sameTypeReqs.empty()));
44914491

4492-
return { env, extInterfaceType };
4492+
return env;
44934493
}
44944494

44954495
static bool isNonGenericTypeAliasType(Type type) {
@@ -4500,69 +4500,65 @@ static bool isNonGenericTypeAliasType(Type type) {
45004500
return false;
45014501
}
45024502

4503-
static void validateExtendedType(ExtensionDecl *ext, TypeChecker &tc) {
4504-
// If we didn't parse a type, fill in an error type and bail out.
4505-
if (!ext->getExtendedTypeLoc().getTypeRepr()) {
4503+
static Type validateExtendedType(ExtensionDecl *ext) {
4504+
auto error = [&ext]() {
45064505
ext->setInvalid();
4507-
ext->getExtendedTypeLoc().setInvalidType(tc.Context);
4508-
return;
4509-
}
4506+
return ErrorType::get(ext->getASTContext());
4507+
};
4508+
4509+
// If we didn't parse a type, fill in an error type and bail out.
4510+
if (!ext->getExtendedTypeLoc().getTypeRepr())
4511+
return error();
45104512

4511-
// Validate the extended type.
4513+
// Compute the extended type.
45124514
TypeResolutionOptions options(TypeResolverContext::ExtensionBinding);
45134515
options |= TypeResolutionFlags::AllowUnboundGenerics;
4514-
if (tc.validateType(ext->getExtendedTypeLoc(),
4515-
TypeResolution::forInterface(ext->getDeclContext()),
4516-
options)) {
4517-
ext->setInvalid();
4518-
ext->getExtendedTypeLoc().setInvalidType(tc.Context);
4519-
return;
4520-
}
4516+
auto tr = TypeResolution::forStructural(ext->getDeclContext());
4517+
auto extendedType = tr.resolveType(ext->getExtendedTypeLoc().getTypeRepr(),
4518+
options);
4519+
ext->getExtendedTypeLoc().setType(extendedType);
45214520

4522-
// Dig out the extended type.
4523-
auto extendedType = ext->getExtendedType();
4521+
if (extendedType->hasError())
4522+
return error();
45244523

45254524
// Hack to allow extending a generic typealias.
45264525
if (auto *unboundGeneric = extendedType->getAs<UnboundGenericType>()) {
45274526
if (auto *aliasDecl = dyn_cast<TypeAliasDecl>(unboundGeneric->getDecl())) {
45284527
auto extendedNominal = aliasDecl->getDeclaredInterfaceType()->getAnyNominal();
4529-
if (extendedNominal) {
4530-
extendedType = extendedNominal->getDeclaredType();
4531-
if (!isPassThroughTypealias(aliasDecl))
4532-
ext->getExtendedTypeLoc().setType(extendedType);
4533-
}
4528+
if (extendedNominal)
4529+
return isPassThroughTypealias(aliasDecl)
4530+
? extendedType
4531+
: extendedNominal->getDeclaredType();
45344532
}
45354533
}
45364534

4535+
auto &diags = ext->getASTContext().Diags;
4536+
45374537
// Cannot extend a metatype.
45384538
if (extendedType->is<AnyMetatypeType>()) {
4539-
tc.diagnose(ext->getLoc(), diag::extension_metatype, extendedType)
4540-
.highlight(ext->getExtendedTypeLoc().getSourceRange());
4541-
ext->setInvalid();
4542-
ext->getExtendedTypeLoc().setInvalidType(tc.Context);
4543-
return;
4539+
diags.diagnose(ext->getLoc(), diag::extension_metatype, extendedType)
4540+
.highlight(ext->getExtendedTypeLoc().getSourceRange());
4541+
return error();
45444542
}
45454543

45464544
// Cannot extend function types, tuple types, etc.
45474545
if (!extendedType->getAnyNominal()) {
4548-
tc.diagnose(ext->getLoc(), diag::non_nominal_extension, extendedType)
4549-
.highlight(ext->getExtendedTypeLoc().getSourceRange());
4550-
ext->setInvalid();
4551-
ext->getExtendedTypeLoc().setInvalidType(tc.Context);
4552-
return;
4546+
diags.diagnose(ext->getLoc(), diag::non_nominal_extension, extendedType)
4547+
.highlight(ext->getExtendedTypeLoc().getSourceRange());
4548+
return error();
45534549
}
45544550

45554551
// Cannot extend a bound generic type, unless it's referenced via a
45564552
// non-generic typealias type.
45574553
if (extendedType->isSpecialized() &&
45584554
!isNonGenericTypeAliasType(extendedType)) {
4559-
tc.diagnose(ext->getLoc(), diag::extension_specialization,
4560-
extendedType->getAnyNominal()->getName())
4561-
.highlight(ext->getExtendedTypeLoc().getSourceRange());
4562-
ext->setInvalid();
4563-
ext->getExtendedTypeLoc().setInvalidType(tc.Context);
4564-
return;
4555+
diags.diagnose(ext->getLoc(), diag::extension_specialization,
4556+
extendedType->getAnyNominal()->getName())
4557+
.highlight(ext->getExtendedTypeLoc().getSourceRange());
4558+
return error();
45654559
}
4560+
4561+
return extendedType;
45664562
}
45674563

45684564
void TypeChecker::validateExtension(ExtensionDecl *ext) {
@@ -4573,7 +4569,7 @@ void TypeChecker::validateExtension(ExtensionDecl *ext) {
45734569

45744570
DeclValidationRAII IBV(ext);
45754571

4576-
validateExtendedType(ext, *this);
4572+
auto extendedType = validateExtendedType(ext);
45774573

45784574
if (auto *nominal = ext->getExtendedNominal()) {
45794575
// If this extension was not already bound, it means it is either in an
@@ -4586,14 +4582,11 @@ void TypeChecker::validateExtension(ExtensionDecl *ext) {
45864582
// Validate the nominal type declaration being extended.
45874583
validateDecl(nominal);
45884584

4589-
if (auto *genericParams = ext->getGenericParams()) {
4590-
GenericEnvironment *env;
4591-
Type extendedType = ext->getExtendedType();
4592-
std::tie(env, extendedType) = checkExtensionGenericParams(
4593-
*this, ext, extendedType,
4594-
genericParams);
4585+
ext->getExtendedTypeLoc().setType(extendedType);
45954586

4596-
ext->getExtendedTypeLoc().setType(extendedType);
4587+
if (auto *genericParams = ext->getGenericParams()) {
4588+
GenericEnvironment *env =
4589+
checkExtensionGenericParams(*this, ext, extendedType, genericParams);
45974590
ext->setGenericEnvironment(env);
45984591
}
45994592
}

‎lib/Sema/TypeCheckGeneric.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ static void checkGenericParamList(TypeChecker &tc,
7676

7777
if (auto decl = owner.dc->getAsDecl()) {
7878
if (auto extDecl = dyn_cast<ExtensionDecl>(decl)) {
79-
auto extType = extDecl->getExtendedType();
79+
auto extType = extDecl->getDeclaredInterfaceType();
8080
auto extSelfType = extDecl->getSelfInterfaceType();
8181
auto reqLHSType = req.getFirstType();
8282
auto reqRHSType = req.getSecondType();

‎lib/Sema/TypeCheckType.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -536,11 +536,20 @@ Type TypeChecker::resolveTypeInContext(
536536
parentDC = parentDC->getParent()) {
537537
if (auto *ext = dyn_cast<ExtensionDecl>(parentDC)) {
538538
auto extendedType = ext->getExtendedType();
539+
if (auto *unboundGeneric = dyn_cast<UnboundGenericType>(extendedType.getPointer())) {
540+
if (auto *ugAliasDecl = dyn_cast<TypeAliasDecl>(unboundGeneric->getAnyGeneric())) {
541+
if (ugAliasDecl == aliasDecl)
542+
return resolution.mapTypeIntoContext(
543+
aliasDecl->getDeclaredInterfaceType());
544+
545+
extendedType = unboundGeneric->getParent();
546+
continue;
547+
}
548+
}
539549
if (auto *aliasType = dyn_cast<TypeAliasType>(extendedType.getPointer())) {
540-
if (aliasType->getDecl() == aliasDecl) {
550+
if (aliasType->getDecl() == aliasDecl)
541551
return resolution.mapTypeIntoContext(
542552
aliasDecl->getDeclaredInterfaceType());
543-
}
544553

545554
extendedType = aliasType->getParent();
546555
continue;

‎lib/Serialization/Deserialization.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3931,9 +3931,8 @@ class swift::DeclDeserializer {
39313931
MF.configureGenericEnvironment(extension, genericEnvID);
39323932

39333933
auto baseTy = MF.getType(baseID);
3934-
auto nominal = baseTy->getAnyNominal();
3935-
assert(!baseTy->hasUnboundGenericType());
39363934
extension->getExtendedTypeLoc().setType(baseTy);
3935+
auto nominal = extension->getExtendedNominal();
39373936

39383937
if (isImplicit)
39393938
extension->setImplicit();

‎lib/Serialization/Serialization.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2871,7 +2871,6 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
28712871

28722872
auto contextID = S.addDeclContextRef(extension->getDeclContext());
28732873
Type baseTy = extension->getExtendedType();
2874-
assert(!baseTy->hasUnboundGenericType());
28752874
assert(!baseTy->hasArchetype());
28762875

28772876
// FIXME: Use the canonical type here in order to minimize circularity

‎test/Frontend/debug-generic-signatures.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ struct NonRecur: P2 {
7373
// Conditional conformance.
7474

7575
struct Generic<T> {}
76-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=Generic<T>
76+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=Generic
7777
// CHECK-NEXT: (normal_conformance type=Generic<T> protocol=P1
7878
// CHECK-NEXT: (assoc_type req=A type=T)
7979
// CHECK-NEXT: (value req=f() witness=main.(file).Generic extension.f()@{{.*}})
@@ -86,7 +86,7 @@ extension Generic: P1 where T: P1 {
8686

8787
// Satisfying associated types with requirements with generic params
8888
class Super<T, U> {}
89-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=Super<T, U>
89+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=Super
9090
// CHECK-NEXT: (normal_conformance type=Super<T, U> protocol=P2
9191
// CHECK-NEXT: (assoc_type req=A type=T)
9292
// CHECK-NEXT: (assoc_type req=B type=T)

‎test/Generics/conditional_conformances.swift

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func takes_P2<X: P2>(_: X) {}
2121
func takes_P5<X: P5>(_: X) {}
2222

2323
struct Free<T> {}
24-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=Free<T>
24+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=Free
2525
// CHECK-NEXT: (normal_conformance type=Free<T> protocol=P2
2626
// CHECK-NEXT: conforms_to: T P1)
2727
extension Free: P2 where T: P1 {} // expected-note {{requirement from conditional conformance of 'Free<U>' to 'P2'}}
@@ -33,7 +33,7 @@ func free_bad<U>(_: U) {
3333
}
3434

3535
struct Constrained<T: P1> {}
36-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=Constrained<T>
36+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=Constrained
3737
// CHECK-NEXT: (normal_conformance type=Constrained<T> protocol=P2
3838
// CHECK-NEXT: conforms_to: T P3)
3939
extension Constrained: P2 where T: P3 {} // expected-note {{requirement from conditional conformance of 'Constrained<U>' to 'P2'}}
@@ -45,17 +45,17 @@ func constrained_bad<U: P1>(_: U) {
4545
}
4646

4747
struct RedundantSame<T: P1> {}
48-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=RedundantSame<T>
48+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=RedundantSame
4949
// CHECK-NEXT: (normal_conformance type=RedundantSame<T> protocol=P2)
5050
extension RedundantSame: P2 where T: P1 {}
5151

5252
struct RedundantSuper<T: P4> {}
53-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=RedundantSuper<T>
53+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=RedundantSuper
5454
// CHECK-NEXT: (normal_conformance type=RedundantSuper<T> protocol=P2)
5555
extension RedundantSuper: P2 where T: P1 {}
5656

5757
struct OverlappingSub<T: P1> {}
58-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=OverlappingSub<T>
58+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=OverlappingSub
5959
// CHECK-NEXT: (normal_conformance type=OverlappingSub<T> protocol=P2
6060
// CHECK-NEXT: conforms_to: T P4)
6161
extension OverlappingSub: P2 where T: P4 {} // expected-note {{requirement from conditional conformance of 'OverlappingSub<U>' to 'P2'}}
@@ -68,7 +68,7 @@ func overlapping_sub_bad<U: P1>(_: U) {
6868

6969

7070
struct SameType<T> {}
71-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=SameType<T>
71+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=SameType
7272
// CHECK-NEXT: (normal_conformance type=SameType<T> protocol=P2
7373
// CHECK-NEXT: same_type: T Int)
7474
extension SameType: P2 where T == Int {}
@@ -84,7 +84,7 @@ func same_type_bad<U>(_: U) {
8484

8585

8686
struct SameTypeGeneric<T, U> {}
87-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=SameTypeGeneric<T, U>
87+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=SameTypeGeneric
8888
// CHECK-NEXT: (normal_conformance type=SameTypeGeneric<T, U> protocol=P2
8989
// CHECK-NEXT: same_type: T U)
9090
extension SameTypeGeneric: P2 where T == U {}
@@ -108,7 +108,7 @@ func same_type_bad<U, V>(_: U, _: V) {
108108

109109

110110
struct Infer<T, U> {}
111-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=Infer<T, U>
111+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=Infer
112112
// CHECK-NEXT: (normal_conformance type=Infer<T, U> protocol=P2
113113
// CHECK-NEXT: same_type: T Constrained<U>
114114
// CHECK-NEXT: conforms_to: U P1)
@@ -126,7 +126,7 @@ func infer_bad<U: P1, V>(_: U, _: V) {
126126
}
127127

128128
struct InferRedundant<T, U: P1> {}
129-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=InferRedundant<T, U>
129+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=InferRedundant
130130
// CHECK-NEXT: (normal_conformance type=InferRedundant<T, U> protocol=P2
131131
// CHECK-NEXT: same_type: T Constrained<U>)
132132
extension InferRedundant: P2 where T == Constrained<U> {}
@@ -146,7 +146,7 @@ class C2: C1 {}
146146
class C3: C2 {}
147147

148148
struct ClassFree<T> {}
149-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=ClassFree<T>
149+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=ClassFree
150150
// CHECK-NEXT: (normal_conformance type=ClassFree<T> protocol=P2
151151
// CHECK-NEXT: superclass: T C1)
152152
extension ClassFree: P2 where T: C1 {}
@@ -159,7 +159,7 @@ func class_free_bad<U>(_: U) {
159159
}
160160

161161
struct ClassMoreSpecific<T: C1> {}
162-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=ClassMoreSpecific<T>
162+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=ClassMoreSpecific
163163
// CHECK-NEXT: (normal_conformance type=ClassMoreSpecific<T> protocol=P2
164164
// CHECK-NEXT: superclass: T C3)
165165
extension ClassMoreSpecific: P2 where T: C3 {} // expected-note {{requirement from conditional conformance of 'ClassMoreSpecific<U>' to 'P2'}}
@@ -173,7 +173,7 @@ func class_more_specific_bad<U: C1>(_: U) {
173173

174174

175175
struct ClassLessSpecific<T: C3> {}
176-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=ClassLessSpecific<T>
176+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=ClassLessSpecific
177177
// CHECK-NEXT: (normal_conformance type=ClassLessSpecific<T> protocol=P2)
178178
extension ClassLessSpecific: P2 where T: C1 {}
179179

@@ -195,11 +195,11 @@ func subclass_bad() {
195195
// Inheriting conformances:
196196

197197
struct InheritEqual<T> {}
198-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=InheritEqual<T>
198+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=InheritEqual
199199
// CHECK-NEXT: (normal_conformance type=InheritEqual<T> protocol=P2
200200
// CHECK-NEXT: conforms_to: T P1)
201201
extension InheritEqual: P2 where T: P1 {} // expected-note {{requirement from conditional conformance of 'InheritEqual<U>' to 'P2'}}
202-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=InheritEqual<T>
202+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=InheritEqual
203203
// CHECK-NEXT: (normal_conformance type=InheritEqual<T> protocol=P5
204204
// CHECK-NEXT: (normal_conformance type=InheritEqual<T> protocol=P2
205205
// CHECK-NEXT: conforms_to: T P1)
@@ -223,11 +223,11 @@ extension InheritLess: P5 {} // expected-error{{type 'T' does not conform to pro
223223

224224

225225
struct InheritMore<T> {}
226-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=InheritMore<T>
226+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=InheritMore
227227
// CHECK-NEXT: (normal_conformance type=InheritMore<T> protocol=P2
228228
// CHECK-NEXT: conforms_to: T P1)
229229
extension InheritMore: P2 where T: P1 {} // expected-note {{requirement from conditional conformance of 'InheritMore<U>' to 'P2'}}
230-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=InheritMore<T>
230+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=InheritMore
231231
// CHECK-NEXT: (normal_conformance type=InheritMore<T> protocol=P5
232232
// CHECK-NEXT: (normal_conformance type=InheritMore<T> protocol=P2
233233
// CHECK-NEXT: conforms_to: T P1)
@@ -310,12 +310,12 @@ extension TwoDisjointConformances: P2 where T == String {}
310310
// signature, meaning the stored conditional requirement is T: P1, which isn't
311311
// true in the original type's generic signature.
312312
struct RedundancyOrderDependenceGood<T: P1, U> {}
313-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=RedundancyOrderDependenceGood<T, U>
313+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=RedundancyOrderDependenceGood
314314
// CHECK-NEXT: (normal_conformance type=RedundancyOrderDependenceGood<T, U> protocol=P2
315315
// CHECK-NEXT: same_type: T U)
316316
extension RedundancyOrderDependenceGood: P2 where U: P1, T == U {}
317317
struct RedundancyOrderDependenceBad<T, U: P1> {}
318-
// CHECK-LABEL: ExtensionDecl line={{.*}} base=RedundancyOrderDependenceBad<T, U>
318+
// CHECK-LABEL: ExtensionDecl line={{.*}} base=RedundancyOrderDependenceBad
319319
// CHECK-NEXT: (normal_conformance type=RedundancyOrderDependenceBad<T, U> protocol=P2
320320
// CHECK-NEXT: conforms_to: T P1
321321
// CHECK-NEXT: same_type: T U)

‎test/Sema/Inputs/composition_extensions_usage.swift

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
struct S {}
2+
3+
extension S : P2 {}
4+
5+
extension S : P1_1 {}
6+
7+
func f() {
8+
let s = S.init()
9+
s.p1_1()
10+
}

0 commit comments

Comments
 (0)