Skip to content

Commit 2dbeeb0

Browse files
slavapestovslavapestov
committed
AST: Make SubstFlags::UseErrorType the default behavior
We've fixed a number of bugs recently where callers did not expect to get a null Type out of subst(). This occurs particularly often in SourceKit, where the input AST is often invalid and the types resulting from substitution are mostly used for display. Let's fix all these potential problems in one fell swoop by changing subst() to always return a Type, possibly one containing ErrorTypes. Only a couple of places depended on the old behavior, and they were easy enough to change from checking for a null Type to checking if the result responds with true to hasError(). Also while we're at it, simplify a few call sites of subst().
1 parent 1857a37 commit 2dbeeb0

36 files changed

+146
-209
lines changed

‎include/swift/AST/ProtocolConformance.h

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -152,13 +152,13 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance {
152152

153153
/// Retrieve the type witness for the given associated type.
154154
Type getTypeWitness(AssociatedTypeDecl *assocType,
155-
SubstOptions options = None) const;
155+
SubstOptions options=None) const;
156156

157157
/// Retrieve the type witness and type decl (if one exists)
158158
/// for the given associated type.
159159
std::pair<Type, TypeDecl *>
160160
getTypeWitnessAndDecl(AssociatedTypeDecl *assocType,
161-
SubstOptions options = None) const;
161+
SubstOptions options=None) const;
162162

163163
/// Apply the given function object to each type witness within this
164164
/// protocol conformance.
@@ -311,13 +311,14 @@ class alignas(1 << DeclAlignInBits) ProtocolConformance {
311311

312312
/// Substitute the conforming type and produce a ProtocolConformance that
313313
/// applies to the substituted type.
314-
ProtocolConformance *subst(SubstitutionMap subMap) const;
314+
ProtocolConformance *subst(SubstitutionMap subMap,
315+
SubstOptions options=None) const;
315316

316317
/// Substitute the conforming type and produce a ProtocolConformance that
317318
/// applies to the substituted type.
318319
ProtocolConformance *subst(TypeSubstitutionFn subs,
319320
LookupConformanceFn conformances,
320-
SubstOptions options = None) const;
321+
SubstOptions options=None) const;
321322

322323
void dump() const;
323324
void dump(llvm::raw_ostream &out, unsigned indent = 0) const;
@@ -589,7 +590,7 @@ class NormalProtocolConformance : public RootProtocolConformance,
589590
/// for the given associated type.
590591
std::pair<Type, TypeDecl *>
591592
getTypeWitnessAndDecl(AssociatedTypeDecl *assocType,
592-
SubstOptions options = None) const;
593+
SubstOptions options=None) const;
593594

594595
/// Determine whether the protocol conformance has a type witness for the
595596
/// given associated type.
@@ -714,12 +715,12 @@ class SelfProtocolConformance : public RootProtocolConformance {
714715

715716
std::pair<Type, TypeDecl *>
716717
getTypeWitnessAndDecl(AssociatedTypeDecl *assocType,
717-
SubstOptions options) const {
718+
SubstOptions options=None) const {
718719
llvm_unreachable("self-conformances never have associated types");
719720
}
720721

721722
Type getTypeWitness(AssociatedTypeDecl *assocType,
722-
SubstOptions options) const {
723+
SubstOptions options=None) const {
723724
llvm_unreachable("self-conformances never have associated types");
724725
}
725726

@@ -860,7 +861,7 @@ class SpecializedProtocolConformance : public ProtocolConformance,
860861
/// for the given associated type.
861862
std::pair<Type, TypeDecl *>
862863
getTypeWitnessAndDecl(AssociatedTypeDecl *assocType,
863-
SubstOptions options = None) const;
864+
SubstOptions options=None) const;
864865

