Skip to content

[gardening] Slim down StmtChecker::visitSwitchStmt even more #23334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 15, 2019
Merged

[gardening] Slim down StmtChecker::visitSwitchStmt even more #23334

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
181ca35
[gardening] Extract out type checking of case label items into its ow…
gottesmm Mar 15, 2019
f241f10
[gardening] Extract out checking of @unknown attr from visitSwitchStm…
gottesmm Mar 15, 2019
8d3d952
[gardening] Extract out checking of fallthrough pattern bindings and …
gottesmm Mar 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
268 changes: 142 additions & 126 deletions lib/Sema/TypeCheckStmt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,142 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
return S;
}

void checkCaseLabelItem(CaseStmt *caseBlock, CaseLabelItem &labelItem,
bool &limitExhaustivityChecks, Type subjectType) {
SWIFT_DEFER {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am actually going to rip out the usage of SWIFT_DEFER here in a subsequent commit and hoist it back into visitSwitchStmt. I just don't want to stop the testing now.

// Check the guard expression, if present.
if (auto *guard = labelItem.getGuardExpr()) {
limitExhaustivityChecks |= TC.typeCheckCondition(guard, DC);
labelItem.setGuardExpr(guard);
}
};

Pattern *pattern = labelItem.getPattern();
auto *newPattern = TC.resolvePattern(pattern, DC,
/*isStmtCondition*/ false);
if (!newPattern)
return;
pattern = newPattern;
// Coerce the pattern to the subject's type.
TypeResolutionOptions patternOptions(TypeResolverContext::InExpression);
if (!subjectType ||
TC.coercePatternToType(pattern, TypeResolution::forContextual(DC),
subjectType, patternOptions)) {
limitExhaustivityChecks = true;

// If that failed, mark any variables binding pieces of the pattern
// as invalid to silence follow-on errors.
pattern->forEachVariable([&](VarDecl *VD) { VD->markInvalid(); });
}
labelItem.setPattern(pattern);

// For each variable in the pattern, make sure its type is identical to what
// it was in the first label item's pattern.
auto *firstPattern = caseBlock->getCaseLabelItems()[0].getPattern();
SmallVector<VarDecl *, 4> vars;
firstPattern->collectVariables(vars);
pattern->forEachVariable([&](VarDecl *vd) {
if (!vd->hasName())
return;
for (auto *expected : vars) {
if (expected->hasName() && expected->getName() == vd->getName()) {
if (vd->hasType() && expected->hasType() && !expected->isInvalid() &&
!vd->getType()->isEqual(expected->getType())) {
TC.diagnose(vd->getLoc(), diag::type_mismatch_multiple_pattern_list,
vd->getType(), expected->getType());
vd->markInvalid();
expected->markInvalid();
}
if (expected->isLet() != vd->isLet()) {
auto diag = TC.diagnose(
vd->getLoc(), diag::mutability_mismatch_multiple_pattern_list,
vd->isLet(), expected->isLet());

VarPattern *foundVP = nullptr;
vd->getParentPattern()->forEachNode([&](Pattern *P) {
if (auto *VP = dyn_cast<VarPattern>(P))
if (VP->getSingleVar() == vd)
foundVP = VP;
});
if (foundVP)
diag.fixItReplace(foundVP->getLoc(),
expected->isLet() ? "let" : "var");
vd->markInvalid();
expected->markInvalid();
}
return;
}
}
});
}

