Skip to content

Commit 742f6b2

Browse files
CodaFiCodaFi
committed
Drastically Simplify VarDecl Validation
This is an amalgam of simplifications to the way VarDecls are checked and assigned interface types. First, remove TypeCheckPattern's ability to assign the interface and contextual types for a given var decl. Instead, replace it with the notion of a "naming pattern". This is the pattern that semantically binds a given VarDecl into scope, and whose type will be used to compute the interface type. Note that not all VarDecls have a naming pattern because they may not be canonical. Second, remove VarDecl's separate contextual type member, and force the contextual type to be computed the way it always was: by mapping the interface type into the parent decl context. Third, introduce a catch-all diagnostic to properly handle the change in the way that circularity checking occurs. This is also motivated by TypeCheckPattern not being principled about which parts of the AST it chooses to invalidate, especially the parent pattern and naming patterns for a given VarDecl. Once VarDecls are invalidated along with their parent patterns, a large amount of this diagnostic churn can disappear. Unfortunately, if this isn't here, we will fail to catch a number of obviously circular cases and fail to emit a diagnostic.
1 parent dd82b63 commit 742f6b2

40 files changed

+130
-134
lines changed

‎include/swift/AST/Decl.h

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ namespace swift {
7171
class GenericTypeParamDecl;
7272
class GenericTypeParamType;
7373
class ModuleDecl;
74+
class NamedPattern;
7475
class EnumCaseDecl;
7576
class EnumElementDecl;
7677
class ParameterList;
@@ -4773,6 +4774,8 @@ enum class PropertyWrapperSynthesizedPropertyKind {
47734774

47744775
/// VarDecl - 'var' and 'let' declarations.
47754776
class VarDecl : public AbstractStorageDecl {
4777+
NamedPattern *NamingPattern = nullptr;
4778+
47764779
public:
47774780
enum class Introducer : uint8_t {
47784781
Let = 0,
@@ -4786,8 +4789,6 @@ class VarDecl : public AbstractStorageDecl {
47864789
bool issCaptureList, SourceLoc nameLoc, Identifier name,
47874790
DeclContext *dc, StorageIsMutable_t supportsMutation);
47884791

4789-
Type typeInContext;
4790-
47914792
public:
47924793
VarDecl(bool isStatic, Introducer introducer, bool isCaptureList,
47934794
SourceLoc nameLoc, Identifier name, DeclContext *dc)
@@ -4804,19 +4805,10 @@ class VarDecl : public AbstractStorageDecl {
48044805
return hasName() ? getBaseName().getIdentifier().str() : "_";
48054806
}
48064807

4807-
bool hasType() const {
4808-
// We have a type if either the type has been computed already or if
4809-
// this is a deserialized declaration with an interface type.
4810-
return !typeInContext.isNull();
4811-
}
4812-
48134808
/// Get the type of the variable within its context. If the context is generic,
48144809
/// this will use archetypes.
48154810
Type getType() const;
48164811

4817-
/// Set the type of the variable within its context.
4818-
void setType(Type t);
4819-
48204812
void markInvalid();
48214813

48224814
/// Retrieve the source range of the variable type, or an invalid range if the
@@ -4905,6 +4897,9 @@ class VarDecl : public AbstractStorageDecl {
49054897
Parent = v;
49064898
}
49074899

4900+
NamedPattern *getNamingPattern() const { return NamingPattern; }
4901+
void setNamingPattern(NamedPattern *Pat) { NamingPattern = Pat; }
4902+
49084903
/// If this is a VarDecl that does not belong to a CaseLabelItem's pattern,
49094904
/// return this. Otherwise, this VarDecl must belong to a CaseStmt's
49104905
/// CaseLabelItem. In that case, return the first case label item of the first

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1509,6 +1509,9 @@ ERROR(pattern_binds_no_variables,none,
15091509
"%select{property|global variable}0 declaration does not bind any "
15101510
"variables",
15111511
(unsigned))
1512+
ERROR(variable_bound_by_no_pattern,none,
1513+
"variable %0 is not bound by any pattern",
1514+
(DeclName))
15121515

15131516
WARNING(optional_ambiguous_case_ref,none,
15141517
"assuming you mean '%0.%2'; did you mean '%1.%2' instead?",

‎lib/AST/ASTDumper.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -725,7 +725,7 @@ namespace {
725725

726726
if (auto *var = dyn_cast<VarDecl>(VD)) {
727727
PrintWithColorRAII(OS, TypeColor) << " type='";
728-
if (var->hasType())
728+
if (auto varTy = var->hasInterfaceType())
729729
var->getType().print(PrintWithColorRAII(OS, TypeColor).getOS());
730730
else
731731
PrintWithColorRAII(OS, TypeColor) << "<null type>";
@@ -954,13 +954,11 @@ namespace {
954954
PrintWithColorRAII(OS, IdentifierColor)
955955
<< " apiName=" << P->getArgumentName();
956956

957-
if (P->hasType()) {
957+
if (P->hasInterfaceType()) {
958958
PrintWithColorRAII(OS, TypeColor) << " type='";
959959
P->getType().print(PrintWithColorRAII(OS, TypeColor).getOS());
960960
PrintWithColorRAII(OS, TypeColor) << "'";
961-
}
962961

963-
if (P->hasInterfaceType()) {
964962
PrintWithColorRAII(OS, InterfaceTypeColor) << " interface type='";
965963
P->getInterfaceType().print(
966964
PrintWithColorRAII(OS, InterfaceTypeColor).getOS());

‎lib/AST/Decl.cpp

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5007,24 +5007,11 @@ VarDecl::VarDecl(DeclKind kind, bool isStatic, VarDecl::Introducer introducer,
50075007
}
50085008

50095009
Type VarDecl::getType() const {
5010-
if (!typeInContext) {
5011-
const_cast<VarDecl *>(this)->typeInContext =
5012-
getDeclContext()->mapTypeIntoContext(
5013-
getInterfaceType());
5014-
}
5015-
5016-
return typeInContext;
5017-
}
5018-
5019-
void VarDecl::setType(Type t) {
5020-
assert(t.isNull() || !t->is<InOutType>());
5021-
typeInContext = t;
5010+
return getDeclContext()->mapTypeIntoContext(getInterfaceType());
50225011
}
50235012

50245013
void VarDecl::markInvalid() {
5025-
auto &Ctx = getASTContext();
5026-
setType(ErrorType::get(Ctx));
5027-
setInterfaceType(ErrorType::get(Ctx));
5014+
setInterfaceType(ErrorType::get(getASTContext()));
50285015
}
50295016

50305017
/// Returns whether the var is settable in the specified context: this
@@ -5269,9 +5256,6 @@ Pattern *VarDecl::getParentPattern() const {
52695256
if (pat->containsVarDecl(this))
52705257
return pat;
52715258
}
5272-
5273-
//stmt->dump();
5274-
assert(0 && "Unknown parent pattern statement?");
52755259
}
52765260

52775261
// Otherwise, check if we have to walk our case stmt's var decl list to find
@@ -5290,7 +5274,7 @@ TypeRepr *VarDecl::getTypeReprOrParentPatternTypeRepr() const {
52905274
return param->getTypeRepr();
52915275

52925276
if (auto *parentPattern = dyn_cast_or_null<TypedPattern>(getParentPattern()))
5293-
return parentPattern->getTypeLoc().getTypeRepr();
5277+
return parentPattern->getTypeRepr();
52945278

52955279
return nullptr;
52965280
}

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2110,7 +2110,6 @@ applyPropertyOwnership(VarDecl *prop,
21102110
if (attrs & clang::ObjCPropertyDecl::OBJC_PR_weak) {
21112111
prop->getAttrs().add(new (ctx)
21122112
ReferenceOwnershipAttr(ReferenceOwnership::Weak));
2113-
prop->setType(WeakStorageType::get(prop->getType(), ctx));
21142113
prop->setInterfaceType(WeakStorageType::get(
21152114
prop->getInterfaceType(), ctx));
21162115
return;
@@ -2119,7 +2118,6 @@ applyPropertyOwnership(VarDecl *prop,
21192118
(attrs & clang::ObjCPropertyDecl::OBJC_PR_unsafe_unretained)) {
21202119
prop->getAttrs().add(
21212120
new (ctx) ReferenceOwnershipAttr(ReferenceOwnership::Unmanaged));
2122-
prop->setType(UnmanagedStorageType::get(prop->getType(), ctx));
21232121
prop->setInterfaceType(UnmanagedStorageType::get(
21242122
prop->getInterfaceType(), ctx));
21252123
return;

‎lib/Sema/CSApply.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4455,7 +4455,6 @@ namespace {
44554455
SourceLoc(),
44564456
/*argument label*/ SourceLoc(), Identifier(),
44574457
/*parameter name*/ SourceLoc(), ctx.getIdentifier("$0"), closure);
4458-
param->setType(baseTy);
44594458
param->setInterfaceType(baseTy->mapTypeOutOfContext());
44604459
param->setSpecifier(ParamSpecifier::Default);
44614460

@@ -4471,7 +4470,6 @@ namespace {
44714470
/*argument label*/ SourceLoc(), Identifier(),
44724471
/*parameter name*/ SourceLoc(),
44734472
ctx.getIdentifier("$kp$"), outerClosure);
4474-
outerParam->setType(keyPathTy);
44754473
outerParam->setInterfaceType(keyPathTy->mapTypeOutOfContext());
44764474
outerParam->setSpecifier(ParamSpecifier::Default);
44774475

@@ -4650,7 +4648,6 @@ namespace {
46504648
Expr *visitTapExpr(TapExpr *E) {
46514649
auto type = simplifyType(cs.getType(E));
46524650

4653-
E->getVar()->setType(type);
46544651
E->getVar()->setInterfaceType(type->mapTypeOutOfContext());
46554652

46564653
cs.setType(E, type);

‎lib/Sema/CSDiag.cpp

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -998,7 +998,6 @@ namespace {
998998
llvm::DenseMap<Expr*, Type> ExprTypes;
999999
llvm::DenseMap<TypeLoc*, Type> TypeLocTypes;
10001000
llvm::DenseMap<Pattern*, Type> PatternTypes;
1001-
llvm::DenseMap<ParamDecl*, Type> ParamDeclTypes;
10021001
llvm::DenseMap<ParamDecl*, Type> ParamDeclInterfaceTypes;
10031002
llvm::DenseSet<ValueDecl*> PossiblyInvalidDecls;
10041003
ExprTypeSaverAndEraser(const ExprTypeSaverAndEraser&) = delete;
@@ -1045,10 +1044,6 @@ namespace {
10451044
// associated TypeLocs or resynthesized as fresh typevars.
10461045
if (auto *CE = dyn_cast<ClosureExpr>(expr))
10471046
for (auto P : *CE->getParameters()) {
1048-
if (P->hasType()) {
1049-
TS->ParamDeclTypes[P] = P->getType();
1050-
P->setType(Type());
1051-
}
10521047
if (P->hasInterfaceType()) {
10531048
TS->ParamDeclInterfaceTypes[P] = P->getInterfaceType();
10541049
P->setInterfaceType(Type());
@@ -1100,13 +1095,7 @@ namespace {
11001095

11011096
for (auto patternElt : PatternTypes)
11021097
patternElt.first->setType(patternElt.second);
1103-
1104-
for (auto paramDeclElt : ParamDeclTypes) {
1105-
assert(!paramDeclElt.first->isImmutable() ||
1106-
!paramDeclElt.second->is<InOutType>());
1107-
paramDeclElt.first->setType(paramDeclElt.second->getInOutObjectType());
1108-
}
1109-
1098+
11101099
for (auto paramDeclIfaceElt : ParamDeclInterfaceTypes) {
11111100
assert(!paramDeclIfaceElt.first->isImmutable() ||
11121101
!paramDeclIfaceElt.second->is<InOutType>());
@@ -1144,11 +1133,6 @@ namespace {
11441133
for (auto patternElt : PatternTypes)
11451134
if (!patternElt.first->hasType())
11461135
patternElt.first->setType(patternElt.second);
1147-
1148-
for (auto paramDeclElt : ParamDeclTypes)
1149-
if (!paramDeclElt.first->hasType()) {
1150-
paramDeclElt.first->setType(getParamBaseType(paramDeclElt));
1151-
}
11521136

11531137
for (auto paramDeclIfaceElt : ParamDeclInterfaceTypes)
11541138
if (!paramDeclIfaceElt.first->hasInterfaceType()) {
@@ -3762,10 +3746,9 @@ bool FailureDiagnosis::diagnoseClosureExpr(
37623746
//
37633747
// Handle this by rewriting the arguments to UnresolvedType().
37643748
for (auto VD : *CE->getParameters()) {
3765-
if (VD->hasType() && (VD->getType()->hasTypeVariable() ||
3766-
VD->getType()->hasError())) {
3767-
VD->setType(CS.getASTContext().TheUnresolvedType);
3768-
VD->setInterfaceType(VD->getType());
3749+
if (VD->hasInterfaceType() && (VD->getType()->hasTypeVariable() ||
3750+
VD->getType()->hasError())) {
3751+
VD->setInterfaceType(CS.getASTContext().TheUnresolvedType);
37693752
}
37703753
}
37713754

‎lib/Sema/DerivedConformanceCodable.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ deriveBodyEncodable_encode(AbstractFunctionDecl *encodeDecl, void *) {
584584
auto codingKeysType = codingKeysEnum->getDeclaredType();
585585
auto *containerDecl = createKeyedContainer(C, funcDC,
586586
C.getKeyedEncodingContainerDecl(),
587-
codingKeysType,
587+
codingKeysEnum->getDeclaredInterfaceType(),
588588
VarDecl::Introducer::Var);
589589

590590
auto *containerExpr = new (C) DeclRefExpr(ConcreteDeclRef(containerDecl),
@@ -806,7 +806,7 @@ deriveBodyDecodable_init(AbstractFunctionDecl *initDecl, void *) {
806806
auto codingKeysType = codingKeysEnum->getDeclaredType();
807807
auto *containerDecl = createKeyedContainer(C, funcDC,
808808
C.getKeyedDecodingContainerDecl(),
809-
codingKeysType,
809+
codingKeysEnum->getDeclaredInterfaceType(),
810810
VarDecl::Introducer::Let);
811811

812812
auto *containerExpr = new (C) DeclRefExpr(ConcreteDeclRef(containerDecl),

‎lib/Sema/DerivedConformanceEquatableHashable.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ static VarDecl *indexedVarDecl(char prefixChar, int index, Type type,
186186
/*IsCaptureList*/true, SourceLoc(),
187187
C.getIdentifier(indexStrRef),
188188
varContext);
189-
varDecl->setType(type);
189+
varDecl->setInterfaceType(type);
190190
varDecl->setHasNonPatternBindingInit(true);
191191
return varDecl;
192192
}
@@ -1225,7 +1225,7 @@ static ValueDecl *deriveHashable_hashValue(DerivedConformance &derived) {
12251225
new (C) VarDecl(/*IsStatic*/false, VarDecl::Introducer::Var,
12261226
/*IsCaptureList*/false, SourceLoc(),
12271227
C.Id_hashValue, parentDC);
1228-
hashValueDecl->setType(intType);
1228+
hashValueDecl->setInterfaceType(intType);
12291229

12301230
ParameterList *params = ParameterList::createEmpty(C);
12311231

‎lib/Sema/MiscDiagnostics.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2077,7 +2077,7 @@ class VarDeclUsageChecker : public ASTWalker {
20772077

20782078
// If the variable was invalid, ignore it and notice that the code is
20792079
// malformed.
2080-
if (VD->isInvalid() || !VD->hasType()) {
2080+
if (!VD->getInterfaceType() || VD->isInvalid()) {
20812081
sawError = true;
20822082
return false;
20832083
}

0 commit comments

Comments
 (0)