865866
/// Given that the requirement signature of the protocol directly states
866867
/// that the given dependent type must conform to the given protocol,
@@ -972,7 +973,7 @@ class InheritedProtocolConformance : public ProtocolConformance,
972973
/// for the given associated type.
973974
std::pair<Type, TypeDecl *>
974975
getTypeWitnessAndDecl(AssociatedTypeDecl *assocType,
975-
SubstOptions options = None) const {
976+
SubstOptions options=None) const {
976977
return InheritedConformance->getTypeWitnessAndDecl(assocType, options);
977978
}
978979

‎include/swift/AST/ProtocolConformanceRef.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,14 @@ class ProtocolConformanceRef {
102102

103103
/// Apply a substitution to the conforming type.
104104
ProtocolConformanceRef subst(Type origType,
105-
SubstitutionMap subMap) const;
105+
SubstitutionMap subMap,
106+
SubstOptions options=None) const;
106107

107108
/// Apply a substitution to the conforming type.
108109
ProtocolConformanceRef subst(Type origType,
109110
TypeSubstitutionFn subs,
110111
LookupConformanceFn conformances,
111-
SubstOptions options = None) const;
112+
SubstOptions options=None) const;
112113

113114
/// Map contextual types to interface types in the conformance.
114115
ProtocolConformanceRef mapConformanceOutOfContext() const;

