ogoffart updated this revision to Diff 119142.
ogoffart added a comment.
Updated the patch so that ActOnStartOfSwitchStmt returns void, and
ActOnFinishSwitchStmt will skip some checks in case of error
https://reviews.llvm.org/D26350
Files:
include/clang/Sema/Sema.h
lib/Parse/ParseStmt.cpp
lib/Sema/SemaStmt.cpp
lib/Sema/TreeTransform.h
test/Misc/ast-dump-invalid-switch.cpp
test/SemaCXX/switch.cpp
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
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: |-<<<NULL>>>
+// CHECK-NEXT: |-<<<NULL>>>
+// CHECK-NEXT: |-OpaqueValueExpr
+// CHECK-NEXT: `-CompoundStmt
+// CHECK-NEXT: |-CaseStmt
+// CHECK-NEXT: | |-IntegerLiteral {{.*}} 'int' 0
+// CHECK-NEXT: | |-<<<NULL>>>
+// 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: |-<<<NULL>>>
+// CHECK-NEXT: |-<<<NULL>>>
+// CHECK-NEXT: |-ImplicitCastExpr
+// CHECK-NEXT: | `-DeclRefExpr {{.*}} 'x' 'int *'
+// CHECK-NEXT: `-CompoundStmt
+// CHECK-NEXT: |-CaseStmt
+// CHECK-NEXT: | |-IntegerLiteral {{.*}} 'int' 0
+// CHECK-NEXT: | |-<<<NULL>>>
+// 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: |-<<<NULL>>>
+// CHECK-NEXT: |-<<<NULL>>>
+// 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: | |-<<<NULL>>>
+// 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: 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,19 +6656,15 @@
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());
if (Body.isInvalid())
return StmtError();
// Complete the switch statement.
- return getDerived().RebuildSwitchStmtBody(S->getSwitchLoc(), Switch.get(),
- Body.get());
+ return getDerived().RebuildSwitchStmtBody(S->getSwitchLoc(), Body.get());
}
template<typename Derived>
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<SwitchStmt>(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<OpaqueValueExpr>(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<DefaultStmt>(SC)) {
if (TheDefaultStmt) {
@@ -865,7 +871,7 @@
Expr *Lo = CS->getLHS();
if (Lo->isTypeDependent() || Lo->isValueDependent()) {
- HasDependentValue = true;
+ HasDependentValueOrError = true;
break;
}
@@ -908,21 +914,21 @@
if (CS->getRHS()) {
if (CS->getRHS()->isTypeDependent() ||
CS->getRHS()->isValueDependent()) {
- HasDependentValue = true;
+ HasDependentValueOrError = true;
break;
}
CaseRanges.push_back(std::make_pair(LoVal, CS));
} else
CaseVals.push_back(std::make_pair(LoVal, CS));
}
}
- 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/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: 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,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
