Skip to content

Redo Exhaustiveness Analysis #8908

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
Apr 29, 2017
Merged

Redo Exhaustiveness Analysis #8908

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
42c5955
Introduce the Space Engine
CodaFi Apr 4, 2017
39494b2
Rearrange test code for exhaustiveness
CodaFi Apr 21, 2017
1dfab98
Look through parens to the semantic pattern below
CodaFi Apr 28, 2017
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
5 changes: 0 additions & 5 deletions include/swift/AST/ASTPrinter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,11 +322,6 @@ uint8_t getKeywordLen(tok keyword);
/// Get <#code#>;
StringRef getCodePlaceholder();

/// Given an array of enum element decls, print them as case statements with
/// placeholders as contents.
void printEnumElementsAsCases(llvm::DenseSet<EnumElementDecl*> &UnhandledElements,
llvm::raw_ostream &OS);

} // namespace swift

#endif // LLVM_SWIFT_AST_ASTPRINTER_H
23 changes: 15 additions & 8 deletions include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2750,14 +2750,6 @@ ERROR(trailing_closure_requires_parens,none,
"trailing closure requires parentheses for disambiguation in this"
" context", ())

ERROR(empty_switch_stmt,none,
"%select{'switch' statement body must have at least one 'case' or 'default' block; |}0"
"do you want to add "
"%select{missing cases?|a default case?}1",(bool, bool))
ERROR(non_exhaustive_switch,none,
"%select{switch must be exhaustive, consider adding |do you want to add }0"
"%select{missing cases|a default clause}1"
"%select{|?}0", (bool, bool))
//------------------------------------------------------------------------------
// Type Check Patterns
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -3591,6 +3583,21 @@ WARNING(debug_long_closure_body, none,
"closure took %0ms to type-check (limit: %1ms)",
(unsigned, unsigned))

//------------------------------------------------------------------------------
// Pattern match diagnostics
//------------------------------------------------------------------------------


ERROR(empty_switch_stmt,none,
"'switch' statement body must have at least one 'case' or 'default' "
"block; do you want to add a default case?",())
ERROR(non_exhaustive_switch,none,
"%select{switch must be exhaustive, consider adding |do you want to add }0"
"%select{missing cases|a default clause}1"
"%select{:|?}0 %2", (bool, bool, StringRef))
NOTE(missing_particular_case,none,
"missing case: '%0'", (StringRef))

#ifndef DIAG_NO_UNDEF
# if defined(DIAG)
# undef DIAG
Expand Down
1 change: 0 additions & 1 deletion include/swift/AST/DiagnosticsSema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ namespace swift {
extern detail::DiagWithArguments<void Signature>::type ID;
#include "DiagnosticsSema.def"
}
void diagnoseMissingCases(ASTContext &Context, const SwitchStmt *SwitchS);
}

