Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -3726,11 +3726,9 @@ Stmt *InitStmt, ConditionResult Cond, Stmt *ThenVal, SourceLocation ElseLoc, Stmt *ElseVal); - StmtResult ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, - Stmt *InitStmt, - ConditionResult Cond); - StmtResult ActOnFinishSwitchStmt(SourceLocation SwitchLoc, - Stmt *Switch, Stmt *Body); + void ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, Stmt *InitStmt, + ConditionResult Cond); + StmtResult ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Body); StmtResult ActOnWhileStmt(SourceLocation WhileLoc, ConditionResult Cond, Stmt *Body); StmtResult ActOnDoStmt(SourceLocation DoLoc, Stmt *Body, Index: lib/Parse/ParseStmt.cpp =================================================================== --- lib/Parse/ParseStmt.cpp +++ lib/Parse/ParseStmt.cpp @@ -788,6 +788,9 @@ if (SubStmt.isInvalid()) SubStmt = Actions.ActOnNullStmt(SourceLocation()); Actions.ActOnCaseStmtBody(DeepestParsedCaseStmt, SubStmt.get()); + } else { + // The case statement is invalid, recover by returning the statement body. + return SubStmt; } // Return the top level parsed statement tree. @@ -1306,21 +1309,7 @@ Sema::ConditionKind::Switch)) return StmtError(); - StmtResult Switch = - Actions.ActOnStartOfSwitchStmt(SwitchLoc, InitStmt.get(), Cond); - - if (Switch.isInvalid()) { - // Skip the switch body. - // FIXME: This is not optimal recovery, but parsing the body is more - // dangerous due to the presence of case and default statements, which - // will have no place to connect back with the switch. - if (Tok.is(tok::l_brace)) { - ConsumeBrace(); - SkipUntil(tok::r_brace); - } else - SkipUntil(tok::semi); - return Switch; - } + Actions.ActOnStartOfSwitchStmt(SwitchLoc, InitStmt.get(), Cond); // C99 6.8.4p3 - In C99, the body of the switch statement is a scope, even if // there is no compound stmt. C90 does not have this clause. We only do this @@ -1348,7 +1337,7 @@ InnerScope.Exit(); SwitchScope.Exit(); - return Actions.ActOnFinishSwitchStmt(SwitchLoc, Switch.get(), Body.get()); + return Actions.ActOnFinishSwitchStmt(SwitchLoc, Body.get()); } /// ParseWhileStatement Index: lib/Sema/SemaStmt.cpp =================================================================== --- lib/Sema/SemaStmt.cpp +++ lib/Sema/SemaStmt.cpp @@ -669,17 +669,21 @@ return UsualUnaryConversions(CondResult.get()); } -StmtResult Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, - Stmt *InitStmt, ConditionResult Cond) { +void Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc, Stmt *InitStmt, + ConditionResult Cond) { if (Cond.isInvalid()) - return StmtError(); + Cond = ConditionResult( + *this, nullptr, + MakeFullExpr(new (Context) OpaqueValueExpr(SourceLocation(), + Context.IntTy, VK_RValue), + SwitchLoc), + false); getCurFunction()->setHasBranchIntoScope(); SwitchStmt *SS = new (Context) SwitchStmt(Context, InitStmt, Cond.get().first, Cond.get().second); getCurFunction()->SwitchStack.push_back(SS); - return SS; } static void AdjustAPSInt(llvm::APSInt &Val, unsigned BitWidth, bool IsSigned) { @@ -769,12 +773,11 @@ << Case->getSourceRange(); } -StmtResult -Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch, - Stmt *BodyStmt) { - SwitchStmt *SS = cast(Switch); - assert(SS == getCurFunction()->SwitchStack.back() && - "switch stack missing push/pop!"); +StmtResult Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, + Stmt *BodyStmt) { + + SwitchStmt *SS = getCurFunction()->SwitchStack.back(); + assert(SS && "switch stack missing push/pop!"); getCurFunction()->SwitchStack.pop_back(); @@ -802,7 +805,7 @@ // type, when we started the switch statement. If we don't have an // appropriate type now, just return an error. if (!CondType->isIntegralOrEnumerationType()) - return StmtError(); + return SS; if (CondExpr->isKnownToHaveBooleanValue()) { // switch(bool_expr) {...} is often a programmer error, e.g. @@ -815,17 +818,20 @@ // Get the bitwidth of the switched-on value after promotions. We must // convert the integer case values to this width before comparison. - bool HasDependentValue - = CondExpr->isTypeDependent() || CondExpr->isValueDependent(); - unsigned CondWidth = HasDependentValue ? 0 : Context.getIntWidth(CondType); + bool HasDependentValueOrError = CondExpr->isTypeDependent() || + CondExpr->isValueDependent() || + isa(CondExpr); + unsigned CondWidth = + HasDependentValueOrError ? 0 : Context.getIntWidth(CondType); bool CondIsSigned = CondType->isSignedIntegerOrEnumerationType(); // Get the width and signedness that the condition might actually have, for // warning purposes. // FIXME: Grab an IntRange for the condition rather than using the unpromoted // type. - unsigned CondWidthBeforePromotion - = HasDependentValue ? 0 : Context.getIntWidth(CondTypeBeforePromotion); + unsigned CondWidthBeforePromotion = + HasDependentValueOrError ? 0 + : Context.getIntWidth(CondTypeBeforePromotion); bool CondIsSignedBeforePromotion = CondTypeBeforePromotion->isSignedIntegerOrEnumerationType(); @@ -843,8 +849,8 @@ bool CaseListIsErroneous = false; - for (SwitchCase *SC = SS->getSwitchCaseList(); SC && !HasDependentValue; - SC = SC->getNextSwitchCase()) { + for (SwitchCase *SC = SS->getSwitchCaseList(); + SC && !HasDependentValueOrError; SC = SC->getNextSwitchCase()) { if (DefaultStmt *DS = dyn_cast(SC)) { if (TheDefaultStmt) { @@ -865,7 +871,7 @@ Expr *Lo = CS->getLHS(); if (Lo->isTypeDependent() || Lo->isValueDependent()) { - HasDependentValue = true; + HasDependentValueOrError = true; break; } @@ -908,7 +914,7 @@ if (CS->getRHS()) { if (CS->getRHS()->isTypeDependent() || CS->getRHS()->isValueDependent()) { - HasDependentValue = true; + HasDependentValueOrError = true; break; } CaseRanges.push_back(std::make_pair(LoVal, CS)); @@ -917,12 +923,12 @@ } } - if (!HasDependentValue) { + if (!HasDependentValueOrError) { // If we don't have a default statement, check whether the // condition is constant. llvm::APSInt ConstantCondValue; bool HasConstantCond = false; - if (!HasDependentValue && !TheDefaultStmt) { + if (!HasDependentValueOrError && !TheDefaultStmt) { HasConstantCond = CondExpr->EvaluateAsInt(ConstantCondValue, Context, Expr::SE_AllowSideEffects); assert(!HasConstantCond || @@ -1207,11 +1213,6 @@ DiagnoseEmptyStmtBody(CondExpr->getLocEnd(), BodyStmt, diag::warn_empty_switch_body); - // FIXME: If the case list was broken is some way, we don't have a good system - // to patch it up. Instead, just return the whole substmt as broken. - if (CaseListIsErroneous) - return StmtError(); - return SS; } Index: lib/Sema/TreeTransform.h =================================================================== --- lib/Sema/TreeTransform.h +++ lib/Sema/TreeTransform.h @@ -1256,18 +1256,17 @@ /// /// By default, performs semantic analysis to build the new statement. /// Subclasses may override this routine to provide different behavior. - StmtResult RebuildSwitchStmtStart(SourceLocation SwitchLoc, Stmt *Init, - Sema::ConditionResult Cond) { - return getSema().ActOnStartOfSwitchStmt(SwitchLoc, Init, Cond); + void RebuildSwitchStmtStart(SourceLocation SwitchLoc, Stmt *Init, + Sema::ConditionResult Cond) { + getSema().ActOnStartOfSwitchStmt(SwitchLoc, Init, Cond); } /// \brief Attach the body to the switch statement. /// /// By default, performs semantic analysis to build the new statement. /// Subclasses may override this routine to provide different behavior. - StmtResult RebuildSwitchStmtBody(SourceLocation SwitchLoc, - Stmt *Switch, Stmt *Body) { - return getSema().ActOnFinishSwitchStmt(SwitchLoc, Switch, Body); + StmtResult RebuildSwitchStmtBody(SourceLocation SwitchLoc, Stmt *Body) { + return getSema().ActOnFinishSwitchStmt(SwitchLoc, Body); } /// \brief Build a new while statement. @@ -6657,10 +6656,7 @@ return StmtError(); // Rebuild the switch statement. - StmtResult Switch - = getDerived().RebuildSwitchStmtStart(S->getSwitchLoc(), Init.get(), Cond); - if (Switch.isInvalid()) - return StmtError(); + getDerived().RebuildSwitchStmtStart(S->getSwitchLoc(), Init.get(), Cond); // Transform the body of the switch statement. StmtResult Body = getDerived().TransformStmt(S->getBody()); @@ -6668,8 +6664,7 @@ return StmtError(); // Complete the switch statement. - return getDerived().RebuildSwitchStmtBody(S->getSwitchLoc(), Switch.get(), - Body.get()); + return getDerived().RebuildSwitchStmtBody(S->getSwitchLoc(), Body.get()); } template Index: test/Misc/ast-dump-invalid-switch.cpp =================================================================== --- /dev/null +++ test/Misc/ast-dump-invalid-switch.cpp @@ -0,0 +1,105 @@ +// RUN: not %clang_cc1 -std=c++11 -triple x86_64-linux-gnu -fms-extensions -ast-dump -ast-dump-filter Test %s | FileCheck -check-prefix CHECK -strict-whitespace %s + +/* This test ensures that the AST is still complete, even for invalid code */ + +namespace TestInvalidSwithCondition { +int f(int x) { + switch (_invalid_) { + case 0: + return 1; + default: + return 2; + } +} +} + +// CHECK: NamespaceDecl {{.*}} TestInvalidSwithCondition +// CHECK-NEXT: `-FunctionDecl +// CHECK-NEXT: |-ParmVarDecl +// CHECK-NEXT: `-CompoundStmt +// CHECK-NEXT: `-SwitchStmt +// CHECK-NEXT: |-<<>> +// CHECK-NEXT: |-<<>> +// CHECK-NEXT: |-OpaqueValueExpr +// CHECK-NEXT: `-CompoundStmt +// CHECK-NEXT: |-CaseStmt +// CHECK-NEXT: | |-IntegerLiteral {{.*}} 'int' 0 +// CHECK-NEXT: | |-<<>> +// CHECK-NEXT: | `-ReturnStmt +// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1 +// CHECK-NEXT: `-DefaultStmt +// CHECK-NEXT: `-ReturnStmt +// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 2 + +namespace TestSwitchConditionNotIntegral { +int g(int *x) { + switch (x) { + case 0: + return 1; + default: + return 2; + } +} +} + +// CHECK: NamespaceDecl {{.*}} TestSwitchConditionNotIntegral +// CHECK-NEXT: `-FunctionDecl +// CHECK-NEXT: |-ParmVarDecl +// CHECK-NEXT: `-CompoundStmt +// CHECK-NEXT: `-SwitchStmt +// CHECK-NEXT: |-<<>> +// CHECK-NEXT: |-<<>> +// CHECK-NEXT: |-ImplicitCastExpr +// CHECK-NEXT: | `-DeclRefExpr {{.*}} 'x' 'int *' +// CHECK-NEXT: `-CompoundStmt +// CHECK-NEXT: |-CaseStmt +// CHECK-NEXT: | |-IntegerLiteral {{.*}} 'int' 0 +// CHECK-NEXT: | |-<<>> +// CHECK-NEXT: | `-ReturnStmt +// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1 +// CHECK-NEXT: `-DefaultStmt +// CHECK-NEXT: `-ReturnStmt +// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 2 + +namespace TestSwitchInvalidCases { +int g(int x) { + switch (x) { + case _invalid_: + return 1; + case _invalid_: + return 2; + case x: + return 3; + default: + return 4; + default: + return 5; + } +} +} + +// CHECK: NamespaceDecl {{.*}} TestSwitchInvalidCases +// CHECK-NEXT: `-FunctionDecl +// CHECK-NEXT: |-ParmVarDecl +// CHECK-NEXT: `-CompoundStmt +// CHECK-NEXT: `-SwitchStmt +// CHECK-NEXT: |-<<>> +// CHECK-NEXT: |-<<>> +// CHECK-NEXT: |-ImplicitCastExpr +// CHECK-NEXT: | `-DeclRefExpr {{.*}}'x' 'int' +// CHECK-NEXT: `-CompoundStmt +// CHECK-NEXT: |-ReturnStmt +// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 1 +// CHECK-NEXT: |-ReturnStmt +// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 2 +// CHECK-NEXT: |-CaseStmt +// CHECK-NEXT: | |-DeclRefExpr {{.*}} 'x' 'int' +// CHECK-NEXT: | |-<<>> +// CHECK-NEXT: | `-ReturnStmt +// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 3 +// CHECK-NEXT: |-DefaultStmt +// CHECK-NEXT: | `-ReturnStmt +// CHECK-NEXT: | `-IntegerLiteral {{.*}} 'int' 4 +// CHECK-NEXT: `-DefaultStmt +// CHECK-NEXT: `-ReturnStmt +// CHECK-NEXT: `-IntegerLiteral {{.*}} 'int' 5 Index: test/SemaCXX/switch.cpp =================================================================== --- test/SemaCXX/switch.cpp +++ test/SemaCXX/switch.cpp @@ -130,3 +130,20 @@ } } + +namespace InvalidCondition { +enum class color { red, + blue, + green }; +void test() { + // When the condition is invalid, there should be no errors or warnings + switch (invalidCode) { // expected-error {{use of undeclared identifier}} + case 0: + case -(1ll << 62) - 1: + case (1ll << 62) + 1: + case color::red: + default: + break; + } +} +} // namespace InvalidCondition