Skip to content

Commit 827ec9d

Browse files
committed
[ASTScope] Use ASTScope to find fallthrough source/dest cases.
Rather than depending on the tracking of state in switch cases to remember the case statements that are the source and destination for a `fallthrough` statement, compute them using ASTScope to find the nearest enclosing case (the source) and then find the next case in the same `switch` statement (the destination).
1 parent 2cd2be5 commit 827ec9d

File tree

5 files changed

+86
-21
lines changed

5 files changed

+86
-21
lines changed

include/swift/AST/ASTScope.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,9 @@ class ASTScopeImpl {
423423
computeIsCascadingUse(ArrayRef<const ASTScopeImpl *> history,
424424
Optional<bool> initialIsCascadingUse);
425425

426+
static std::pair<CaseStmt *, CaseStmt *>
427+
lookupFallthroughSourceAndDest(SourceFile *sourceFile, SourceLoc loc);
428+
426429
#pragma mark - - lookup- starting point
427430
private:
428431
static const ASTScopeImpl *findStartingScopeForLookup(SourceFile *,

include/swift/AST/NameLookup.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,18 @@ class ASTScope {
711711
static llvm::SmallVector<LabeledStmt *, 4>
712712
lookupLabeledStmts(SourceFile *sourceFile, SourceLoc loc);
713713

714+
/// Look for the directly enclosing case statement and the next case
715+
/// statement, which together act as the source and destination for a
716+
/// 'fallthrough' statement within a switch case.
717+
///
718+
/// \returns a pair (fallthrough source, fallthrough dest). If the location
719+
/// is not within the body of a case statement at all, the fallthrough
720+
/// source will be \c nullptr. If there is a fallthrough source that case is
721+
/// the last one, the fallthrough destination will be \c nullptr. A
722+
/// well-formed 'fallthrough' statement has both a source and destination.
723+
static std::pair<CaseStmt *, CaseStmt *>
724+
lookupFallthroughSourceAndDest(SourceFile *sourceFile, SourceLoc loc);
725+
714726
SWIFT_DEBUG_DUMP;
715727
void print(llvm::raw_ostream &) const;
716728
void dumpOneScopeMapLocation(std::pair<unsigned, unsigned>);

lib/AST/ASTScope.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ llvm::SmallVector<LabeledStmt *, 4> ASTScope::lookupLabeledStmts(
5959
return ASTScopeImpl::lookupLabeledStmts(sourceFile, loc);
6060
}
6161

62+
std::pair<CaseStmt *, CaseStmt *> ASTScope::lookupFallthroughSourceAndDest(
63+
SourceFile *sourceFile, SourceLoc loc) {
64+
return ASTScopeImpl::lookupFallthroughSourceAndDest(sourceFile, loc);
65+
}
66+
6267
#if SWIFT_COMPILER_IS_MSVC
6368
#pragma warning(push)
6469
#pragma warning(disable : 4996)

lib/AST/ASTScopeLookup.cpp

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,3 +905,55 @@ ASTScopeImpl::lookupLabeledStmts(SourceFile *sourceFile, SourceLoc loc) {
905905

906906
return labeledStmts;
907907
}
908+
909+
std::pair<CaseStmt *, CaseStmt *> ASTScopeImpl::lookupFallthroughSourceAndDest(
910+
SourceFile *sourceFile, SourceLoc loc) {
911+
// Find the innermost scope from which to start our search.
912+
auto *const fileScope = sourceFile->getScope().impl;
913+
const auto *innermost = fileScope->findInnermostEnclosingScope(loc, nullptr);
914+
ASTScopeAssert(innermost->getWasExpanded(),
915+
"If looking in a scope, it must have been expanded.");
916+
917+
// Look for the enclosing case statement and its 'switch' statement.
918+
CaseStmt *fallthroughSource = nullptr;
919+
SwitchStmt *switchStmt = nullptr;
920+
for (auto scope = innermost; scope && !scope->isLabeledStmtLookupTerminator();
921+
scope = scope->getParent().getPtrOrNull()) {
922+
// If we have a case statement, record it.
923+
auto stmt = scope->getStmtIfAny();
924+
if (!stmt) continue;
925+
926+
// If we've found the first case statement of a switch, record it as the
927+
// fallthrough source. do-catch statements don't support fallthrough.
928+
if (auto caseStmt = dyn_cast<CaseStmt>(stmt.get())) {
929+
if (!fallthroughSource &&
930+
caseStmt->getParentKind() == CaseParentKind::Switch)
931+
fallthroughSource = caseStmt;
932+
933+
continue;
934+
}
935+
936+
// If we've found the first switch statement, record it and we're done.
937+
switchStmt = dyn_cast<SwitchStmt>(stmt.get());
938+
if (switchStmt)
939+
break;
940+
}
941+
942+
// If we don't have both a fallthrough source and a switch statement
943+
// enclosing it, the 'fallthrough' statement is ill-formed.
944+
if (!fallthroughSource || !switchStmt)
945+
return { nullptr, nullptr };
946+
947+
// Find this case in the list of cases for the switch. If we don't find it
948+
// here, it means that the case isn't directly nested inside the switch, so
949+
// the case and fallthrough are both ill-formed.
950+
auto caseIter = llvm::find(switchStmt->getCases(), fallthroughSource);
951+
if (caseIter == switchStmt->getCases().end())
952+
return { nullptr, nullptr };
953+
954+
// Move along to the next case. This is the fallthrough destination.
955+
++caseIter;
956+
auto fallthroughDest = caseIter == switchStmt->getCases().end() ? nullptr
957+
: *caseIter;
958+
return { fallthroughSource, fallthroughDest };
959+
}

lib/Sema/TypeCheckStmt.cpp

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -466,9 +466,6 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
466466
/// where this is greater than one.
467467
SmallVector<LabeledStmt*, 2> ActiveLabeledStmts;
468468

469-
/// The level of 'switch' nesting. 'fallthrough' is valid only in scopes where
470-
/// this is greater than one.
471-
unsigned SwitchLevel = 0;
472469
/// The destination block for a 'fallthrough' statement. Null if the switch
473470
/// scope depth is zero or if we are checking the final 'case' of the current
474471
/// switch.
@@ -536,11 +533,9 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
536533
CaseStmt *OuterFallthroughDest;
537534
AddSwitchNest(StmtChecker &SC) : SC(SC),
538535
OuterFallthroughDest(SC.FallthroughDest) {
539-
++SC.SwitchLevel;
540536
}
541537

542538
~AddSwitchNest() {
543-
--SC.SwitchLevel;
544539
SC.FallthroughDest = OuterFallthroughDest;
545540
}
546541
};
@@ -896,33 +891,31 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
896891
}
897892

898893
Stmt *visitFallthroughStmt(FallthroughStmt *S) {
899-
auto sourceFile = DC->getParentSourceFile();
900-
bool hasAnySwitches;
894+
CaseStmt *fallthroughSource;
895+
CaseStmt *fallthroughDest;
901896
if (getASTContext().LangOpts.EnableASTScopeLookup) {
902-
auto activeLabeledStmts = ASTScope::lookupLabeledStmts(
903-
sourceFile, S->getLoc());
904-
auto numSwitches = llvm::count_if(activeLabeledStmts,
905-
[](LabeledStmt *labeledStmt) {
906-
return isa<SwitchStmt>(labeledStmt);
907-
});
908-
assert(numSwitches == SwitchLevel);
909-
hasAnySwitches = numSwitches > 0;
897+
auto sourceFile = DC->getParentSourceFile();
898+
std::tie(fallthroughSource, fallthroughDest) =
899+
ASTScope::lookupFallthroughSourceAndDest(sourceFile, S->getLoc());
900+
assert(fallthroughSource == FallthroughSource);
901+
assert(fallthroughDest == FallthroughDest);
910902
} else {
911-
hasAnySwitches = SwitchLevel > 0;
903+
fallthroughSource = FallthroughSource;
904+
fallthroughDest = FallthroughDest;
912905
}
913906

914-
if (!hasAnySwitches) {
907+
if (!fallthroughSource) {
915908
getASTContext().Diags.diagnose(S->getLoc(),
916909
diag::fallthrough_outside_switch);
917910
return nullptr;
918911
}
919-
if (!FallthroughDest) {
912+
if (!fallthroughDest) {
920913
getASTContext().Diags.diagnose(S->getLoc(),
921914
diag::fallthrough_from_last_case);
922915
return nullptr;
923916
}
924-
S->setFallthroughSource(FallthroughSource);
925-
S->setFallthroughDest(FallthroughDest);
917+
S->setFallthroughSource(fallthroughSource);
918+
S->setFallthroughDest(fallthroughDest);
926919
PreviousFallthrough = S;
927920
return S;
928921
}
@@ -1676,7 +1669,7 @@ Stmt *StmtChecker::visitBraceStmt(BraceStmt *BS) {
16761669
void TypeChecker::typeCheckASTNode(ASTNode &node, DeclContext *DC,
16771670
bool LeaveBodyUnchecked) {
16781671
StmtChecker stmtChecker(DC);
1679-
// FIXME: 'ActiveLabeledStmts', 'SwitchLevel', etc. in StmtChecker are not
1672+
// FIXME: 'ActiveLabeledStmts', etc. in StmtChecker are not
16801673
// populated. Since they don't affect "type checking", it's doesn't cause
16811674
// any issue for now. But it should be populated nonetheless.
16821675
stmtChecker.LeaveBraceStmtBodyUnchecked = LeaveBodyUnchecked;

0 commit comments

Comments
 (0)