From 8311f2b0503b4c1be455c3c685577f91ded508cb Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Thu, 28 Mar 2019 14:36:00 -0700 Subject: [PATCH 1/3] [ast] Add helpers for grabbing various case stmt corresponding var decls to a specific case stmt var decl. --- include/swift/AST/Decl.h | 25 ++++++++++++++++ include/swift/Basic/NullablePtr.h | 8 +++-- lib/AST/Decl.cpp | 49 +++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 2 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 36b5f38621dbb..413c3081a9d63 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -36,6 +36,7 @@ #include "swift/Basic/ArrayRefView.h" #include "swift/Basic/Compiler.h" #include "swift/Basic/InlineBitfield.h" +#include "swift/Basic/NullablePtr.h" #include "swift/Basic/OptionalEnum.h" #include "swift/Basic/Range.h" #include "llvm/ADT/DenseMap.h" @@ -4747,8 +4748,32 @@ class VarDecl : public AbstractStorageDecl { /// return this. Otherwise, this VarDecl must belong to a CaseStmt's /// CaseLabelItem. In that case, return the first case label item of the first /// case stmt in a sequence of case stmts that fallthrough into each other. + /// + /// NOTE: During type checking, we emit an error if we have a single case + /// label item with a pattern that has multiple var decls of the same + /// name. This means that during type checking and before type checking, we + /// may have a _malformed_ switch stmt var decl linked list since var decls in + /// the same case label item that have the same name will point at the same + /// canonical var decl, namely the first var decl with the name in the + /// canonical case label item's var decl list. This is ok, since we are going + /// to emit the error, but it requires us to be more careful/cautious before + /// type checking has been complete when relying on canonical var decls + /// matching up. VarDecl *getCanonicalVarDecl() const; + /// If this is a case stmt var decl, return the var decl that corresponds to + /// this var decl in the first case label item of the case stmt. Returns + /// nullptr if this isn't a VarDecl that is part of a case stmt. + NullablePtr getCorrespondingFirstCaseLabelItemVarDecl() const; + + /// If this is a case stmt var decl, return the case body var decl that this + /// var decl maps to. + NullablePtr getCorrespondingCaseBodyVariable() const; + + /// Return true if this var decl is an implicit var decl belonging to a case + /// stmt's body. + bool isCaseBodyVariable() const; + /// True if the global stored property requires lazy initialization. bool isLazilyInitializedGlobal() const; diff --git a/include/swift/Basic/NullablePtr.h b/include/swift/Basic/NullablePtr.h index af9980c2aeb37..a584282e3a0d1 100644 --- a/include/swift/Basic/NullablePtr.h +++ b/include/swift/Basic/NullablePtr.h @@ -61,9 +61,13 @@ class NullablePtr { explicit operator bool() const { return Ptr; } - bool operator==(NullablePtr &&other) const { return other.Ptr == Ptr; } + bool operator==(const NullablePtr &other) const { + return other.Ptr == Ptr; + } - bool operator!=(NullablePtr &&other) const { return !(*this == other); } + bool operator!=(const NullablePtr &other) const { + return !(*this == other); + } bool operator==(const T *other) const { return other == Ptr; } diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index d6630adf0e07d..8706853643ef9 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -5066,6 +5066,55 @@ Pattern *VarDecl::getParentPattern() const { return nullptr; } +NullablePtr +VarDecl::getCorrespondingFirstCaseLabelItemVarDecl() const { + if (!hasName()) + return nullptr; + + auto *caseStmt = dyn_cast_or_null(getRecursiveParentPatternStmt()); + if (!caseStmt) + return nullptr; + + auto *pattern = caseStmt->getCaseLabelItems().front().getPattern(); + SmallVector vars; + pattern->collectVariables(vars); + for (auto *vd : vars) { + if (vd->hasName() && vd->getName() == getName()) + return vd; + } + return nullptr; +} + +bool VarDecl::isCaseBodyVariable() const { + auto *caseStmt = dyn_cast_or_null(getRecursiveParentPatternStmt()); + if (!caseStmt) + return false; + return llvm::any_of(caseStmt->getCaseBodyVariablesOrEmptyArray(), + [&](VarDecl *vd) { return vd == this; }); +} + +NullablePtr VarDecl::getCorrespondingCaseBodyVariable() const { + // Only var decls associated with case statements can have child var decls. + auto *caseStmt = dyn_cast_or_null(getRecursiveParentPatternStmt()); + if (!caseStmt) + return nullptr; + + // If this var decl doesn't have a name, it can not have a corresponding case + // body variable. + if (!hasName()) + return nullptr; + + auto name = getName(); + + // A var decl associated with a case stmt implies that the case stmt has body + // var decls. So we can access the optional value here without worry. + auto caseBodyVars = *caseStmt->getCaseBodyVariables(); + auto result = llvm::find_if(caseBodyVars, [&](VarDecl *caseBodyVar) { + return caseBodyVar->getName() == name; + }); + return (result != caseBodyVars.end()) ? *result : nullptr; +} + bool VarDecl::isSelfParameter() const { if (isa(this)) { if (auto *AFD = dyn_cast(getDeclContext())) From ed94b13d3fdb670c1817ab8f670b67fec3f3262b Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Fri, 29 Mar 2019 13:37:03 -0700 Subject: [PATCH 2/3] [ast] Add a helper method for checking if a pattern contains a var decl and adopt it. --- include/swift/AST/Pattern.h | 7 +++++++ lib/AST/Decl.cpp | 16 +++++----------- lib/Sema/MiscDiagnostics.cpp | 12 ++++-------- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/include/swift/AST/Pattern.h b/include/swift/AST/Pattern.h index 27397b1dd1e16..869fd2b9895ea 100644 --- a/include/swift/AST/Pattern.h +++ b/include/swift/AST/Pattern.h @@ -168,6 +168,13 @@ class alignas(8) Pattern { /// pattern. void forEachVariable(llvm::function_ref f) const; + /// Returns true if \p vd is in the pattern. + bool containsVarDecl(const VarDecl *inputVD) const { + bool result = false; + forEachVariable([&](VarDecl *vd) { result |= inputVD == vd; }); + return result; + } + /// apply the specified function to all pattern nodes recursively in /// this pattern. This is a pre-order traversal. void forEachNode(llvm::function_ref f); diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 8706853643ef9..73e951fa2da91 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -1307,14 +1307,14 @@ unsigned PatternBindingDecl::getPatternEntryIndexForVarDecl(const VarDecl *VD) c auto List = getPatternList(); if (List.size() == 1) { - assert(patternContainsVarDeclBinding(List[0].getPattern(), VD) && + assert(List[0].getPattern()->containsVarDecl(VD) && "Single entry PatternBindingDecl is set up wrong"); return 0; } unsigned Result = 0; for (auto entry : List) { - if (patternContainsVarDeclBinding(entry.getPattern(), VD)) + if (entry.getPattern()->containsVarDecl(VD)) return Result; ++Result; } @@ -4927,12 +4927,6 @@ SourceRange VarDecl::getTypeSourceRangeForDiagnostics() const { return SourceRange(); } -static bool isVarInPattern(const VarDecl *vd, Pattern *p) { - bool foundIt = false; - p->forEachVariable([&](VarDecl *foundFD) { foundIt |= foundFD == vd; }); - return foundIt; -} - static Optional> findParentPatternCaseStmtAndPattern(const VarDecl *inputVD) { auto getMatchingPattern = [&](CaseStmt *cs) -> Pattern * { @@ -4946,7 +4940,7 @@ findParentPatternCaseStmtAndPattern(const VarDecl *inputVD) { // Then check the rest of our case label items. for (auto &item : cs->getMutableCaseLabelItems()) { - if (isVarInPattern(inputVD, item.getPattern())) { + if (item.getPattern()->containsVarDecl(inputVD)) { return item.getPattern(); } } @@ -5039,7 +5033,7 @@ Pattern *VarDecl::getParentPattern() const { // In a case statement, search for the pattern that contains it. This is // a bit silly, because you can't have something like "case x, y:" anyway. for (auto items : cs->getCaseLabelItems()) { - if (isVarInPattern(this, items.getPattern())) + if (items.getPattern()->containsVarDecl(this)) return items.getPattern(); } } @@ -5047,7 +5041,7 @@ Pattern *VarDecl::getParentPattern() const { if (auto *LCS = dyn_cast(stmt)) { for (auto &elt : LCS->getCond()) if (auto pat = elt.getPatternOrNull()) - if (isVarInPattern(this, pat)) + if (pat->containsVarDecl(this)) return pat; } diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index 8f40b9e0c9cfb..47648e12135d3 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -2418,14 +2418,10 @@ VarDeclUsageChecker::~VarDeclUsageChecker() { // Only diagnose VarDecls from the first CaseLabelItem in CaseStmts, as // the remaining items must match it anyway. auto CaseItems = CS->getCaseLabelItems(); - if (!CaseItems.empty()) { - bool InFirstCaseLabelItem = false; - CaseItems.front().getPattern()->forEachVariable([&](VarDecl *D) { - InFirstCaseLabelItem |= var == D; - }); - if (!InFirstCaseLabelItem) - continue; - } + assert(!CaseItems.empty() && + "If we have any case stmt var decls, we should have a case item"); + if (!CaseItems.front().getPattern()->containsVarDecl(var)) + continue; } // If this is a 'let' value, any stores to it are actually initializations, From 7b0d8455cae55a65fe97b0a23cf51f0d5ff725fb Mon Sep 17 00:00:00 2001 From: Michael Gottesman Date: Fri, 29 Mar 2019 13:47:51 -0700 Subject: [PATCH 3/3] [ast][silgen] Wire up the case body var decls and use them in SILGenPattern emission to fix the evil fallthrough bug. rdar://47467128 --- include/swift/Parse/Parser.h | 4 +- include/swift/Parse/Scope.h | 3 +- lib/AST/ASTVerifier.cpp | 14 ++ lib/AST/Decl.cpp | 8 - lib/AST/NameLookup.cpp | 10 +- lib/Parse/ParseStmt.cpp | 14 ++ lib/Parse/Scope.cpp | 13 +- lib/SILGen/SILGenPattern.cpp | 153 ++++++------- lib/Sema/MiscDiagnostics.cpp | 70 ++++-- test/SILGen/switch.swift | 215 +++++++++++++++++- .../cursor_vardecl_across_fallthrough.swift | 4 +- 11 files changed, 385 insertions(+), 123 deletions(-) diff --git a/include/swift/Parse/Parser.h b/include/swift/Parse/Parser.h index badf942dc1bbb..1a521c40d03d4 100644 --- a/include/swift/Parse/Parser.h +++ b/include/swift/Parse/Parser.h @@ -689,11 +689,11 @@ class Parser { swift::ScopeInfo &getScopeInfo() { return State->getScopeInfo(); } /// Add the given Decl to the current scope. - void addToScope(ValueDecl *D) { + void addToScope(ValueDecl *D, bool diagnoseRedefinitions = true) { if (Context.LangOpts.EnableASTScopeLookup) return; - getScopeInfo().addToScope(D, *this); + getScopeInfo().addToScope(D, *this, diagnoseRedefinitions); } ValueDecl *lookupInScope(DeclName Name) { diff --git a/include/swift/Parse/Scope.h b/include/swift/Parse/Scope.h index 14175d515d1e6..2fb49bf112d42 100644 --- a/include/swift/Parse/Scope.h +++ b/include/swift/Parse/Scope.h @@ -52,7 +52,8 @@ class ScopeInfo { /// addToScope - Register the specified decl as being in the current lexical /// scope. - void addToScope(ValueDecl *D, Parser &TheParser); + void addToScope(ValueDecl *D, Parser &TheParser, + bool diagnoseRedefinitions = true); bool isInactiveConfigBlock() const; diff --git a/lib/AST/ASTVerifier.cpp b/lib/AST/ASTVerifier.cpp index cbf8067d52e1a..6a5c740530e0b 100644 --- a/lib/AST/ASTVerifier.cpp +++ b/lib/AST/ASTVerifier.cpp @@ -2500,6 +2500,20 @@ class Verifier : public ASTWalker { } } + if (auto *caseStmt = + dyn_cast_or_null(var->getRecursiveParentPatternStmt())) { + // In a type checked AST, a case stmt that is a recursive parent pattern + // stmt of a var decl, must have bound decls. This is because we + // guarantee that all case label items bind corresponding patterns and + // the case body var decls of a case stmt are created from the var decls + // of the first case label items. + if (!caseStmt->hasBoundDecls()) { + Out << "parent CaseStmt of VarDecl does not have any case body " + "decls?!\n"; + abort(); + } + } + verifyCheckedBase(var); } diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 73e951fa2da91..991a0cf36c057 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -1294,14 +1294,6 @@ VarDecl *PatternBindingInitializer::getInitializedLazyVar() const { return nullptr; } -static bool patternContainsVarDeclBinding(const Pattern *P, const VarDecl *VD) { - bool Result = false; - P->forEachVariable([&](VarDecl *FoundVD) { - Result |= FoundVD == VD; - }); - return Result; -} - unsigned PatternBindingDecl::getPatternEntryIndexForVarDecl(const VarDecl *VD) const { assert(VD && "Cannot find a null VarDecl"); diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index a11c88857a8cc..01980d12a2f71 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -2347,8 +2347,14 @@ void FindLocalVal::visitCaseStmt(CaseStmt *S) { } } } - if (!inPatterns && !items.empty()) - checkPattern(items[0].getPattern(), DeclVisibilityKind::LocalVariable); + + if (!inPatterns && !items.empty()) { + if (auto caseBodyVars = S->getCaseBodyVariables()) { + for (auto *vd : *caseBodyVars) { + checkValueDecl(vd, DeclVisibilityKind::LocalVariable); + } + } + } visit(S->getBody()); } diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp index 54c54c9e080db..c8a6ef62b409c 100644 --- a/lib/Parse/ParseStmt.cpp +++ b/lib/Parse/ParseStmt.cpp @@ -2476,6 +2476,20 @@ ParserResult Parser::parseStmtCase(bool IsActive) { assert(!CaseLabelItems.empty() && "did not parse any labels?!"); + // Add a scope so that the parser can find our body bound decls if it emits + // optimized accesses. + Optional BodyScope; + if (CaseBodyDecls) { + BodyScope.emplace(this, ScopeKind::CaseVars); + for (auto *v : *CaseBodyDecls) { + setLocalDiscriminator(v); + // If we had any bad redefinitions, we already diagnosed them against the + // first case label item. + if (v->hasName()) + addToScope(v, false /*diagnoseRedefinitions*/); + } + } + SmallVector BodyItems; SourceLoc StartOfBody = Tok.getLoc(); diff --git a/lib/Parse/Scope.cpp b/lib/Parse/Scope.cpp index d3db84e7899f4..ad8400e7320e5 100644 --- a/lib/Parse/Scope.cpp +++ b/lib/Parse/Scope.cpp @@ -104,7 +104,8 @@ static bool checkValidOverload(const ValueDecl *D1, const ValueDecl *D2, /// addToScope - Register the specified decl as being in the current lexical /// scope. -void ScopeInfo::addToScope(ValueDecl *D, Parser &TheParser) { +void ScopeInfo::addToScope(ValueDecl *D, Parser &TheParser, + bool diagnoseRedefinitions) { if (!CurScope->isResolvable()) return; @@ -121,9 +122,13 @@ void ScopeInfo::addToScope(ValueDecl *D, Parser &TheParser) { // If this is in a resolvable scope, diagnose redefinitions. Later // phases will handle scopes like module-scope, etc. - if (CurScope->getDepth() >= ResolvableDepth) - return TheParser.diagnoseRedefinition(PrevDecl, D); - + if (CurScope->getDepth() >= ResolvableDepth) { + if (diagnoseRedefinitions) { + return TheParser.diagnoseRedefinition(PrevDecl, D); + } + return; + } + // If this is at top-level scope, validate that the members of the overload // set all agree. diff --git a/lib/SILGen/SILGenPattern.cpp b/lib/SILGen/SILGenPattern.cpp index 15b8aaeac9783..aa7870cfbfbbf 100644 --- a/lib/SILGen/SILGenPattern.cpp +++ b/lib/SILGen/SILGenPattern.cpp @@ -445,6 +445,12 @@ class PatternMatchEmission { void emitCaseBody(CaseStmt *caseBlock); + SILValue getAddressOnlyTemporary(VarDecl *decl) { + auto found = Temporaries.find(decl); + assert(found != Temporaries.end()); + return found->second; + } + private: void emitWildcardDispatch(ClauseMatrix &matrix, ArgArray args, unsigned row, const FailureHandler &failure); @@ -2326,16 +2332,12 @@ void PatternMatchEmission::initSharedCaseBlockDest(CaseStmt *caseBlock, auto *block = SGF.createBasicBlock(); result.first->second.first = block; - // If we do not have any bound decls, we do not need to setup any phi - // arguments for the shared case block. Just bail early. - if (!caseBlock->hasBoundDecls()) { + // Add args for any pattern variables + auto caseBodyVars = caseBlock->getCaseBodyVariables(); + if (!caseBodyVars) return; - } - auto pattern = caseBlock->getCaseLabelItems()[0].getPattern(); - SmallVector patternVarDecls; - pattern->collectVariables(patternVarDecls); - for (auto *vd : patternVarDecls) { + for (auto *vd : *caseBodyVars) { if (!vd->hasName()) continue; @@ -2363,39 +2365,18 @@ void PatternMatchEmission::emitAddressOnlyAllocations() { for (auto &entry : SharedCases) { CaseStmt *caseBlock = entry.first; - // If we do not have any bound decls, we can bail early. - if (!caseBlock->hasBoundDecls()) { + auto caseBodyVars = caseBlock->getCaseBodyVariables(); + if (!caseBodyVars) continue; - } - // If we have a shared case with bound decls, then the 0th pattern has the - // order of variables that are the incoming BB arguments. Setup the VarLocs - // to point to the incoming args and setup initialization so any args needing - // cleanup will get that as well. - auto pattern = caseBlock->getCaseLabelItems()[0].getPattern(); - SmallVector patternVarDecls; - pattern->collectVariables(patternVarDecls); - for (auto *vd : patternVarDecls) { + // If we have a shared case with bound decls, setup the arguments for the + // shared block by emitting the temporary allocation used for the arguments + // of the shared block. + for (auto *vd : *caseBodyVars) { if (!vd->hasName()) continue; SILType ty = SGF.getLoweredType(vd->getType()); - if (ty.isNull()) { - // If we're making the shared block on behalf of a previous case's - // fallthrough, caseBlock's VarDecl's won't be in the SGF yet, so - // determine phi types by using current vars of the same name. - for (auto var : SGF.VarLocs) { - auto varDecl = dyn_cast(var.getFirst()); - if (!varDecl || !varDecl->hasName() || - varDecl->getName() != vd->getName()) - continue; - ty = var.getSecond().value->getType(); - if (var.getSecond().box) { - ty = ty.getObjectType(); - } - } - } - if (!ty.isAddressOnly(SGF.F.getModule())) continue; assert(!Temporaries[vd]); @@ -2455,23 +2436,19 @@ void PatternMatchEmission::emitSharedCaseBlocks() { assert(SGF.getCleanupsDepth() == PatternMatchStmtDepth); SWIFT_DEFER { assert(SGF.getCleanupsDepth() == PatternMatchStmtDepth); }; - // If we do not have any bound decls, just emit the case body since we do - // not need to setup any var locs. - if (!caseBlock->hasBoundDecls()) { + auto caseBodyVars = caseBlock->getCaseBodyVariables(); + if (!caseBodyVars) { emitCaseBody(caseBlock); continue; } - // If we have a shared case with bound decls, then the 0th pattern has the - // order of variables that are the incoming BB arguments. Setup the VarLocs - // to point to the incoming args and setup initialization so any args - // needing cleanup will get that as well. + // If we have a shared case with bound decls, then the case stmt pattern has + // the order of variables that are the incoming BB arguments. Setup the + // VarLocs to point to the incoming args and setup initialization so any + // args needing Cleanup will get that as well. Scope scope(SGF.Cleanups, CleanupLocation(caseBlock)); - auto pattern = caseBlock->getCaseLabelItems()[0].getPattern(); - SmallVector patternVarDecls; - pattern->collectVariables(patternVarDecls); unsigned argIndex = 0; - for (auto *vd : patternVarDecls) { + for (auto *vd : *caseBodyVars) { if (!vd->hasName()) continue; @@ -2622,10 +2599,33 @@ static void switchCaseStmtSuccessCallback(SILGenFunction &SGF, // Certain case statements can be entered along multiple paths, either because // they have multiple labels or because of fallthrough. When we need multiple - // entrance path, we factor the paths with a shared block. If we don't have a - // fallthrough or a multi-pattern 'case', we can just emit the body inline and - // save some dead blocks. Emit the statement here and bail early. + // entrance path, we factor the paths with a shared block. + // + // If we don't have a fallthrough or a multi-pattern 'case', we can emit the + // body inline. Emit the statement here and bail early. if (!row.hasFallthroughTo() && caseBlock->getCaseLabelItems().size() == 1) { + // If we have case body vars, set them up to point at the matching var + // decls. + if (auto caseBodyVars = caseBlock->getCaseBodyVariables()) { + // Since we know that we only have one case label item, grab its pattern + // vars and use that to update expected with the right SILValue. + // + // TODO: Do we need a copy here? + SmallVector patternVars; + row.getCasePattern()->collectVariables(patternVars); + for (auto *expected : *caseBodyVars) { + if (!expected->hasName()) + continue; + for (auto *vd : patternVars) { + if (!vd->hasName() || vd->getName() != expected->getName()) { + continue; + } + + // Ok, we found a match. Update the VarLocs for the case block. + SGF.VarLocs[expected] = SGF.VarLocs[vd]; + } + } + } emission.emitCaseBody(caseBlock); return; } @@ -2639,7 +2639,10 @@ static void switchCaseStmtSuccessCallback(SILGenFunction &SGF, // If we do not have any bound decls, we do not need to setup any // variables. Just jump to the shared destination. - if (!caseBlock->hasBoundDecls()) { + auto caseBodyVars = caseBlock->getCaseBodyVariables(); + if (!caseBodyVars) { + // Don't emit anything yet, we emit it at the cleanup level of the switch + // statement. JumpDest sharedDest = emission.getSharedCaseBlockDest(caseBlock); SGF.Cleanups.emitBranchAndCleanups(sharedDest, caseBlock); return; @@ -2650,18 +2653,14 @@ static void switchCaseStmtSuccessCallback(SILGenFunction &SGF, // +1 along to the shared case block dest. (The cleanups still happen, as they // are threaded through here messily, but the explicit retains here counteract // them, and then the retain/release pair gets optimized out.) - ArrayRef labelItems = caseBlock->getCaseLabelItems(); SmallVector args; - SmallVector expectedVarOrder; - SmallVector vars; - labelItems[0].getPattern()->collectVariables(expectedVarOrder); - row.getCasePattern()->collectVariables(vars); - SILModule &M = SGF.F.getModule(); - for (auto expected : expectedVarOrder) { + SmallVector patternVars; + row.getCasePattern()->collectVariables(patternVars); + for (auto *expected : *caseBodyVars) { if (!expected->hasName()) continue; - for (auto *var : vars) { + for (auto *var : patternVars) { if (!var->hasName() || var->getName() != expected->getName()) continue; @@ -2690,6 +2689,7 @@ static void switchCaseStmtSuccessCallback(SILGenFunction &SGF, } } + // Now that we have initialized our arguments, branch to the shared dest. SGF.Cleanups.emitBranchAndCleanups(sharedDest, caseBlock, args); } @@ -2837,45 +2837,46 @@ void SILGenFunction::emitSwitchStmt(SwitchStmt *S) { void SILGenFunction::emitSwitchFallthrough(FallthroughStmt *S) { assert(!SwitchStack.empty() && "fallthrough outside of switch?!"); PatternMatchContext *context = SwitchStack.back(); - + // Get the destination block. - CaseStmt *caseStmt = S->getFallthroughDest(); - JumpDest sharedDest = - context->Emission.getSharedCaseBlockDest(caseStmt); + CaseStmt *destCaseStmt = S->getFallthroughDest(); + JumpDest sharedDest = context->Emission.getSharedCaseBlockDest(destCaseStmt); - if (!caseStmt->hasBoundDecls()) { + // If our destination case doesn't have any bound decls, there is no rebinding + // to do. Just jump to the shared dest. + auto destCaseBodyVars = destCaseStmt->getCaseBodyVariables(); + if (!destCaseBodyVars) { Cleanups.emitBranchAndCleanups(sharedDest, S); return; } // Generate branch args to pass along current vars to fallthrough case. SILModule &M = F.getModule(); - ArrayRef labelItems = caseStmt->getCaseLabelItems(); SmallVector args; - SmallVector expectedVarOrder; - labelItems[0].getPattern()->collectVariables(expectedVarOrder); + CaseStmt *fallthroughSourceStmt = S->getFallthroughSource(); - for (auto *expected : expectedVarOrder) { + for (auto *expected : *destCaseBodyVars) { if (!expected->hasName()) continue; - for (auto var : VarLocs) { - auto varDecl = dyn_cast(var.getFirst()); - if (!varDecl || !varDecl->hasName() || - varDecl->getName() != expected->getName()) { + + // The type checker enforces that if our destination case has variables then + // our fallthrough source must as well. + for (auto *var : *fallthroughSourceStmt->getCaseBodyVariables()) { + if (!var->hasName() || var->getName() != expected->getName()) { continue; } - SILValue value = var.getSecond().value; + auto varLoc = VarLocs[var]; + SILValue value = varLoc.value; if (value->getType().isAddressOnly(M)) { context->Emission.emitAddressOnlyInitialization(expected, value); break; } - if (var.getSecond().box) { - auto &lowering = getTypeLowering(value->getType()); - auto argValue = lowering.emitLoad(B, CurrentSILLoc, value, - LoadOwnershipQualifier::Copy); + if (varLoc.box) { + SILValue argValue = B.emitLoadValueOperation( + CurrentSILLoc, value, LoadOwnershipQualifier::Copy); args.push_back(argValue); break; } diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index 47648e12135d3..f2a3d4c1f78b1 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -2152,6 +2152,7 @@ bool swift::fixItOverrideDeclarationTypes(InFlightDiagnostic &diag, //===----------------------------------------------------------------------===// namespace { + class VarDeclUsageChecker : public ASTWalker { DiagnosticEngine &Diags; // Keep track of some information about a variable. @@ -2204,9 +2205,12 @@ class VarDeclUsageChecker : public ASTWalker { VarDeclUsageChecker(DiagnosticEngine &Diags) : Diags(Diags) {} - VarDeclUsageChecker(TypeChecker &TC, VarDecl *VD) : Diags(TC.Diags) { + VarDeclUsageChecker(TypeChecker &tc, VarDecl *vd) : Diags(tc.Diags) { // Track a specific VarDecl - VarDecls[VD] = 0; + VarDecls[vd] = 0; + if (auto *childVd = vd->getCorrespondingCaseBodyVariable().getPtrOrNull()) { + VarDecls[childVd] = 0; + } } void suppressDiagnostics() { @@ -2288,16 +2292,30 @@ class VarDeclUsageChecker : public ASTWalker { handleIfConfig(ICD); // If this is a VarDecl, then add it to our list of things to track. - if (auto *vd = dyn_cast(D)) + if (auto *vd = dyn_cast(D)) { if (shouldTrackVarDecl(vd)) { - unsigned defaultFlags = 0; - // If this VarDecl is nested inside of a CaptureListExpr, remember that - // fact for better diagnostics. - auto parentAsExpr = Parent.getAsExpr(); - if (parentAsExpr && isa(parentAsExpr)) - defaultFlags = RK_CaptureList; + // Inline constructor. + auto defaultFlags = [&]() -> unsigned { + // If this VarDecl is nested inside of a CaptureListExpr, remember + // that fact for better diagnostics. + auto parentAsExpr = Parent.getAsExpr(); + if (parentAsExpr && isa(parentAsExpr)) + return RK_CaptureList; + // Otherwise, return none. + return 0; + }(); + + if (!vd->isImplicit()) { + if (auto *childVd = + vd->getCorrespondingCaseBodyVariable().getPtrOrNull()) { + // Child vars are never in capture lists. + assert(defaultFlags == 0); + VarDecls[childVd] |= 0; + } + } VarDecls[vd] |= defaultFlags; } + } if (auto *afd = dyn_cast(D)) { // If this is a nested function with a capture list, mark any captured @@ -2393,11 +2411,20 @@ class VarDeclUsageChecker : public ASTWalker { }); } } - + + // Make sure that we setup our case body variables. + if (auto *caseStmt = dyn_cast(S)) { + if (auto caseBoundDecls = caseStmt->getCaseBodyVariables()) { + for (auto *vd : *caseBoundDecls) { + VarDecls[vd] |= 0; + } + } + } + return { true, S }; } - }; + } // end anonymous namespace @@ -2409,19 +2436,24 @@ VarDeclUsageChecker::~VarDeclUsageChecker() { // lets let the bigger issues get resolved first. if (sawError) return; - - for (auto elt : VarDecls) { - auto *var = elt.first; - unsigned access = elt.second; - if (auto *CS = dyn_cast_or_null(var->getRecursiveParentPatternStmt())) { + for (auto p : VarDecls) { + VarDecl *var; + unsigned access; + std::tie(var, access) = p; + + if (auto *caseStmt = + dyn_cast_or_null(var->getRecursiveParentPatternStmt())) { // Only diagnose VarDecls from the first CaseLabelItem in CaseStmts, as // the remaining items must match it anyway. - auto CaseItems = CS->getCaseLabelItems(); - assert(!CaseItems.empty() && + auto caseItems = caseStmt->getCaseLabelItems(); + assert(!caseItems.empty() && "If we have any case stmt var decls, we should have a case item"); - if (!CaseItems.front().getPattern()->containsVarDecl(var)) + if (!caseItems.front().getPattern()->containsVarDecl(var)) continue; + + auto *childVar = var->getCorrespondingCaseBodyVariable().get(); + access |= VarDecls[childVar]; } // If this is a 'let' value, any stores to it are actually initializations, diff --git a/test/SILGen/switch.swift b/test/SILGen/switch.swift index 74bdb5d27d4bd..d24593775d1dd 100644 --- a/test/SILGen/switch.swift +++ b/test/SILGen/switch.swift @@ -1,5 +1,9 @@ // RUN: %target-swift-emit-silgen -module-name switch %s | %FileCheck %s +////////////////// +// Declarations // +////////////////// + func markUsed(_ t: T) {} // TODO: Implement tuple equality in the library. @@ -25,6 +29,43 @@ func e() {} func f() {} func g() {} +func a(_ k: Klass) {} +func b(_ k: Klass) {} +func c(_ k: Klass) {} +func d(_ k: Klass) {} +func e(_ k: Klass) {} +func f(_ k: Klass) {} +func g(_ k: Klass) {} + +class Klass { + var isTrue: Bool { return true } + var isFalse: Bool { return false } +} + +enum TrivialSingleCaseEnum { +case a +} + +enum NonTrivialSingleCaseEnum { +case a(Klass) +} + +enum MultipleNonTrivialCaseEnum { +case a(Klass) +case b(Klass) +case c(Klass) +} + +enum MultipleAddressOnlyCaseEnum { +case a(T) +case b(T) +case c(T) +} + +/////////// +// Tests // +/////////// + // CHECK-LABEL: sil hidden [ossa] @$s6switch5test1yyF func test1() { switch foo() { @@ -1091,15 +1132,6 @@ func testUninhabitedSwitchScrutinee() { // Make sure that we properly can handle address only tuples with loadable // subtypes. -class Klass {} - -enum TrivialSingleCaseEnum { -case a -} - -enum NonTrivialSingleCaseEnum { -case a(Klass) -} // CHECK-LABEL: sil hidden [ossa] @$s6switch33address_only_with_trivial_subtypeyyAA21TrivialSingleCaseEnumO_yptF : $@convention(thin) (TrivialSingleCaseEnum, @in_guaranteed Any) -> () { // CHECK: [[MEM:%.*]] = alloc_stack $(TrivialSingleCaseEnum, Any) @@ -1303,3 +1335,168 @@ func testVoidType() { break } } + +//////////////////////////////////////////////// +// Fallthrough Multiple Case Label Item Tests // +//////////////////////////////////////////////// + +// CHECK-LABEL: sil hidden [ossa] @$s6switch28addressOnlyFallthroughCalleeyyAA015MultipleAddressC8CaseEnumOyxGSzRzlF : $@convention(thin) (@in_guaranteed MultipleAddressOnlyCaseEnum) -> () { +// CHECK: bb0([[ARG:%.*]] : +// CHECK: [[AB_PHI:%.*]] = alloc_stack $T, let, name "x" +// CHECK: [[ABB_PHI:%.*]] = alloc_stack $T, let, name "x" +// CHECK: [[ABBC_PHI:%.*]] = alloc_stack $T, let, name "x" +// CHECK: [[SWITCH_ENUM_ARG:%.*]] = alloc_stack $MultipleAddressOnlyCaseEnum +// CHECK: copy_addr [[ARG]] to [initialization] [[SWITCH_ENUM_ARG]] +// CHECK: switch_enum_addr [[SWITCH_ENUM_ARG]] : $*MultipleAddressOnlyCaseEnum, case #MultipleAddressOnlyCaseEnum.a!enumelt.1: [[BB_A:bb[0-9]+]], case #MultipleAddressOnlyCaseEnum.b!enumelt.1: [[BB_B:bb[0-9]+]], case #MultipleAddressOnlyCaseEnum.c!enumelt.1: [[BB_C:bb[0-9]+]] +// +// CHECK: [[BB_A]]: +// CHECK: [[SWITCH_ENUM_ARG_PROJ:%.*]] = unchecked_take_enum_data_addr [[SWITCH_ENUM_ARG]] +// CHECK: [[CASE_BODY_VAR_A:%.*]] = alloc_stack $T, let, name "x" +// CHECK: copy_addr [take] [[SWITCH_ENUM_ARG_PROJ]] to [initialization] [[CASE_BODY_VAR_A]] +// CHECK: copy_addr [[CASE_BODY_VAR_A]] to [initialization] [[AB_PHI]] +// CHECK: destroy_addr [[CASE_BODY_VAR_A]] +// CHECK: br [[BB_AB:bb[0-9]+]] +// +// CHECK: [[BB_B]]: +// CHECK: [[SWITCH_ENUM_ARG_PROJ:%.*]] = unchecked_take_enum_data_addr [[SWITCH_ENUM_ARG]] +// CHECK: [[CASE_BODY_VAR_B:%.*]] = alloc_stack $T, let, name "x" +// CHECK: copy_addr [[SWITCH_ENUM_ARG_PROJ]] to [initialization] [[CASE_BODY_VAR_B]] +// CHECK: [[FUNC_CMP:%.*]] = function_ref @$sSzsE2eeoiySbx_qd__tSzRd__lFZ : +// CHECK: [[GUARD_RESULT:%.*]] = apply [[FUNC_CMP]]([[CASE_BODY_VAR_B]], {{%.*}}, {{%.*}}) +// CHECK: [[GUARD_RESULT_EXT:%.*]] = struct_extract [[GUARD_RESULT]] +// CHECK: cond_br [[GUARD_RESULT_EXT]], [[BB_B_GUARD_SUCC:bb[0-9]+]], [[BB_B_GUARD_FAIL:bb[0-9]+]] +// +// CHECK: [[BB_B_GUARD_SUCC]]: +// CHECK: copy_addr [[CASE_BODY_VAR_B]] to [initialization] [[AB_PHI]] +// CHECK: destroy_addr [[CASE_BODY_VAR_B]] +// CHECK: destroy_addr [[SWITCH_ENUM_ARG_PROJ]] +// CHECK: br [[BB_AB]] +// +// CHECK: [[BB_AB]]: +// CHECK: copy_addr [[AB_PHI]] to [initialization] [[ABB_PHI]] +// CHECK: destroy_addr [[AB_PHI]] +// CHECK: br [[BB_AB_CONT:bb[0-9]+]] +// +// CHECK: [[BB_AB_CONT]]: +// CHECK: copy_addr [[ABB_PHI]] to [initialization] [[ABBC_PHI]] +// CHECK: destroy_addr [[ABB_PHI]] +// CHECK: br [[BB_FINAL_CONT:bb[0-9]+]] +// +// CHECK: [[BB_B_GUARD_FAIL]]: +// CHECK: destroy_addr [[CASE_BODY_VAR_B]] +// CHECK: [[CASE_BODY_VAR_B_2:%.*]] = alloc_stack $T, let, name "x" +// CHECK: copy_addr [take] [[SWITCH_ENUM_ARG_PROJ]] to [initialization] [[CASE_BODY_VAR_B_2]] +// CHECK: copy_addr [[CASE_BODY_VAR_B_2]] to [initialization] [[ABB_PHI]] +// CHECK: br [[BB_AB_CONT]] +// +// CHECK: [[BB_C]]: +// CHECK: [[SWITCH_ENUM_ARG_PROJ:%.*]] = unchecked_take_enum_data_addr [[SWITCH_ENUM_ARG]] +// CHECK: [[CASE_BODY_VAR_C:%.*]] = alloc_stack $T, let, name "x" +// CHECK: copy_addr [take] [[SWITCH_ENUM_ARG_PROJ]] to [initialization] [[CASE_BODY_VAR_C]] +// CHECK: copy_addr [[CASE_BODY_VAR_C]] to [initialization] [[ABBC_PHI]] +// CHECK: destroy_addr [[CASE_BODY_VAR_C]] +// CHECK: br [[BB_FINAL_CONT]] +// +// CHECK: [[BB_FINAL_CONT]]: +// CHECK: destroy_addr [[ABBC_PHI]] +// CHECK: return +// CHECK: } // end sil function '$s6switch28addressOnlyFallthroughCalleeyyAA015MultipleAddressC8CaseEnumOyxGSzRzlF' +func addressOnlyFallthroughCallee(_ e : MultipleAddressOnlyCaseEnum) { + switch e { + case .a(let x): fallthrough + case .b(let x) where x == 2: fallthrough + case .b(let x): fallthrough + case .c(let x): + print(x) + } +} + +func addressOnlyFallthroughCaller() { + var myFoo : MultipleAddressOnlyCaseEnum = MultipleAddressOnlyCaseEnum.a(10) + addressOnlyFallthroughCallee(myFoo) +} + +// CHECK-LABEL: sil hidden [ossa] @$s6switch35nonTrivialLoadableFallthroughCalleeyyAA011MultipleNonC8CaseEnumOF : $@convention(thin) (@guaranteed MultipleNonTrivialCaseEnum) -> () { +// CHECK: bb0([[ARG:%.*]] : @guaranteed $MultipleNonTrivialCaseEnum): +// CHECK: switch_enum [[ARG]] : $MultipleNonTrivialCaseEnum, case #MultipleNonTrivialCaseEnum.a!enumelt.1: [[BB_A:bb[0-9]+]], case #MultipleNonTrivialCaseEnum.b!enumelt.1: [[BB_B:bb[0-9]+]], case #MultipleNonTrivialCaseEnum.c!enumelt.1: [[BB_C:bb[0-9]+]] +// +// CHECK: [[BB_A]]([[BB_A_ARG:%.*]] : @guaranteed +// CHECK: [[BB_A_ARG_COPY:%.*]] = copy_value [[BB_A_ARG]] +// CHECK: [[BB_A_ARG_COPY_BORROW:%.*]] = begin_borrow [[BB_A_ARG_COPY]] +// CHECK: apply {{%.*}}([[BB_A_ARG_COPY_BORROW]]) +// CHECK: [[RESULT:%.*]] = copy_value [[BB_A_ARG_COPY]] +// CHECK: br [[BB_AB:bb[0-9]+]]([[RESULT]] : +// +// CHECK: [[BB_B]]([[BB_B_ARG:%.*]] : @guaranteed +// CHECK: [[BB_B_ARG_COPY:%.*]] = copy_value [[BB_B_ARG]] +// CHECK: [[RESULT:%.*]] = copy_value [[BB_B_ARG_COPY]] +// CHECK: br [[BB_AB:bb[0-9]+]]([[RESULT]] : +// +// CHECK: [[BB_AB:bb[0-9]+]]([[BB_AB_PHI:%.*]] : @owned +// CHECK: [[BB_AB_PHI_BORROW:%.*]] = begin_borrow [[BB_AB_PHI]] +// CHECK: apply {{%.*}}([[BB_AB_PHI_BORROW]]) +// CHECK: [[RESULT:%.*]] = copy_value [[BB_AB_PHI]] +// CHECK: br [[BB_ABC:bb[0-9]+]]([[RESULT]] : +// +// CHECK: [[BB_C]]([[BB_C_ARG:%.*]] : @guaranteed +// CHECK: [[BB_C_COPY:%.*]] = copy_value [[BB_C_ARG]] +// CHECK: [[RESULT:%.*]] = copy_value [[BB_C_COPY]] +// CHECK: br [[BB_ABC]]([[RESULT]] : +// +// CHECK: [[BB_ABC]]([[BB_ABC_ARG:%.*]] : @owned +// CHECK: [[BB_ABC_ARG_BORROW:%.*]] = begin_borrow [[BB_ABC_ARG]] +// CHECK: apply {{%.*}}([[BB_ABC_ARG_BORROW]]) +// CHECK: return +// CHECK: } // end sil function '$s6switch35nonTrivialLoadableFallthroughCalleeyyAA011MultipleNonC8CaseEnumOF' +func nonTrivialLoadableFallthroughCallee(_ e : MultipleNonTrivialCaseEnum) { + switch e { + case .a(let x): + a(x) + fallthrough + case .b(let x): + b(x) + fallthrough + case .c(let x): + c(x) + } +} + +// Just make sure that we do not crash on this. +func nonTrivialLoadableFallthroughCalleeGuards(_ e : MultipleNonTrivialCaseEnum) { + switch e { + case .a(let x) where x.isFalse: + a(x) + fallthrough + case .a(let x) where x.isTrue: + a(x) + fallthrough + case .b(let x) where x.isTrue: + b(x) + fallthrough + case .b(let x) where x.isFalse: + b(x) + fallthrough + case .c(let x) where x.isTrue: + c(x) + fallthrough + case .c(let x) where x.isFalse: + c(x) + break + default: + d() + } +} + +func nonTrivialLoadableFallthroughCallee2(_ e : MultipleNonTrivialCaseEnum) { + switch e { + case .a(let x): + a(x) + fallthrough + case .b(let x): + b(x) + break + default: + break + } +} + diff --git a/test/SourceKit/CursorInfo/cursor_vardecl_across_fallthrough.swift b/test/SourceKit/CursorInfo/cursor_vardecl_across_fallthrough.swift index 020f19fbee851..2e348abd3e76d 100644 --- a/test/SourceKit/CursorInfo/cursor_vardecl_across_fallthrough.swift +++ b/test/SourceKit/CursorInfo/cursor_vardecl_across_fallthrough.swift @@ -31,7 +31,7 @@ switch p { // CHECK2DECL: source.lang.swift.decl.var.local (16:20-16:21) // CHECK2DECL2: source.lang.swift.decl.var.local (16:42-16:43) -// CHECK2REF: source.lang.swift.ref.var.local (16:20-16:21) +// CHECK2REF: source.lang.swift.ref.var.local (16:42-16:43) // CHECKX: x // CHECKX: s:33cursor_vardecl_across_fallthrough1xL_Sivp @@ -50,7 +50,7 @@ switch p { // CHECK4DECL: source.lang.swift.decl.var.local (16:27-16:28) // CHECK4DECL2: source.lang.swift.decl.var.local (16:49-16:50) -// CHECK4REF: source.lang.swift.ref.var.local (16:27-16:28) +// CHECK4REF: source.lang.swift.ref.var.local (16:49-16:50) // CHECKY: y // CHECKY: s:33cursor_vardecl_across_fallthrough1yL_SSvp