#endif
54 changes: 0 additions & 54 deletions lib/AST/ASTPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,60 +611,6 @@ uint8_t swift::getKeywordLen(tok keyword) {

StringRef swift::getCodePlaceholder() { return "<#code#>"; }

void swift::
printEnumElementsAsCases(llvm::DenseSet<EnumElementDecl*> &UnhandledElements,
llvm::raw_ostream &OS) {
// Sort the missing elements to a vector because set does not guarantee orders.
SmallVector<EnumElementDecl*, 4> SortedElements;
SortedElements.insert(SortedElements.begin(), UnhandledElements.begin(),
UnhandledElements.end());
std::sort(SortedElements.begin(),SortedElements.end(),
[](EnumElementDecl *LHS, EnumElementDecl *RHS) {
return LHS->getNameStr().compare(RHS->getNameStr()) < 0;
});

auto printPayloads = [](EnumElementDecl *EE, llvm::raw_ostream &OS) {
// If the enum element has no payloads, return.
auto TL = EE->getArgumentTypeLoc();
if (TL.isNull())
return;
TypeRepr* TR = EE->getArgumentTypeLoc().getTypeRepr();
if (auto *TTR = dyn_cast<TupleTypeRepr>(TR)) {
SmallVector<Identifier, 4> NameBuffer;
ArrayRef<Identifier> Names;
if (TTR->hasElementNames()) {
// Get the name from the tuple repr, if exist.
Names = TTR->getElementNames();
} else {
// Create same amount of empty names to the elements.
NameBuffer.resize(TTR->getNumElements());
Names = llvm::makeArrayRef(NameBuffer);
}
OS << "(";
// Print each element in the pattern match.
for (unsigned I = 0, N = Names.size(); I < N; I ++) {
auto Id = Names[I];
if (Id.empty())
OS << "_";
else
OS << tok::kw_let << " " << Id.str();
if (I + 1 != N) {
OS << ", ";
}
}
OS << ")";
}
};

// Print each enum element name.
std::for_each(SortedElements.begin(), SortedElements.end(),
[&](EnumElementDecl *EE) {
OS << tok::kw_case << " ." << EE->getNameStr();
printPayloads(EE, OS);
OS <<": " << getCodePlaceholder() << "\n";
});
}

ASTPrinter &operator<<(ASTPrinter &printer, tok keyword) {
SmallString<16> Buffer;
llvm::raw_svector_ostream OS(Buffer);
Expand Down
6 changes: 0 additions & 6 deletions lib/SILOptimizer/Mandatory/DataflowDiagnostics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,6 @@ static void diagnoseUnreachable(const SILInstruction *I,
return;
}

// A non-exhaustive switch would also produce an unreachable instruction.
if (L.isASTNode<SwitchStmt>()) {
diagnoseMissingCases(Context, L.getAsASTNode<SwitchStmt>());
return;
}

if (auto *Guard = L.getAsASTNode<GuardStmt>()) {
diagnose(Context, Guard->getBody()->getEndLoc(),
diag::guard_body_must_not_fallthrough);
Expand Down
1 change: 1 addition & 0 deletions lib/Sema/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ add_swift_library(swiftSema STATIC
TypeCheckREPL.cpp
TypeCheckRequest.cpp
TypeCheckStmt.cpp
TypeCheckSwitchStmt.cpp
TypeCheckType.cpp
TypeChecker.cpp
LINK_LIBRARIES
Expand Down
95 changes: 13 additions & 82 deletions lib/Sema/TypeCheckStmt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,80 +197,6 @@ namespace {
};
} // end anonymous namespace

/// If the pattern handles an enum element, remove the enum element from the
/// given set. If seeing uncertain patterns, e.g. any pattern, return true;
/// otherwise return false.
static bool
removedHandledEnumElements(Pattern *P,
llvm::DenseSet<EnumElementDecl*> &UnhandledElements) {
if (auto *EEP = dyn_cast<EnumElementPattern>(P)) {
UnhandledElements.erase(EEP->getElementDecl());
} else if (auto *VP = dyn_cast<VarPattern>(P)) {
return removedHandledEnumElements(VP->getSubPattern(), UnhandledElements);
} else {
return true;
}
return false;
};

void swift::
diagnoseMissingCases(ASTContext &Context, const SwitchStmt *SwitchS) {
bool Empty = SwitchS->getCases().empty();
SourceLoc StartLoc = SwitchS->getStartLoc();
SourceLoc EndLoc = SwitchS->getEndLoc();
StringRef Placeholder = getCodePlaceholder();
SmallString<32> Buffer;
llvm::raw_svector_ostream OS(Buffer);

bool InEditor = Context.LangOpts.DiagnosticsEditorMode;

auto DefaultDiag = [&]() {
OS << tok::kw_default << ": " << Placeholder << "\n";
Context.Diags.diagnose(StartLoc, Empty ? diag::empty_switch_stmt :
diag::non_exhaustive_switch, InEditor, true).fixItInsert(EndLoc,
Buffer.str());
};
// To find the subject enum decl for this switch statement.
EnumDecl *SubjectED = nullptr;
if (auto SubjectTy = SwitchS->getSubjectExpr()->getType()) {
if (auto *ND = SubjectTy->getAnyNominal()) {
SubjectED = ND->getAsEnumOrEnumExtensionContext();
}
}

// The switch is not about an enum decl, add "default:" instead.
if (!SubjectED) {
DefaultDiag();
return;
}

// Assume enum elements are not handled in the switch statement.
llvm::DenseSet<EnumElementDecl*> UnhandledElements;

SubjectED->getAllElements(UnhandledElements);
for (auto Current : SwitchS->getCases()) {
// For each handled enum element, remove it from the bucket.
for (auto Item : Current->getCaseLabelItems()) {
if (removedHandledEnumElements(Item.getPattern(), UnhandledElements)) {
DefaultDiag();
return;
}
}
}

// No unhandled cases, so we are not sure the exact case to add, fall back to
// default instead.
if (UnhandledElements.empty()) {
DefaultDiag();
return;
}

printEnumElementsAsCases(UnhandledElements, OS);
Context.Diags.diagnose(StartLoc, Empty ? diag::empty_switch_stmt :
diag::non_exhaustive_switch, InEditor, false).fixItInsert(EndLoc,
Buffer.str());
}

static void setAutoClosureDiscriminators(DeclContext *DC, Stmt *S) {
S->walk(ContextualizeClosures(DC));
}
Expand Down Expand Up @@ -928,9 +854,11 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
}

Stmt *visitSwitchStmt(SwitchStmt *S) {
bool hadError = false;

// Type-check the subject expression.
Expr *subjectExpr = S->getSubjectExpr();
TC.typeCheckExpression(subjectExpr, DC);
hadError |= TC.typeCheckExpression(subjectExpr, DC);
if (Expr *newSubjectExpr = TC.coerceToMaterializable(subjectExpr))
subjectExpr = newSubjectExpr;
S->setSubjectExpr(subjectExpr);
Expand All @@ -940,9 +868,6 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
AddSwitchNest switchNest(*this);
AddLabeledStmt labelNest(*this, S);

// Reject switch statements with empty blocks.
if (S->getCases().empty())
diagnoseMissingCases(TC.Context, S);
for (unsigned i = 0, e = S->getCases().size(); i < e; ++i) {
auto *caseBlock = S->getCases()[i];
// Fallthrough transfers control to the next case block. In the
Expand All @@ -958,6 +883,8 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
// Coerce the pattern to the subject's type.
if (TC.coercePatternToType(pattern, DC, subjectType,
TR_InExpression)) {
hadError = true;

// If that failed, mark any variables binding pieces of the pattern
// as invalid to silence follow-on errors.
pattern->forEachVariable([&](VarDecl *VD) {
Expand Down Expand Up @@ -992,17 +919,21 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {

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

// Type-check the body statements.
Stmt *body = caseBlock->getBody();
typeCheckStmt(body);
hadError |= typeCheckStmt(body);
caseBlock->setBody(body);
}


if (!S->isImplicit()) {
TC.checkSwitchExhaustiveness(S, /*limitChecking*/hadError);
}

return S;
}

Expand Down Expand Up @@ -1057,8 +988,8 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
// There is nothing more to do.
return S;
}

};

} // end anonymous namespace

bool TypeChecker::typeCheckCatchPattern(CatchStmt *S, DeclContext *DC) {
Expand Down
Loading