void checkUnknownAttrRestrictions(CaseStmt *caseBlock,
bool &limitExhaustivityChecks) {
if (caseBlock->getCaseLabelItems().size() != 1) {
assert(!caseBlock->getCaseLabelItems().empty() &&
"parser should not produce case blocks with no items");
TC.diagnose(caseBlock->getLoc(), diag::unknown_case_multiple_patterns)
.highlight(caseBlock->getCaseLabelItems()[1].getSourceRange());
limitExhaustivityChecks = true;
}

if (FallthroughDest != nullptr) {
if (!caseBlock->isDefault())
TC.diagnose(caseBlock->getLoc(), diag::unknown_case_must_be_last);
limitExhaustivityChecks = true;
}

const auto &labelItem = caseBlock->getCaseLabelItems().front();
if (labelItem.getGuardExpr() && !labelItem.isDefault()) {
TC.diagnose(labelItem.getStartLoc(), diag::unknown_case_where_clause)
.highlight(labelItem.getGuardExpr()->getSourceRange());
}

const Pattern *pattern =
labelItem.getPattern()->getSemanticsProvidingPattern();
if (!isa<AnyPattern>(pattern)) {
TC.diagnose(labelItem.getStartLoc(), diag::unknown_case_must_be_catchall)
.highlight(pattern->getSourceRange());
}
}

void checkFallthroughPatternBindingsAndTypes(CaseStmt *caseBlock,
CaseStmt *previousBlock) {
auto firstPattern = caseBlock->getCaseLabelItems()[0].getPattern();
SmallVector<VarDecl *, 4> vars;
firstPattern->collectVariables(vars);

for (auto &labelItem : previousBlock->getCaseLabelItems()) {
const Pattern *pattern = labelItem.getPattern();
SmallVector<VarDecl *, 4> PreviousVars;
pattern->collectVariables(PreviousVars);
for (auto expected : vars) {
bool matched = false;
if (!expected->hasName())
continue;
for (auto previous : PreviousVars) {
if (previous->hasName() &&
expected->getName() == previous->getName()) {
if (!previous->getType()->isEqual(expected->getType())) {
TC.diagnose(previous->getLoc(),
diag::type_mismatch_fallthrough_pattern_list,
previous->getType(), expected->getType());
previous->markInvalid();
expected->markInvalid();
}
matched = true;
break;
}
}
if (!matched) {
TC.diagnose(PreviousFallthrough->getLoc(),
diag::fallthrough_into_case_with_var_binding,
expected->getName());
}
}
}
}

