Skip to content

Commit 6c3ad10

Browse files
committed
[Statement checker] Always wire up case vars with bindSwitchCasePatternVars
bindSwitchCasePatternVars() was introduced as a simpler way to wire up the "parent" links for case variables with same-named case variables from the previous case item, and is used in the function builders code to handle switch statements. It duplicated some logic from the statement checker that did the same thing using a more complicated algorithm. Switch (ha ha) the logic in the statement checker over to using bindSwitchCasePatternVars(), fixing a bug involving unresolved patterns along the way, and remove the old code that incrementally wired up the parent links. The resulting code is simpler and is unified across the various code paths.
1 parent 09fd729 commit 6c3ad10

File tree

8 files changed

+126
-202
lines changed

8 files changed

+126
-202
lines changed

include/swift/AST/Stmt.h

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -814,13 +814,13 @@ class CaseLabelItem {
814814
Default,
815815
};
816816

817-
Pattern *CasePattern;
817+
llvm::PointerIntPair<Pattern *, 1, bool> CasePatternAndResolved;
818818
SourceLoc WhereLoc;
819819
llvm::PointerIntPair<Expr *, 1, Kind> GuardExprAndKind;
820820

821821
CaseLabelItem(Kind kind, Pattern *casePattern, SourceLoc whereLoc,
822822
Expr *guardExpr)
823-
: CasePattern(casePattern), WhereLoc(whereLoc),
823+
: CasePatternAndResolved(casePattern, false), WhereLoc(whereLoc),
824824
GuardExprAndKind(guardExpr, kind) {}
825825