‎include/swift/AST/Requirement.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,15 +94,15 @@ class Requirement {
9494
template <typename... Args>
9595
Optional<Requirement> subst(Args &&... args) const {
9696
auto newFirst = getFirstType().subst(std::forward<Args>(args)...);
97-
if (!newFirst)
97+
if (newFirst->hasError())
9898
return None;
9999

100100
switch (getKind()) {
101101
case RequirementKind::Conformance:
102102
case RequirementKind::Superclass:
103103
case RequirementKind::SameType: {
104104
auto newSecond = getSecondType().subst(std::forward<Args>(args)...);
105-
if (!newSecond)
105+
if (newSecond->hasError())
106106
return None;
107107
return Requirement(getKind(), newFirst, newSecond);
108108
}

‎include/swift/AST/SubstitutionMap.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,13 +167,14 @@ class SubstitutionMap {
167167

168168
/// Apply a substitution to all replacement types in the map. Does not
169169
/// change keys.
170-
SubstitutionMap subst(SubstitutionMap subMap) const;
170+
SubstitutionMap subst(SubstitutionMap subMap,
171+
SubstOptions options=None) const;
171172

172173
/// Apply a substitution to all replacement types in the map. Does not
173174
/// change keys.
174175
SubstitutionMap subst(TypeSubstitutionFn subs,
175176
LookupConformanceFn conformances,
176-
SubstOptions options = None) const;
177+
SubstOptions options=None) const;
177178

178179
/// Create a substitution map for a protocol conformance.
179180
static SubstitutionMap

‎include/swift/AST/Type.h

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -136,19 +136,16 @@ class LookUpConformanceInSignature {
136136

137137
/// Flags that can be passed when substituting into a type.
138138
enum class SubstFlags {
139-
/// If a type cannot be produced because some member type is
140-
/// missing, place an 'error' type into the position of the base.
141-
UseErrorType = 0x01,
142139
/// Allow substitutions to recurse into SILFunctionTypes.
143140
/// Normally, SILType::subst() should be used for lowered
144141
/// types, however in special cases where the substitution
145142
/// is just changing between contextual and interface type
146143
/// representations, using Type::subst() is allowed.
147-
AllowLoweredTypes = 0x02,
144+
AllowLoweredTypes = 0x01,
148145
/// Map member types to their desugared witness type.
149-
DesugarMemberTypes = 0x04,
146+
DesugarMemberTypes = 0x02,
150147
/// Substitute types involving opaque type archetypes.
151-
SubstituteOpaqueArchetypes = 0x08,
148+
SubstituteOpaqueArchetypes = 0x04,
152149
};
153150

154151
/// Options for performing substitutions into a type.
@@ -295,7 +292,7 @@ class Type {
295292
///
296293
/// \returns the substituted type, or a null type if an error occurred.
297294
Type subst(SubstitutionMap substitutions,
298-
SubstOptions options = None) const;
295+
SubstOptions options=None) const;
299296

300297
/// Replace references to substitutable types with new, concrete types and
301298
/// return the substituted result.
@@ -310,7 +307,7 @@ class Type {
310307
/// \returns the substituted type, or a null type if an error occurred.
311308
Type subst(TypeSubstitutionFn substitutions,
312309
LookupConformanceFn conformances,
313-
SubstOptions options = None) const;
310+
SubstOptions options=None) const;
314311

315312
/// Replace references to substitutable types with error types.
316313
Type substDependentTypesWithErrorTypes() const;

‎include/swift/AST/Types.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2818,7 +2818,7 @@ class AnyFunctionType : public TypeBase {
28182818
return Param(getType(), Identifier(), getFlags().asParamFlags());
28192819
}
28202820

2821-
Yield subst(SubstitutionMap subs, SubstOptions options = None) const {
2821+
Yield subst(SubstitutionMap subs, SubstOptions options=None) const {
28222822
return Yield(getType().subst(subs, options), getFlags());
28232823
}
28242824

@@ -2839,7 +2839,7 @@ class AnyFunctionType : public TypeBase {
28392839
CanType getType() const { return CanType(Yield::getType()); }
28402840
CanParam asParam() const { return CanParam::getFromParam(Yield::asParam());}
28412841

2842-
CanYield subst(SubstitutionMap subs, SubstOptions options = None) const {
2842+
CanYield subst(SubstitutionMap subs, SubstOptions options=None) const {
28432843
return CanYield(getType().subst(subs, options)->getCanonicalType(),
28442844
getFlags());
28452845
}

‎lib/AST/ASTContext.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2227,14 +2227,11 @@ TypeAliasType *TypeAliasType::get(TypeAliasDecl *typealias, Type parent,
22272227
if (parent->hasTypeVariable())
22282228
storedProperties |= RecursiveTypeProperties::HasTypeVariable;
22292229
}
2230-
auto genericSig = substitutions.getGenericSignature();
2231-
if (genericSig) {
2232-
for (Type gp : genericSig->getGenericParams()) {
2233-
auto substGP = gp.subst(substitutions, SubstFlags::UseErrorType);
2234-
properties |= substGP->getRecursiveProperties();
2235-
if (substGP->hasTypeVariable())
2236-
storedProperties |= RecursiveTypeProperties::HasTypeVariable;
2237-
}
2230+
2231+
for (auto substGP : substitutions.getReplacementTypes()) {
2232+
properties |= substGP->getRecursiveProperties();
2233+
if (substGP->hasTypeVariable())
2234+
storedProperties |= RecursiveTypeProperties::HasTypeVariable;
22382235
}
22392236

22402237
// Figure out which arena this type will go into.
@@ -2252,6 +2249,7 @@ TypeAliasType *TypeAliasType::get(TypeAliasDecl *typealias, Type parent,
22522249
return result;
22532250

22542251
// Build a new type.
2252+
auto *genericSig = typealias->getGenericSignature();
22552253
auto size = totalSizeToAlloc<Type, SubstitutionMap>(parent ? 1 : 0,
22562254
genericSig ? 1 : 0);
22572255
auto mem = ctx.Allocate(size, alignof(TypeAliasType), arena);

‎lib/AST/ASTDemangler.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -198,9 +198,6 @@ Type ASTBuilder::createTypeAliasType(GenericTypeDecl *decl, Type parent) {
198198

199199
// FIXME: subst() should build the sugar for us
200200
declaredType = declaredType.subst(subs);
201-
if (!declaredType)
202-
return Type();
203-
204201
return TypeAliasType::get(aliasDecl, parent, subs, declaredType);
205202
}
206203

@@ -251,8 +248,7 @@ Type ASTBuilder::createBoundGenericType(GenericTypeDecl *decl,
251248

252249
// FIXME: We're not checking that the type satisfies the generic
253250
// requirements of the signature here.
254-
auto substType = origType.subst(subs);
255-
return substType;
251+
return origType.subst(subs);
256252
}
257253

258254
Type ASTBuilder::resolveOpaqueType(NodePointer opaqueDescriptor,
@@ -335,9 +331,6 @@ Type ASTBuilder::createBoundGenericType(GenericTypeDecl *decl,
335331

336332
// FIXME: subst() should build the sugar for us
337333
auto declaredType = aliasDecl->getDeclaredInterfaceType().subst(subMap);
338-
if (!declaredType)
339-
return Type();
340-
341334
return TypeAliasType::get(aliasDecl, parent, subMap, declaredType);
342335
}
343336

‎lib/AST/ASTPrinter.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -672,8 +672,7 @@ class PrintAST : public ASTVisitor<PrintAST> {
672672
M, cast<ValueDecl>(Current));
673673
}
674674

675-
T = T.subst(subMap,
676-
SubstFlags::DesugarMemberTypes | SubstFlags::UseErrorType);
675+
T = T.subst(subMap, SubstFlags::DesugarMemberTypes);
677676
}
678677

679678
printTypeWithOptions(T, options);
@@ -1447,10 +1446,12 @@ void PrintAST::printSingleDepthOfGenericSignature(
14471446
second = req.getSecondType();
14481447

14491448
if (!subMap.empty()) {
1450-
if (Type subFirst = substParam(first))
1449+
Type subFirst = substParam(first);
1450+
if (!subFirst->hasError())
14511451
first = subFirst;
14521452
if (second) {
1453-
if (Type subSecond = substParam(second))
1453+
Type subSecond = substParam(second);
1454+
if (!subSecond->hasError())
14541455
second = subSecond;
14551456
if (!(first->is<ArchetypeType>() || first->isTypeParameter()) &&
14561457
!(second->is<ArchetypeType>() || second->isTypeParameter()))

‎lib/AST/GenericEnvironment.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,7 @@ Type GenericEnvironment::mapTypeIntoContext(
180180

181181
Type result = type.subst(QueryInterfaceTypeSubstitutions(this),
182182
lookupConformance,
183-
(SubstFlags::AllowLoweredTypes|
184-
SubstFlags::UseErrorType));
183+
SubstFlags::AllowLoweredTypes);
185184
assert((!result->hasTypeParameter() || result->hasError()) &&
186185
"not fully substituted");
187186
return result;

‎lib/AST/GenericSignatureBuilder.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3817,7 +3817,7 @@ static Type substituteConcreteType(GenericSignatureBuilder &builder,
38173817
subMap = SubstitutionMap::getProtocolSubstitutions(
38183818
proto, parentType, ProtocolConformanceRef(proto));
38193819

3820-
type = type.subst(subMap, SubstFlags::UseErrorType);
3820+
type = type.subst(subMap);
38213821
} else {
38223822
// Substitute in the superclass type.
38233823
auto parentPA = basePA->getEquivalenceClassIfPresent();
@@ -3827,7 +3827,7 @@ static Type substituteConcreteType(GenericSignatureBuilder &builder,
38273827

38283828
subMap = parentType->getMemberSubstitutionMap(parentDecl->getParentModule(),
38293829
concreteDecl);
3830-
type = type.subst(subMap, SubstFlags::UseErrorType);
3830+
type = type.subst(subMap);
38313831
}
38323832

38333833
// If we had a typealias, form a sugared type.
@@ -5187,7 +5187,7 @@ GenericSignatureBuilder::addRequirement(const Requirement &req,
51875187
// Local substitution for types in the requirement.
51885188
auto subst = [&](Type t) {
51895189
if (subMap)
5190-
return t.subst(*subMap, SubstFlags::UseErrorType);
5190+
return t.subst(*subMap);
51915191

51925192
return t;
51935193
};

0 commit comments

Comments
 (0)