Stmt *visitSwitchStmt(SwitchStmt *switchStmt) {
// Type-check the subject expression.
Expr *subjectExpr = switchStmt->getSubjectExpr();
Expand Down Expand Up @@ -979,141 +1115,21 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
FallthroughDest = std::next(i) == e ? nullptr : *std::next(i);

for (auto &labelItem : caseBlock->getMutableCaseLabelItems()) {
// Resolve the pattern in the label.
Pattern *pattern = labelItem.getPattern();
if (auto *newPattern = TC.resolvePattern(pattern, DC,
/*isStmtCondition*/false)) {
pattern = newPattern;
// Coerce the pattern to the subject's type.
TypeResolutionOptions patternOptions(TypeResolverContext::InExpression);
if (!subjectType ||
TC.coercePatternToType(pattern, TypeResolution::forContextual(DC),
subjectType, patternOptions)) {
limitExhaustivityChecks = true;

// If that failed, mark any variables binding pieces of the pattern
// as invalid to silence follow-on errors.
pattern->forEachVariable([&](VarDecl *VD) {
VD->markInvalid();
});
}
labelItem.setPattern(pattern);

// For each variable in the pattern, make sure its type is identical to what it
// was in the first label item's pattern.
auto *firstPattern = caseBlock->getCaseLabelItems()[0].getPattern();
SmallVector<VarDecl *, 4> vars;
firstPattern->collectVariables(vars);
pattern->forEachVariable([&](VarDecl *vd) {
if (!vd->hasName())
return;
for (auto *expected : vars) {
if (expected->hasName() && expected->getName() == vd->getName()) {
if (vd->hasType() && expected->hasType() &&
!expected->isInvalid() &&
!vd->getType()->isEqual(expected->getType())) {
TC.diagnose(vd->getLoc(),
diag::type_mismatch_multiple_pattern_list,
vd->getType(), expected->getType());
vd->markInvalid();
expected->markInvalid();
}
if (expected->isLet() != vd->isLet()) {
auto diag = TC.diagnose(
vd->getLoc(),
diag::mutability_mismatch_multiple_pattern_list,
vd->isLet(), expected->isLet());

VarPattern *foundVP = nullptr;
vd->getParentPattern()->forEachNode([&](Pattern *P) {
if (auto *VP = dyn_cast<VarPattern>(P))
if (VP->getSingleVar() == vd)
foundVP = VP;
});
if (foundVP)
diag.fixItReplace(foundVP->getLoc(),
expected->isLet() ? "let" : "var");
vd->markInvalid();
expected->markInvalid();
}
return;
}
}
});
}
// Check the guard expression, if present.
if (auto *guard = labelItem.getGuardExpr()) {
limitExhaustivityChecks |= TC.typeCheckCondition(guard, DC);
labelItem.setGuardExpr(guard);
}
// Resolve the pattern in our case label if it has not been resolved
// and check that our var decls follow invariants.
checkCaseLabelItem(caseBlock, labelItem, limitExhaustivityChecks,
subjectType);
}

// Check restrictions on '@unknown'.
if (caseBlock->hasUnknownAttr()) {
if (caseBlock->getCaseLabelItems().size() != 1) {
assert(!caseBlock->getCaseLabelItems().empty() &&
"parser should not produce case blocks with no items");
TC.diagnose(caseBlock->getLoc(),
diag::unknown_case_multiple_patterns)
.highlight(caseBlock->getCaseLabelItems()[1].getSourceRange());
limitExhaustivityChecks = true;
}

if (FallthroughDest != nullptr) {
if (!caseBlock->isDefault())
TC.diagnose(caseBlock->getLoc(), diag::unknown_case_must_be_last);
limitExhaustivityChecks = true;
}

const auto &labelItem = caseBlock->getCaseLabelItems().front();
if (labelItem.getGuardExpr() && !labelItem.isDefault()) {
TC.diagnose(labelItem.getStartLoc(),
diag::unknown_case_where_clause)
.highlight(labelItem.getGuardExpr()->getSourceRange());
}

const Pattern *pattern =
labelItem.getPattern()->getSemanticsProvidingPattern();
if (!isa<AnyPattern>(pattern)) {
TC.diagnose(labelItem.getStartLoc(),
diag::unknown_case_must_be_catchall)
.highlight(pattern->getSourceRange());
}
checkUnknownAttrRestrictions(caseBlock, limitExhaustivityChecks);
}

// If the previous case fellthrough, similarly check that that case's bindings
// includes our first label item's pattern bindings and types.
if (PreviousFallthrough && previousBlock) {
auto firstPattern = caseBlock->getCaseLabelItems()[0].getPattern();
SmallVector<VarDecl *, 4> vars;
firstPattern->collectVariables(vars);

for (auto &labelItem : previousBlock->getCaseLabelItems()) {
const Pattern *pattern = labelItem.getPattern();
SmallVector<VarDecl *, 4> PreviousVars;
pattern->collectVariables(PreviousVars);
for (auto expected : vars) {
bool matched = false;
if (!expected->hasName())
continue;
for (auto previous: PreviousVars) {
if (previous->hasName() && expected->getName() == previous->getName()) {
if (!previous->getType()->isEqual(expected->getType())) {
TC.diagnose(previous->getLoc(), diag::type_mismatch_fallthrough_pattern_list,
previous->getType(), expected->getType());
previous->markInvalid();
expected->markInvalid();
}
matched = true;
break;
}
}
if (!matched) {
TC.diagnose(PreviousFallthrough->getLoc(),
diag::fallthrough_into_case_with_var_binding, expected->getName());
}
}
}
checkFallthroughPatternBindingsAndTypes(caseBlock, previousBlock);
}

// Type-check the body statements.
Expand Down