From 89d74d21fe1e234e62d3dab30aa0117c4a918e1f Mon Sep 17 00:00:00 2001 From: Varun Gandhi Date: Tue, 6 Apr 2021 21:59:48 -0700 Subject: [PATCH 1/4] WIP: Support for nested unknowns. Basic support is working. Fixes SR-14452, rdar://problem/76280689. TODO: 1. Add more test cases, including for parsing and parse errors. 2. Maybe check with Robert if overall approach seems right? 3. Check that fix-its are inserted in right place. 4. Fix issue where if let @unknown _ is permitted. --- include/swift/AST/DiagnosticsSema.def | 4 + include/swift/AST/Expr.h | 11 ++- include/swift/AST/Pattern.h | 18 +++- lib/AST/ASTDumper.cpp | 4 + lib/AST/Pattern.cpp | 24 +++++ lib/Parse/ParseExpr.cpp | 16 +++- lib/Parse/ParsePattern.cpp | 10 +- lib/Parse/ParseStmt.cpp | 12 ++- lib/Sema/TypeCheckPattern.cpp | 2 +- lib/Sema/TypeCheckSwitchStmt.cpp | 115 +++++++++++++++++++---- test/Sema/exhaustive_switch.swift | 62 ++++++++++++ utils/gyb_syntax_support/ExprNodes.py | 2 + utils/gyb_syntax_support/PatternNodes.py | 2 + 13 files changed, 241 insertions(+), 41 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index bb76400f8c76e..12deb6d2ede4f 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -5557,6 +5557,10 @@ WARNING(non_exhaustive_switch_unknown_only,none, "switch covers known cases, but %0 may have additional unknown values" "%select{|, possibly added in future versions}1", (Type, bool)) +WARNING(unknown_pattern_for_non_enum,none, + "unknown pattern is not applicable to non-enum type %0", (Type)) +NOTE(non_enum_drop_unknown,none, "remove '@unknown'", ()) + ERROR(override_nsobject_hashvalue_error,none, "'NSObject.hashValue' is not overridable; " "did you mean to override 'NSObject.hash'?", ()) diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index c553390f1800b..e07086f37d399 100644 --- a/include/swift/AST/Expr.h +++ b/include/swift/AST/Expr.h @@ -1164,12 +1164,17 @@ class ObjectLiteralExpr final /// DiscardAssignmentExpr - A '_' in the left-hand side of an assignment, which /// discards the corresponding tuple element on the right-hand side. class DiscardAssignmentExpr : public Expr { + /// HACK: Location for @unknown _ in conditional clauses in switch statements. + /// @unknown isn't permitted in assignment contexts. + SourceLoc UnknownLoc; SourceLoc Loc; public: - DiscardAssignmentExpr(SourceLoc Loc, bool Implicit) - : Expr(ExprKind::DiscardAssignment, Implicit), Loc(Loc) {} - + DiscardAssignmentExpr(SourceLoc UnknownLoc, SourceLoc Loc, bool Implicit) + : Expr(ExprKind::DiscardAssignment, Implicit), UnknownLoc(UnknownLoc), + Loc(Loc) {} + + SourceLoc getUnknownLoc() const { return UnknownLoc; } SourceRange getSourceRange() const { return Loc; } SourceLoc getLoc() const { return Loc; } diff --git a/include/swift/AST/Pattern.h b/include/swift/AST/Pattern.h index f1ac363f351a4..bcb31309ddd73 100644 --- a/include/swift/AST/Pattern.h +++ b/include/swift/AST/Pattern.h @@ -191,6 +191,9 @@ class alignas(8) Pattern { bool isNeverDefaultInitializable() const; + /// Return true if this pattern (or a subpattern) contains @unknown _. + bool hasUnknownAnyPattern() const; + /// Mark all vardecls in this pattern as having an owning statement for /// the pattern. void markOwnedByStatement(Stmt *S) { @@ -369,22 +372,27 @@ class NamedPattern : public Pattern { }; /// A pattern which matches an arbitrary value of a type, but does not -/// bind a name to it. This is spelled "_". +/// bind a name to it. This is spelled "_" or "@unknown _". class AnyPattern : public Pattern { + SourceLoc UnknownLoc; SourceLoc Loc; public: - explicit AnyPattern(SourceLoc Loc) - : Pattern(PatternKind::Any), Loc(Loc) { } + explicit AnyPattern(SourceLoc UnknownLoc, SourceLoc Loc) + : Pattern(PatternKind::Any), UnknownLoc(UnknownLoc), Loc(Loc) {} static AnyPattern *createImplicit(ASTContext &Context) { - auto *AP = new (Context) AnyPattern(SourceLoc()); + auto *AP = new (Context) AnyPattern(SourceLoc(), SourceLoc()); AP->setImplicit(); return AP; } + SourceLoc getUnknownLoc() const { return UnknownLoc; } + bool isUnknown() const { return UnknownLoc.isValid(); } SourceLoc getLoc() const { return Loc; } - SourceRange getSourceRange() const { return Loc; } + SourceRange getSourceRange() const { + return isUnknown() ? SourceRange(UnknownLoc, Loc) : SourceRange(Loc); + } static bool classof(const Pattern *P) { return P->getKind() == PatternKind::Any; diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp index 115e3b9cf45cc..fa27fe1fb01c5 100644 --- a/lib/AST/ASTDumper.cpp +++ b/lib/AST/ASTDumper.cpp @@ -474,6 +474,8 @@ namespace { } void visitAnyPattern(AnyPattern *P) { printCommon(P, "pattern_any"); + if (P->getUnknownLoc().isValid()) + OS << " @unknown"; PrintWithColorRAII(OS, ParenthesisColor) << ')'; } void visitTypedPattern(TypedPattern *P) { @@ -2030,6 +2032,8 @@ class PrintExpr : public ExprVisitor { void visitDiscardAssignmentExpr(DiscardAssignmentExpr *E) { printCommon(E, "discard_assignment_expr"); + if (E->getUnknownLoc().isValid()) + OS << " @unknown"; PrintWithColorRAII(OS, ParenthesisColor) << ')'; } diff --git a/lib/AST/Pattern.cpp b/lib/AST/Pattern.cpp index eed9e33c0341c..b9cd1a25fc156 100644 --- a/lib/AST/Pattern.cpp +++ b/lib/AST/Pattern.cpp @@ -345,6 +345,30 @@ case PatternKind::ID: foundRefutablePattern = true; break; return foundRefutablePattern; } +bool Pattern::hasUnknownAnyPattern() const { + bool foundUnknownAnyPattern = false; + const_cast(this)->forEachNode([&](Pattern *Node) { + switch (Node->getKind()) { + case PatternKind::Any: + foundUnknownAnyPattern |= cast(Node)->isUnknown(); + return; + case PatternKind::Named: + case PatternKind::Expr: + case PatternKind::Bool: + case PatternKind::Is: + case PatternKind::Paren: + case PatternKind::Typed: + case PatternKind::Binding: + case PatternKind::Tuple: + case PatternKind::EnumElement: + case PatternKind::OptionalSome: + return; + } + llvm_unreachable("Unhandled PatternKind!"); + }); + return foundUnknownAnyPattern; +} + /// Standard allocator for Patterns. void *Pattern::operator new(size_t numBytes, const ASTContext &C) { return C.Allocate(numBytes, alignof(Pattern)); diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp index d494e697d683f..50ecb9a151ef9 100644 --- a/lib/Parse/ParseExpr.cpp +++ b/lib/Parse/ParseExpr.cpp @@ -1421,7 +1421,7 @@ ParserResult Parser::parseExprPostfix(Diag<> ID, bool isExprBasic) { /// '.' identifier /// /// expr-discard: -/// '_' +/// @unknown? '_' (@unknown can only appear in patterns) /// /// expr-primary: /// expr-literal @@ -1456,6 +1456,16 @@ ParserResult Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) { // Objective-C programmers habitually type @"foo", so recover gracefully // with a fixit. If this isn't @"foo", just handle it like an unknown // input. + if (peekToken().isContextualKeyword("unknown")) { + auto UnknownLoc = consumeToken(); + consumeToken(); + if (!Tok.is(tok::kw__)) + goto UnknownCharacter; + // Similar to the tok::kw__ case. + ExprContext.setCreateSyntax(SyntaxKind::DiscardAssignmentExpr); + return makeParserResult(new (Context) DiscardAssignmentExpr( + UnknownLoc, consumeToken(), /*Implicit=*/false)); + } if (peekToken().isNot(tok::string_literal)) goto UnknownCharacter; @@ -1558,8 +1568,8 @@ ParserResult Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) { case tok::kw__: // _ ExprContext.setCreateSyntax(SyntaxKind::DiscardAssignmentExpr); - return makeParserResult( - new (Context) DiscardAssignmentExpr(consumeToken(), /*Implicit=*/false)); + return makeParserResult(new (Context) DiscardAssignmentExpr( + /*@unknown*/ SourceLoc(), consumeToken(), /*Implicit=*/false)); case tok::pound_selector: // expr-selector return parseExprSelector(); diff --git a/lib/Parse/ParsePattern.cpp b/lib/Parse/ParsePattern.cpp index 333b5e807f230..9baeb86f726c4 100644 --- a/lib/Parse/ParsePattern.cpp +++ b/lib/Parse/ParsePattern.cpp @@ -957,7 +957,7 @@ ParserResult Parser::parseTypedPattern() { if (result.isNull()) { // Recover by creating AnyPattern. - auto *AP = new (Context) AnyPattern(colonLoc); + auto *AP = new (Context) AnyPattern(/*@unknown*/ SourceLoc(), colonLoc); if (colonLoc.isInvalid()) AP->setImplicit(); result = makeParserErrorResult(AP); @@ -1044,8 +1044,9 @@ ParserResult Parser::parsePattern() { return makeParserResult(NamedPattern::createImplicit(Context, VD)); } PatternCtx.setCreateSyntax(SyntaxKind::WildcardPattern); - return makeParserResult(new (Context) AnyPattern(consumeToken(tok::kw__))); - + return makeParserResult(new (Context) AnyPattern(/*@unknown*/ SourceLoc(), + consumeToken(tok::kw__))); + case tok::identifier: { PatternCtx.setCreateSyntax(SyntaxKind::IdentifierPattern); Identifier name; @@ -1102,7 +1103,8 @@ ParserResult Parser::parsePattern() { .fixItReplace(Tok.getLoc(), "`" + Tok.getText().str() + "`"); SourceLoc Loc = Tok.getLoc(); consumeToken(); - return makeParserErrorResult(new (Context) AnyPattern(Loc)); + return makeParserErrorResult( + new (Context) AnyPattern(/*@unknown*/ SourceLoc(), Loc)); } diagnose(Tok, diag::expected_pattern); return nullptr; diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp index 7320136ee1ca6..d9c4a64ef8613 100644 --- a/lib/Parse/ParseStmt.cpp +++ b/lib/Parse/ParseStmt.cpp @@ -1100,7 +1100,8 @@ static void parseGuardedPattern(Parser &P, GuardedPattern &result, // If that didn't work, use a bogus pattern so that we can fill out // the AST. if (patternResult.isNull()) { - auto *AP = new (P.Context) AnyPattern(P.PreviousLoc); + auto *AP = + new (P.Context) AnyPattern(/*@unknown*/ SourceLoc(), P.PreviousLoc); if (P.PreviousLoc.isInvalid()) AP->setImplicit(); patternResult = makeParserErrorResult(AP); @@ -1551,7 +1552,7 @@ Parser::parseStmtConditionElement(SmallVectorImpl &result, if (ThePattern.isNull()) { // Recover by creating AnyPattern. - auto *AP = new (Context) AnyPattern(PreviousLoc); + auto *AP = new (Context) AnyPattern(/*@unknown*/ SourceLoc(), PreviousLoc); if (PreviousLoc.isInvalid()) AP->setImplicit(); ThePattern = makeParserResult(AP); @@ -2407,7 +2408,7 @@ parseStmtCase(Parser &P, SourceLoc &CaseLoc, static ParserStatus parseStmtCaseDefault(Parser &P, SourceLoc &CaseLoc, SmallVectorImpl &LabelItems, - SourceLoc &ColonLoc) { + SourceLoc UnknownLoc, SourceLoc &ColonLoc) { SyntaxParsingContext CaseContext(P.SyntaxContext, SyntaxKind::SwitchDefaultLabel); ParserStatus Status; @@ -2433,7 +2434,7 @@ parseStmtCaseDefault(Parser &P, SourceLoc &CaseLoc, P.consumeToken(tok::colon); // Create an implicit AnyPattern to represent the default match. - auto Any = new (P.Context) AnyPattern(CaseLoc); + auto Any = new (P.Context) AnyPattern(UnknownLoc, CaseLoc); if (CaseLoc.isInvalid()) Any->setImplicit(); LabelItems.push_back( @@ -2525,7 +2526,8 @@ ParserResult Parser::parseStmtCase(bool IsActive) { Status |= ::parseStmtCase(*this, CaseLoc, CaseLabelItems, BoundDecls, ColonLoc, CaseBodyDecls); } else if (Tok.is(tok::kw_default)) { - Status |= parseStmtCaseDefault(*this, CaseLoc, CaseLabelItems, ColonLoc); + Status |= parseStmtCaseDefault(*this, CaseLoc, CaseLabelItems, + UnknownAttrLoc, ColonLoc); } else { llvm_unreachable("isAtStartOfSwitchCase() lied."); } diff --git a/lib/Sema/TypeCheckPattern.cpp b/lib/Sema/TypeCheckPattern.cpp index 4e1e98e7ccf48..94b28b67c30e7 100644 --- a/lib/Sema/TypeCheckPattern.cpp +++ b/lib/Sema/TypeCheckPattern.cpp @@ -349,7 +349,7 @@ class ResolvePattern : public ASTVisitorisImplicit()) { return AnyPattern::createImplicit(Context); } - return new (Context) AnyPattern(E->getLoc()); + return new (Context) AnyPattern(E->getUnknownLoc(), E->getLoc()); } // Cast expressions 'x as T' get resolved to checked cast patterns. diff --git a/lib/Sema/TypeCheckSwitchStmt.cpp b/lib/Sema/TypeCheckSwitchStmt.cpp index 51fba661ee5bd..65dc80ec9f73a 100644 --- a/lib/Sema/TypeCheckSwitchStmt.cpp +++ b/lib/Sema/TypeCheckSwitchStmt.cpp @@ -137,7 +137,7 @@ namespace { cache.insert(getType().getPointer()); SmallVector spaces; - decompose(DC, getType(), spaces); + decompose(DC, getType(), /*makeUnknownPatterns*/ true, spaces); size_t acc = 0; for (auto &sp : spaces) { // Decomposed pattern spaces grow with the sum of the subspaces. @@ -778,8 +778,42 @@ namespace { } }; + /// Decompose '@unknown _' patterns in arbitrary nested positions. + /// + /// For example, if you have a non-frozen enum E { case a; case b }, + /// and you're matching on a pair of type (E, E): + /// \code + /// (@unknown _, @unknown _) + /// [decompose]-> ((.a | .b), (.a | .b)) + /// [flatten]-> (.a, .a) | (.b, .a) | (.a, .b) | (.b, .b) + /// \endcode + static void decomposeUnknowns(const DeclContext *DC, + const Pattern *pattern, + SmallVectorImpl &outSpaces) { + auto projectAnyPattern = [DC](const AnyPattern *anyPattern) -> Space { + auto type = anyPattern->getType(); + if (anyPattern->isUnknown()) { + if (Space::canDecompose(type, DC)) { + SmallVector spaces; + Space::decompose(DC, type, /*makeUnknownPatterns*/ false, spaces); + return Space::forDisjunct(spaces); + } + auto &diag = DC->getASTContext().Diags; + auto unknownLoc = anyPattern->getUnknownLoc(); + diag.diagnose(unknownLoc, diag::unknown_pattern_for_non_enum, type); + diag.diagnose(unknownLoc, diag::non_enum_drop_unknown) + .fixItRemove( + SourceRange(unknownLoc, unknownLoc.getAdvancedLoc(8))); + } + return Space::forType(anyPattern->getType(), Identifier()); + }; + auto decomposed = projectPattern(pattern, projectAnyPattern); + flatten(decomposed, outSpaces); + } + // Decompose a type into its component spaces. static void decompose(const DeclContext *DC, Type tp, + bool makeUnknownPatterns, SmallVectorImpl &arr) { assert(canDecompose(tp, DC) && "Non-decomposable type?"); @@ -816,10 +850,12 @@ namespace { constElemSpaces); }); - if (!E->isFormallyExhaustive(DC)) { - arr.push_back(Space::forUnknown(/*allowedButNotRequired*/false)); - } else if (!E->getAttrs().hasAttribute()) { - arr.push_back(Space::forUnknown(/*allowedButNotRequired*/true)); + if (makeUnknownPatterns) { + if (!E->isFormallyExhaustive(DC)) { + arr.push_back(Space::forUnknown(/*allowedButNotRequired*/ false)); + } else if (!E->getAttrs().hasAttribute()) { + arr.push_back(Space::forUnknown(/*allowedButNotRequired*/ true)); + } } } else if (auto *TTy = tp->castTo()) { @@ -840,7 +876,7 @@ namespace { static Space decompose(const DeclContext *DC, Type type) { SmallVector spaces; - decompose(DC, type, spaces); + decompose(DC, type, /*makeUnknownPatterns*/ true, spaces); return Space::forDisjunct(spaces); } @@ -955,6 +991,33 @@ namespace { const CaseStmt *unknownCase = nullptr; SmallVector spaces; auto &DE = Context.Diags; + + auto handleUnknownAnyPattern = + [this](const CaseStmt *caseBlock, + const SmallVectorImpl &spacesSoFar, + const Pattern *pattern, const Space &projection) { + if (!pattern->hasUnknownAnyPattern()) + return; + Space coveredSoFar = Space::forDisjunct(spacesSoFar); + Space partialUncovered = + projection.minus(coveredSoFar, DC, /*minusCount*/ nullptr) + .getValue(); + if (!partialUncovered.isEmpty()) { + // TODO: Maybe we should prefer inserting alternatives in-between + // if the @unknown _ is not in the first condition clause? + // case .a, (@unknown _) [fix]-> case .a, .b, (@unknown _) + SmallVector scratchSpaces; + Space::decomposeUnknowns(DC, pattern, scratchSpaces); + Space projectionDecomposed = Space::forDisjunct(scratchSpaces); + partialUncovered = + projectionDecomposed + .minus(coveredSoFar, DC, /*minusCount*/ nullptr) + .getValue(); + diagnoseMissingCases(RequiresDefault::No, partialUncovered, + caseBlock); + } + }; + for (auto *caseBlock : Switch->getCases()) { if (caseBlock->hasUnknownAttr()) { assert(unknownCase == nullptr && "multiple unknown cases"); @@ -972,7 +1035,9 @@ namespace { if (caseItem.isDefault()) return; - Space projection = projectPattern(caseItem.getPattern()); + auto casePattern = caseItem.getPattern(); + Space projection = + projectPattern(casePattern, /*projectAnyPattern*/ nullptr); bool isRedundant = !projection.isEmpty() && llvm::any_of(spaces, [&](const Space &handled) { return projection.isSubspace(handled, DC); @@ -996,8 +1061,10 @@ namespace { continue; } - if (!projection.isEmpty()) + if (!projection.isEmpty()) { + handleUnknownAnyPattern(caseBlock, spaces, casePattern, projection); spaces.push_back(projection); + } } } @@ -1034,7 +1101,8 @@ namespace { if (uncovered.getKind() == SpaceKind::Type) { if (Space::canDecompose(uncovered.getType(), DC)) { SmallVector spaces; - Space::decompose(DC, uncovered.getType(), spaces); + Space::decompose(DC, uncovered.getType(), /*makeUnknownCases*/ true, + spaces); diagnoseMissingCases(RequiresDefault::No, Space::forDisjunct(spaces), unknownCase); } else { @@ -1373,9 +1441,14 @@ namespace { } /// Recursively project a pattern into a Space. - static Space projectPattern(const Pattern *item) { + static Space projectPattern( + const Pattern *item, + llvm::function_ref projectAnyPattern) { switch (item->getKind()) { case PatternKind::Any: + if (projectAnyPattern) { + return projectAnyPattern(cast(item)); + } return Space::forType(item->getType(), Identifier()); case PatternKind::Named: return Space::forType(item->getType(), @@ -1389,7 +1462,7 @@ namespace { case CheckedCastKind::BridgingCoercion: { if (auto *subPattern = IP->getSubPattern()) { // Project the cast target's subpattern. - Space castSubSpace = projectPattern(subPattern); + Space castSubSpace = projectPattern(subPattern, projectAnyPattern); // If we recieved a type space from a named pattern or a wildcard // we have to re-project with the cast's target type to maintain // consistency with the scrutinee's type. @@ -1420,18 +1493,18 @@ namespace { return Space(); case PatternKind::Binding: { auto *VP = cast(item); - return projectPattern(VP->getSubPattern()); + return projectPattern(VP->getSubPattern(), projectAnyPattern); } case PatternKind::Paren: { auto *PP = cast(item); - return projectPattern(PP->getSubPattern()); + return projectPattern(PP->getSubPattern(), projectAnyPattern); } case PatternKind::OptionalSome: { auto *OSP = cast(item); auto &Ctx = OSP->getElementDecl()->getASTContext(); const Identifier name = Ctx.getOptionalSomeDecl()->getBaseIdentifier(); - auto subSpace = projectPattern(OSP->getSubPattern()); + auto subSpace = projectPattern(OSP->getSubPattern(), projectAnyPattern); // To match patterns like (_, _, ...)?, we must rewrite the underlying // tuple pattern to .some(_, _, ...) first. if (subSpace.getKind() == SpaceKind::Constructor && @@ -1458,7 +1531,8 @@ namespace { auto *TP = dyn_cast(SP); llvm::transform(TP->getElements(), std::back_inserter(conArgSpace), [&](TuplePatternElt pate) { - return projectPattern(pate.getPattern()); + return projectPattern(pate.getPattern(), + projectAnyPattern); }); // FIXME: Compound names. return Space::forConstructor(item->getType(), @@ -1482,9 +1556,9 @@ namespace { if (auto *TTy = outerType->getAs()) Space::getTupleTypeSpaces(outerType, TTy, conArgSpace); else - conArgSpace.push_back(projectPattern(SP)); + conArgSpace.push_back(projectPattern(SP, projectAnyPattern)); } else if (SP->getKind() == PatternKind::Tuple) { - Space argTupleSpace = projectPattern(SP); + Space argTupleSpace = projectPattern(SP, projectAnyPattern); // Tuples are modeled as if they are enums with a single, nameless // case, which means argTupleSpace will either be a Constructor or // Empty space. If it's empty (i.e. it contributes nothing to the @@ -1494,7 +1568,7 @@ namespace { assert(argTupleSpace.getKind() == SpaceKind::Constructor); conArgSpace.push_back(argTupleSpace); } else { - conArgSpace.push_back(projectPattern(SP)); + conArgSpace.push_back(projectPattern(SP, projectAnyPattern)); } // FIXME: Compound names. return Space::forConstructor(item->getType(), @@ -1502,7 +1576,7 @@ namespace { conArgSpace); } default: - return projectPattern(SP); + return projectPattern(SP, projectAnyPattern); } } case PatternKind::Tuple: { @@ -1510,7 +1584,8 @@ namespace { SmallVector conArgSpace; llvm::transform(TP->getElements(), std::back_inserter(conArgSpace), [&](TuplePatternElt pate) { - return projectPattern(pate.getPattern()); + return projectPattern(pate.getPattern(), + projectAnyPattern); }); return Space::forConstructor(item->getType(), Identifier(), conArgSpace); diff --git a/test/Sema/exhaustive_switch.swift b/test/Sema/exhaustive_switch.swift index 3d74dee71cc22..fd2860b842dec 100644 --- a/test/Sema/exhaustive_switch.swift +++ b/test/Sema/exhaustive_switch.swift @@ -1450,3 +1450,65 @@ func sr12412() { case (e: .y, b: true)?: break } } + +func nestedUnknown(_ e: NonFrozenEnum) { + switch (e, e) { // expected-warning{{switch must be exhaustive}} + // expected-note@-1{{add missing case: '(.a, .a)'}} + // expected-note@-2{{add missing case: '(.b, .a)'}} + // expected-note@-3{{add missing case: '(.c, .a)'}} + case (@unknown _, .a): () + default: () + } + switch (e, e) { // expected-warning{{switch must be exhaustive}} + // expected-note@-1{{add missing case: '(.c, _)'}} + // expected-note@-2{{add missing case: '(.b, _)'}} + // expected-note@-3{{add missing case: '(.a, _)'}} + case (@unknown _, _): () + } + switch (e, e) { // expected-warning{{switch must be exhaustive}} + // expected-note@-1{{add missing case: '(.a, .a)'}} + // expected-note@-2{{add missing case: '(.b, .a)'}} + // expected-note@-3{{add missing case: '(.c, .a)'}} + // expected-note@-4{{add missing case: '(.a, .b)'}} + // expected-note@-5{{add missing case: '(.b, .b)'}} + // expected-note@-6{{add missing case: '(.c, .b)'}} + // expected-note@-7{{add missing case: '(.a, .c)'}} + // expected-note@-8{{add missing case: '(.b, .c)'}} + // expected-note@-9{{add missing case: '(.c, .c)'}} + case (@unknown _, @unknown _): () + } + switch (e, e) { // expected-warning{{switch must be exhaustive}} + // expected-note@-1{{add missing case: '(.b, .a)'}} + // expected-note@-2{{add missing case: '(.c, .a)'}} + case (.a, _): () + case (@unknown _, .a): () + default: () + } + switch (e, e) { // expected-warning{{switch must be exhaustive}} + // expected-note@-1{{add missing case: '(.b, .a)'}} + // expected-note@-2{{add missing case: '(.c, .a)'}} + case (.a, _), (@unknown _, .a): () + default: () + } + switch Optional.some(e) { // expected-warning{{switch must be exhaustive}} + // expected-note@-1{{add missing case: '.some(.a)'}} + // expected-note@-2{{add missing case: '.some(.b)'}} + // expected-note@-3{{add missing case: '.some(.c)'}} + case .some(@unknown _): () + case .none: () + } +} + +// Check that statements are ill-formed + +let (@unknown _) = NonFrozenEnum.a +// expected-error@-1{{expected pattern}} +// expected-error@-2{{cannot convert value}} +let @unknown _ = NonFrozenEnum.a +// expected-error@-1{{expected pattern}} +for @unknown _ in [NonFrozenEnum.a] {} +// expected-error@-1{{'_' can only appear in a pattern or on the left side of an assignment}} +// expected-error@-2{{expected pattern}} +// expected-error@-3{{expected '{' to start the body of for-each loop}} +if let @unknown _ = .some(NonFrozenEnum.a) {} +// FIXME: [Varun] Not sure where to issue error in Parse/Sema for above code diff --git a/utils/gyb_syntax_support/ExprNodes.py b/utils/gyb_syntax_support/ExprNodes.py index 59801e8e98244..e3f95edead866 100644 --- a/utils/gyb_syntax_support/ExprNodes.py +++ b/utils/gyb_syntax_support/ExprNodes.py @@ -104,6 +104,8 @@ # A _ expression. Node('DiscardAssignmentExpr', kind='Expr', children=[ + Child('Unknown', kind='Attribute', + is_optional=True), Child('Wildcard', kind='WildcardToken'), ]), diff --git a/utils/gyb_syntax_support/PatternNodes.py b/utils/gyb_syntax_support/PatternNodes.py index 454c950ec6e8c..a76dfb43ff1b7 100644 --- a/utils/gyb_syntax_support/PatternNodes.py +++ b/utils/gyb_syntax_support/PatternNodes.py @@ -66,6 +66,8 @@ # wildcard-pattern -> '_' type-annotation? Node('WildcardPattern', kind='Pattern', children=[ + Child('Unknown', kind='Attribute', + is_optional=True), Child('Wildcard', kind='WildcardToken'), Child('TypeAnnotation', kind='TypeAnnotation', is_optional=True), From 0ab1f9c2e38fdaa28fb5ea1eb47a99a86882ef01 Mon Sep 17 00:00:00 2001 From: Daniel Sweeney Date: Thu, 8 Apr 2021 17:23:16 -0600 Subject: [PATCH 2/4] Added parsing tests for nested cases in switch statements. --- test/Parse/switch.swift | 109 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 106 insertions(+), 3 deletions(-) diff --git a/test/Parse/switch.swift b/test/Parse/switch.swift index feea31d5caa7d..2cc3aac35957d 100644 --- a/test/Parse/switch.swift +++ b/test/Parse/switch.swift @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift +// RUN: %target-typecheck-verify-swift -enable-library-evolution // TODO: Implement tuple equality in the library. // BLOCKED: @@ -227,12 +227,12 @@ case (_, 2), (let a, _): // expected-error {{'a' must be bound in every pattern} // OK case (_, 2), (1, _): () - + case (_, var a), (_, var a): // expected-warning {{variable 'a' was never used; consider replacing with '_' or removing it}} // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} // expected-warning@-2 {{case is already handled by previous patterns; consider removing it}} () - + case (var a, var b), (var b, var a): // expected-warning {{variable 'a' was never used; consider replacing with '_' or removing it}} expected-warning {{variable 'b' was never used; consider replacing with '_' or removing it}} // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} // expected-warning@-2 {{case is already handled by previous patterns; consider removing it}} @@ -645,3 +645,106 @@ func testIncompleteArrayLiteral() { () } } + +// Note that library evolution is enabled above. +// Other enums in this file are nonpublic so already @frozen. +public enum E { + case a + case b +} + +func switchTwoWithAtUnknown(_ x: E, _ y: E) { + +// happy path, no diagnostics + switch (x, y) { + case (.a, .a), (.a, .b), (.b, .a), (.b, .b): () + case (@unknown _, .a): () + case (@unknown _, .b): () + case (.a, @unknown _): () + case (.b, @unknown _): () + case (@unknown _, @unknown _): () + default: () + } +// note order-dependency--if `case (@unknown _, @unknown _): ()` is first, diagnostics fire +// I _think_ this is right for the case is already handled--the earlier cases do handle the patterns +// but the switch is exhaustive in this case, that warning/note should not fire + switch (x, y) { // expected-warning {{switch must be exhaustive}} expected-note {{add missing case: '(.a, .a)'}} expected-note {{add missing case: '(.b, .a)'}} expected-note {{add missing case: '(.a, .b)'}} expected-note {{add missing case: '(.b, .b)'}} + case (@unknown _, @unknown _): () + case (.a, .a), (.a, .b), (.b, .a), (.b, .b): () // expected-warning {{case is already handled by previous patterns; consider removing it}} expected-warning {{case is already handled by previous patterns; consider removing it}} expected-warning {{case is already handled by previous patterns; consider removing it}} expected-warning {{case is already handled by previous patterns; consider removing it}} + case (@unknown _, .a): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + case (@unknown _, .b): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + case (.a, @unknown _): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + case (.b, @unknown _): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + default: () + } + + switch (x, y) { // expected-warning {{switch must be exhaustive}} expected-note {{add missing case: '(.a, .a)'}} expected-note {{add missing case: '(.b, .a)'}} + case (@unknown _, .a): () + case (.a, .a), (.a, .b), (.b, .a), (.b, .b): () // expected-warning {{case is already handled by previous patterns; consider removing it}} expected-warning {{case is already handled by previous patterns; consider removing it}} + case (@unknown _, .b): () + case (.a, @unknown _): () + case (.b, @unknown _): () + case (@unknown _, @unknown _): () + default: () + } + // simple missing cases + switch (x, y) { // expected-warning {{switch must be exhaustive}} expected-note {{add missing case: '(.a, .a)'}} + case (.b, .a): () + case (@unknown _, .a): () + default: () + } + + switch (x, y) { // expected-warning {{switch must be exhaustive}} expected-note {{add missing case: '(.a, .a)'}} + case (.b, _): () + case (@unknown _, .a): () + default: () + } + + switch (x, y) { // expected-warning {{switch must be exhaustive}} expected-note {{add missing case: '(.a, _)'}} expected-note {{add missing case: '(.b, _)'}} + case (@unknown _, _): () + } +// cases after `default:` + switch (x, y) { + default: () + case (.a, .a), (.a, .b), (.b, .a), (.b, .b): () // expected-error {{additional 'case' blocks cannot appear after the 'default' block of a 'switch'}} + case (@unknown _, .a): () + case (@unknown _, .b): () + case (.a, @unknown _): () + case (.b, @unknown _): () + case (@unknown _, @unknown _): () + default: () + } +// cases after `case _:`, subtly different from `default`. This is correct. + switch (x, y) { + case _: () + case (.a, .a), (.a, .b), (.b, .a), (.b, .b): () // expected-warning {{case is already handled by previous patterns; consider removing it}} expected-warning {{case is already handled by previous patterns; consider removing it}} expected-warning {{case is already handled by previous patterns; consider removing it}} expected-warning {{case is already handled by previous patterns; consider removing it}} + case (@unknown _, .a): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + case (@unknown _, .b): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + case (.a, @unknown _): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + case (.b, @unknown _): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + case (@unknown _, @unknown _): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + default: () + } +} + +func switchTwoWithAtUnknown(_ x: E, _ y: E, _ z: E) { +// should be happy path, no diagnostics +// but the non-unknown cases are covering the unknown cases, so some extra diagnostics are appearing. +// Unless I am m + switch (x, (y, z)) { + case (.a, (.a, .b)), + (.a, (.b, .a)), + (.b, (.b, .a)), + (.b, (.a, .b)): () + case (.a, _): () + case (.b, _): () + case (_, (.a, .b)): () + case (@unknown _, (.a, .b)): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + case (@unknown _, (.b, .a)): () + case (.a, (@unknown _, .a)): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + case (.b, (.a, @unknown _)): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + case (@unknown _, (@unknown _, @unknown _)): () + default: () + } + +} From 08a9ddc3cb6b986464e53f41088e0161fdec8a26 Mon Sep 17 00:00:00 2001 From: Daniel Sweeney Date: Thu, 8 Apr 2021 17:33:42 -0600 Subject: [PATCH 3/4] Fixed two typos. --- test/Parse/switch.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Parse/switch.swift b/test/Parse/switch.swift index 2cc3aac35957d..e8198fbf82351 100644 --- a/test/Parse/switch.swift +++ b/test/Parse/switch.swift @@ -727,10 +727,10 @@ func switchTwoWithAtUnknown(_ x: E, _ y: E) { } } -func switchTwoWithAtUnknown(_ x: E, _ y: E, _ z: E) { +func switchThreeWithAtUnknown(_ x: E, _ y: E, _ z: E) { // should be happy path, no diagnostics // but the non-unknown cases are covering the unknown cases, so some extra diagnostics are appearing. -// Unless I am m +// unless I am misunderstanding something. switch (x, (y, z)) { case (.a, (.a, .b)), (.a, (.b, .a)), From fe8c3a03c70ccd05445889b9d8200d5917ae3a81 Mon Sep 17 00:00:00 2001 From: Daniel Sweeney Date: Fri, 9 Apr 2021 20:56:14 -0600 Subject: [PATCH 4/4] Moved some expectations to separate lines to make the line lengths more reasonable. --- test/Parse/switch.swift | 79 ++++++++++++++++++++++++++++------------- 1 file changed, 55 insertions(+), 24 deletions(-) diff --git a/test/Parse/switch.swift b/test/Parse/switch.swift index e8198fbf82351..4cfc324ab7793 100644 --- a/test/Parse/switch.swift +++ b/test/Parse/switch.swift @@ -102,8 +102,6 @@ case 1: x = 0 } - - switch x { x = 1 // expected-error{{all statements inside a switch must be covered by a 'case' or 'default'}} default: @@ -668,19 +666,35 @@ func switchTwoWithAtUnknown(_ x: E, _ y: E) { // note order-dependency--if `case (@unknown _, @unknown _): ()` is first, diagnostics fire // I _think_ this is right for the case is already handled--the earlier cases do handle the patterns // but the switch is exhaustive in this case, that warning/note should not fire - switch (x, y) { // expected-warning {{switch must be exhaustive}} expected-note {{add missing case: '(.a, .a)'}} expected-note {{add missing case: '(.b, .a)'}} expected-note {{add missing case: '(.a, .b)'}} expected-note {{add missing case: '(.b, .b)'}} + switch (x, y) { // expected-warning {{switch must be exhaustive}} + // expected-note@-1 {{add missing case: '(.a, .a)'}} + // expected-note@-2 {{add missing case: '(.b, .a)'}} + // expected-note@-3 {{add missing case: '(.a, .b)'}} + // expected-note@-4 {{add missing case: '(.b, .b)'}} case (@unknown _, @unknown _): () - case (.a, .a), (.a, .b), (.b, .a), (.b, .b): () // expected-warning {{case is already handled by previous patterns; consider removing it}} expected-warning {{case is already handled by previous patterns; consider removing it}} expected-warning {{case is already handled by previous patterns; consider removing it}} expected-warning {{case is already handled by previous patterns; consider removing it}} - case (@unknown _, .a): () // expected-warning {{case is already handled by previous patterns; consider removing it}} - case (@unknown _, .b): () // expected-warning {{case is already handled by previous patterns; consider removing it}} - case (.a, @unknown _): () // expected-warning {{case is already handled by previous patterns; consider removing it}} - case (.b, @unknown _): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + case (.a, .a), (.a, .b), (.b, .a), (.b, .b): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} + // expected-warning@-2 {{case is already handled by previous patterns; consider removing it}} + // expected-warning@-3 {{case is already handled by previous patterns; consider removing it}} + // expected-warning@-4 {{case is already handled by previous patterns; consider removing it}} + case (@unknown _, .a): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} + case (@unknown _, .b): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} + case (.a, @unknown _): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} + case (.b, @unknown _): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} default: () } - switch (x, y) { // expected-warning {{switch must be exhaustive}} expected-note {{add missing case: '(.a, .a)'}} expected-note {{add missing case: '(.b, .a)'}} + switch (x, y) { // expected-warning {{switch must be exhaustive}} + // expected-note@-1 {{add missing case: '(.a, .a)'}} + // expected-note@-2 {{add missing case: '(.b, .a)'}} case (@unknown _, .a): () - case (.a, .a), (.a, .b), (.b, .a), (.b, .b): () // expected-warning {{case is already handled by previous patterns; consider removing it}} expected-warning {{case is already handled by previous patterns; consider removing it}} + case (.a, .a), (.a, .b), (.b, .a), (.b, .b): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} + // expected-warning@-2 {{case is already handled by previous patterns; consider removing it}} case (@unknown _, .b): () case (.a, @unknown _): () case (.b, @unknown _): () @@ -688,25 +702,30 @@ func switchTwoWithAtUnknown(_ x: E, _ y: E) { default: () } // simple missing cases - switch (x, y) { // expected-warning {{switch must be exhaustive}} expected-note {{add missing case: '(.a, .a)'}} + switch (x, y) { // expected-warning {{switch must be exhaustive}} + // expected-note@-1 {{add missing case: '(.a, .a)'}} case (.b, .a): () case (@unknown _, .a): () default: () } - switch (x, y) { // expected-warning {{switch must be exhaustive}} expected-note {{add missing case: '(.a, .a)'}} + switch (x, y) { // expected-warning {{switch must be exhaustive}} + // expected-note@-1 {{add missing case: '(.a, .a)'}} case (.b, _): () case (@unknown _, .a): () default: () } - switch (x, y) { // expected-warning {{switch must be exhaustive}} expected-note {{add missing case: '(.a, _)'}} expected-note {{add missing case: '(.b, _)'}} + switch (x, y) { // expected-warning {{switch must be exhaustive}} + // expected-note@-1 {{add missing case: '(.a, _)'}} + // expected-note@-2 {{add missing case: '(.b, _)'}} case (@unknown _, _): () } // cases after `default:` switch (x, y) { default: () - case (.a, .a), (.a, .b), (.b, .a), (.b, .b): () // expected-error {{additional 'case' blocks cannot appear after the 'default' block of a 'switch'}} + case (.a, .a), (.a, .b), (.b, .a), (.b, .b): () + // expected-error@-1 {{additional 'case' blocks cannot appear after the 'default' block of a 'switch'}} case (@unknown _, .a): () case (@unknown _, .b): () case (.a, @unknown _): () @@ -714,15 +733,24 @@ func switchTwoWithAtUnknown(_ x: E, _ y: E) { case (@unknown _, @unknown _): () default: () } -// cases after `case _:`, subtly different from `default`. This is correct. +// cases after `case _:`, subtly different from `default`. This is correct I think. switch (x, y) { case _: () - case (.a, .a), (.a, .b), (.b, .a), (.b, .b): () // expected-warning {{case is already handled by previous patterns; consider removing it}} expected-warning {{case is already handled by previous patterns; consider removing it}} expected-warning {{case is already handled by previous patterns; consider removing it}} expected-warning {{case is already handled by previous patterns; consider removing it}} - case (@unknown _, .a): () // expected-warning {{case is already handled by previous patterns; consider removing it}} - case (@unknown _, .b): () // expected-warning {{case is already handled by previous patterns; consider removing it}} - case (.a, @unknown _): () // expected-warning {{case is already handled by previous patterns; consider removing it}} - case (.b, @unknown _): () // expected-warning {{case is already handled by previous patterns; consider removing it}} - case (@unknown _, @unknown _): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + case (.a, .a), (.a, .b), (.b, .a), (.b, .b): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} + // expected-warning@-2 {{case is already handled by previous patterns; consider removing it}} + // expected-warning@-3 {{case is already handled by previous patterns; consider removing it}} + // expected-warning@-4 {{case is already handled by previous patterns; consider removing it}} + case (@unknown _, .a): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} + case (@unknown _, .b): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} + case (.a, @unknown _): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} + case (.b, @unknown _): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} + case (@unknown _, @unknown _): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} default: () } } @@ -739,10 +767,13 @@ func switchThreeWithAtUnknown(_ x: E, _ y: E, _ z: E) { case (.a, _): () case (.b, _): () case (_, (.a, .b)): () - case (@unknown _, (.a, .b)): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + case (@unknown _, (.a, .b)): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} case (@unknown _, (.b, .a)): () - case (.a, (@unknown _, .a)): () // expected-warning {{case is already handled by previous patterns; consider removing it}} - case (.b, (.a, @unknown _)): () // expected-warning {{case is already handled by previous patterns; consider removing it}} + case (.a, (@unknown _, .a)): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} + case (.b, (.a, @unknown _)): () + // expected-warning@-1 {{case is already handled by previous patterns; consider removing it}} case (@unknown _, (@unknown _, @unknown _)): () default: () }