826826
public:
@@ -848,9 +848,19 @@ class CaseLabelItem {
848848
SourceLoc getEndLoc() const;
849849
SourceRange getSourceRange() const;
850850

851-
Pattern *getPattern() { return CasePattern; }
852-
const Pattern *getPattern() const { return CasePattern; }
853-
void setPattern(Pattern *CasePattern) { this->CasePattern = CasePattern; }
851+
Pattern *getPattern() {
852+
return CasePatternAndResolved.getPointer();
853+
}
854+
const Pattern *getPattern() const {
855+
return CasePatternAndResolved.getPointer();
856+
}
857+
bool isPatternResolved() const {
858+
return CasePatternAndResolved.getInt();
859+
}
860+
void setPattern(Pattern *CasePattern, bool resolved) {
861+
this->CasePatternAndResolved.setPointer(CasePattern);
862+
this->CasePatternAndResolved.setInt(resolved);
863+
}
854864

855865
/// Return the guard expression if present, or null if the case label has
856866
/// no guard.

lib/AST/ASTWalker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1628,7 +1628,7 @@ Stmt *Traversal::visitSwitchStmt(SwitchStmt *S) {
16281628
Stmt *Traversal::visitCaseStmt(CaseStmt *S) {
16291629
for (auto &CLI : S->getMutableCaseLabelItems()) {
16301630
if (auto *newPattern = doIt(CLI.getPattern()))
1631-
CLI.setPattern(newPattern);
1631+
CLI.setPattern(newPattern, /*resolved=*/CLI.isPatternResolved());
16321632
else
16331633
return nullptr;
16341634
if (CLI.getGuardExpr()) {

lib/AST/Stmt.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -392,16 +392,16 @@ SourceLoc RepeatWhileStmt::getEndLoc() const { return Cond->getEndLoc(); }
392392

393393
SourceRange CaseLabelItem::getSourceRange() const {
394394
if (auto *E = getGuardExpr())
395-
return { CasePattern->getStartLoc(), E->getEndLoc() };
396-
return CasePattern->getSourceRange();
395+
return { getPattern()->getStartLoc(), E->getEndLoc() };
396+
return getPattern()->getSourceRange();
397397
}
398398
SourceLoc CaseLabelItem::getStartLoc() const {
399-
return CasePattern->getStartLoc();
399+
return getPattern()->getStartLoc();
400400
}
401401
SourceLoc CaseLabelItem::getEndLoc() const {
402402
if (auto *E = getGuardExpr())
403403
return E->getEndLoc();
404-
return CasePattern->getEndLoc();
404+
return getPattern()->getEndLoc();
405405
}
406406

407407
CaseStmt::CaseStmt(CaseParentKind parentKind, SourceLoc itemIntroducerLoc,

lib/Sema/CSApply.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8262,7 +8262,7 @@ ExprWalker::rewriteTarget(SolutionApplicationTarget target) {
82628262
target.getDeclContext());
82638263
if (auto coercedPattern = TypeChecker::coercePatternToType(
82648264
contextualPattern, patternType, patternOptions)) {
8265-
(*caseLabelItem)->setPattern(coercedPattern);
8265+
(*caseLabelItem)->setPattern(coercedPattern, /*resolved=*/true);
82668266
} else {
82678267
return None;
82688268
}
@@ -8584,7 +8584,8 @@ SolutionApplicationTarget SolutionApplicationTarget::walk(ASTWalker &walker) {
85848584
case Kind::caseLabelItem:
85858585
if (auto newPattern =
85868586
caseLabelItem.caseLabelItem->getPattern()->walk(walker)) {
8587-
caseLabelItem.caseLabelItem->setPattern(newPattern);
8587+
caseLabelItem.caseLabelItem->setPattern(
8588+
newPattern, caseLabelItem.caseLabelItem->isPatternResolved());
85888589
}
85898590
if (auto guardExpr = caseLabelItem.caseLabelItem->getGuardExpr()) {
85908591
if (auto newGuardExpr = guardExpr->walk(walker))

lib/Sema/CSGen.cpp

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2660,12 +2660,14 @@ namespace {
26602660

26612661
// Okay, resolve the pattern.
26622662
Pattern *pattern = LabelItem.getPattern();
2663-
pattern = TypeChecker::resolvePattern(pattern, CS.DC,
2664-
/*isStmtCondition*/false);
2665-
if (!pattern) return false;
2663+
if (!LabelItem.isPatternResolved()) {
2664+
pattern = TypeChecker::resolvePattern(pattern, CS.DC,
2665+
/*isStmtCondition*/false);
2666+
if (!pattern) return false;
26662667

2667-
// Save that aside while we explore the type.
2668-
LabelItem.setPattern(pattern);
2668+
// Save that aside while we explore the type.
2669+
LabelItem.setPattern(pattern, /*resolved=*/true);
2670+
}
26692671

26702672
// Require the pattern to have a particular shape: a number
26712673
// of is-patterns applied to an irrefutable pattern.
@@ -2704,7 +2706,7 @@ namespace {
27042706
if (!pattern)
27052707
return false;
27062708

2707-
LabelItem.setPattern(pattern);
2709+
LabelItem.setPattern(pattern, /*resolved=*/true);
27082710
return LabelItem.isSyntacticallyExhaustive();
27092711
}
27102712

@@ -4229,14 +4231,17 @@ bool ConstraintSystem::generateConstraints(
42294231
CaseStmt *caseStmt, DeclContext *dc, Type subjectType,
42304232
ConstraintLocator *locator) {
42314233
// Pre-bind all of the pattern variables within the case.
4232-
bindSwitchCasePatternVars(caseStmt);
4234+
bindSwitchCasePatternVars(dc, caseStmt);
42334235

42344236
for (auto &caseLabelItem : caseStmt->getMutableCaseLabelItems()) {
42354237
// Resolve the pattern.
4236-
auto *pattern = TypeChecker::resolvePattern(
4237-
caseLabelItem.getPattern(), dc, /*isStmtCondition=*/false);
4238-
if (!pattern)
4239-
return true;
4238+
auto *pattern = caseLabelItem.getPattern();
4239+
if (!caseLabelItem.isPatternResolved()) {
4240+
pattern = TypeChecker::resolvePattern(
4241+
pattern, dc, /*isStmtCondition=*/false);
4242+
if (!pattern)
4243+
return true;
4244+
}
42404245

42414246
// Generate constraints for the pattern, including one-way bindings for
42424247
// any variables that show up in this pattern, because those variables

0 commit comments

Comments
 (0)