From 6d5195b9d5e2e9e9f3ef05f9c6b8b28fbcfdef61 Mon Sep 17 00:00:00 2001
From: Alp Toker <alp@nuanti.com>
Date: Fri, 9 May 2014 08:40:10 +0000
Subject: [PATCH] Add support for partial jump scope checking
This lets us diagnose and perform more complete semantic analysis when faced
with errors in the function body or declaration.
By recovering here we provide more consistent diagnostics, particularly during
interactive editing.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@208394 91177308-0d34-0410-b5e6-96231b3b80d8
---
lib/Sema/JumpDiagnostics.cpp | 33 ++++++++++++++++++----------
lib/Sema/SemaDecl.cpp | 2 --
lib/Sema/SemaExpr.cpp | 1 -
test/SemaCXX/scope-check.cpp | 42 ++++++++++++++++++++++++++----------
4 files changed, 53 insertions(+), 25 deletions(-)
diff --git a/lib/Sema/JumpDiagnostics.cpp b/lib/Sema/JumpDiagnostics.cpp
index 1f5d682bba1..27bca15b4c1 100644
--- a/lib/Sema/JumpDiagnostics.cpp
+++ b/lib/Sema/JumpDiagnostics.cpp
@@ -32,6 +32,10 @@ namespace {
class JumpScopeChecker {
Sema &S;
+ /// Permissive - True when recovering from errors, in which case precautions
+ /// are taken to handle incomplete scope information.
+ const bool Permissive;
+
/// GotoScope - This is a record that we use to keep track of all of the
/// scopes that are introduced by VLAs and other things that scope jumps like
/// gotos. This scope tree has nothing to do with the source scope tree,
@@ -85,8 +89,10 @@ private:
};
} // end anonymous namespace
+#define CHECK_PERMISSIVE(x) (assert(Permissive || !(x)), (Permissive && (x)))
-JumpScopeChecker::JumpScopeChecker(Stmt *Body, Sema &s) : S(s) {
+JumpScopeChecker::JumpScopeChecker(Stmt *Body, Sema &s)
+ : S(s), Permissive(s.hasAnyUnrecoverableErrorsInThisFunction()) {
// Add a scope entry for function scope.
Scopes.push_back(GotoScope(~0U, ~0U, ~0U, SourceLocation()));
@@ -503,7 +509,8 @@ void JumpScopeChecker::VerifyJumps() {
SwitchStmt *SS = cast<SwitchStmt>(Jump);
for (SwitchCase *SC = SS->getSwitchCaseList(); SC;
SC = SC->getNextSwitchCase()) {
- assert(LabelAndGotoScopes.count(SC) && "Case not visited?");
+ if (CHECK_PERMISSIVE(!LabelAndGotoScopes.count(SC)))
+ continue;
SourceLocation Loc;
if (CaseStmt *CS = dyn_cast<CaseStmt>(SC))
Loc = CS->getLocStart();
@@ -557,8 +564,8 @@ void JumpScopeChecker::VerifyIndirectJumps() {
for (SmallVectorImpl<IndirectGotoStmt*>::iterator
I = IndirectJumps.begin(), E = IndirectJumps.end(); I != E; ++I) {
IndirectGotoStmt *IG = *I;
- assert(LabelAndGotoScopes.count(IG) &&
- "indirect jump didn't get added to scopes?");
+ if (CHECK_PERMISSIVE(!LabelAndGotoScopes.count(IG)))
+ continue;
unsigned IGScope = LabelAndGotoScopes[IG];
IndirectGotoStmt *&Entry = JumpScopesMap[IGScope];
if (!Entry) Entry = IG;
@@ -577,8 +584,8 @@ void JumpScopeChecker::VerifyIndirectJumps() {
I = IndirectJumpTargets.begin(), E = IndirectJumpTargets.end();
I != E; ++I) {
LabelDecl *TheLabel = *I;
- assert(LabelAndGotoScopes.count(TheLabel->getStmt()) &&
- "Referenced label didn't get added to scopes?");
+ if (CHECK_PERMISSIVE(!LabelAndGotoScopes.count(TheLabel->getStmt())))
+ continue;
unsigned LabelScope = LabelAndGotoScopes[TheLabel->getStmt()];
LabelDecl *&Target = TargetScopes[LabelScope];
if (!Target) Target = TheLabel;
@@ -683,7 +690,8 @@ static void DiagnoseIndirectJumpStmt(Sema &S, IndirectGotoStmt *Jump,
/// Produce note diagnostics for a jump into a protected scope.
void JumpScopeChecker::NoteJumpIntoScopes(ArrayRef<unsigned> ToScopes) {
- assert(!ToScopes.empty());
+ if (CHECK_PERMISSIVE(ToScopes.empty()))
+ return;
for (unsigned I = 0, E = ToScopes.size(); I != E; ++I)
if (Scopes[ToScopes[I]].InDiag)
S.Diag(Scopes[ToScopes[I]].Loc, Scopes[ToScopes[I]].InDiag);
@@ -694,7 +702,8 @@ void JumpScopeChecker::DiagnoseIndirectJump(IndirectGotoStmt *Jump,
unsigned JumpScope,
LabelDecl *Target,
unsigned TargetScope) {
- assert(JumpScope != TargetScope);
+ if (CHECK_PERMISSIVE(JumpScope == TargetScope))
+ return;
unsigned Common = GetDeepestCommonScope(JumpScope, TargetScope);
bool Diagnosed = false;
@@ -731,10 +740,12 @@ void JumpScopeChecker::DiagnoseIndirectJump(IndirectGotoStmt *Jump,
void JumpScopeChecker::CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc,
unsigned JumpDiagError, unsigned JumpDiagWarning,
unsigned JumpDiagCXX98Compat) {
- assert(LabelAndGotoScopes.count(From) && "Jump didn't get added to scopes?");
- unsigned FromScope = LabelAndGotoScopes[From];
+ if (CHECK_PERMISSIVE(!LabelAndGotoScopes.count(From)))
+ return;
+ if (CHECK_PERMISSIVE(!LabelAndGotoScopes.count(To)))
+ return;
- assert(LabelAndGotoScopes.count(To) && "Jump didn't get added to scopes?");
+ unsigned FromScope = LabelAndGotoScopes[From];
unsigned ToScope = LabelAndGotoScopes[To];
// Common case: exactly the same scope, which is fine.
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 0d448e0aa87..0ffce40e1ea 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -9997,8 +9997,6 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
// Verify that gotos and switch cases don't jump into scopes illegally.
if (getCurFunction()->NeedsScopeChecking() &&
- !dcl->isInvalidDecl() &&
- !hasAnyUnrecoverableErrorsInThisFunction() &&
!PP.isCodeCompletionEnabled())
DiagnoseInvalidJumps(Body);
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 979c28cbd87..3d90f0a655c 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -10558,7 +10558,6 @@ ExprResult Sema::ActOnBlockStmtExpr(SourceLocation CaretLoc,
// If needed, diagnose invalid gotos and switches in the block.
if (getCurFunction()->NeedsScopeChecking() &&
- !hasAnyUnrecoverableErrorsInThisFunction() &&
!PP.isCodeCompletionEnabled())
DiagnoseInvalidJumps(cast<CompoundStmt>(Body));
diff --git a/test/SemaCXX/scope-check.cpp b/test/SemaCXX/scope-check.cpp
index c5fdb09f237..dc15dc8b3e6 100644
--- a/test/SemaCXX/scope-check.cpp
+++ b/test/SemaCXX/scope-check.cpp
@@ -1,6 +1,18 @@
// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -fcxx-exceptions %s -Wno-unreachable-code
// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -fcxx-exceptions -std=gnu++11 %s -Wno-unreachable-code
+namespace testInvalid {
+Invalid inv; // expected-error {{unknown type name}}
+// Make sure this doesn't assert.
+void fn()
+{
+ int c = 0;
+ if (inv)
+Here: ;
+ goto Here;
+}
+}
+
namespace test0 {
struct D { ~D(); };
@@ -409,15 +421,23 @@ namespace PR18217 {
}
}
-// This test must be last, because the error prohibits further jump diagnostics.
-namespace testInvalid {
-Invalid inv; // expected-error {{unknown type name}}
-// Make sure this doesn't assert.
-void fn()
-{
- int c = 0;
- if (inv)
-Here: ;
- goto Here;
-}
+namespace test_recovery {
+ // Test that jump scope checking recovers when there are unspecified errors
+ // in the function declaration or body.
+
+ void test(nexist, int c) { // expected-error {{}}
+ nexist_fn(); // expected-error {{}}
+ goto nexist_label; // expected-error {{use of undeclared label}}
+ goto a0; // expected-error {{goto into protected scope}}
+ int a = 0; // expected-note {{jump bypasses variable initialization}}
+ a0:;
+
+ switch (c) {
+ case $: // expected-error {{}}
+ case 0:
+ int x = 56; // expected-note {{jump bypasses variable initialization}}
+ case 1: // expected-error {{switch case is in protected scope}}
+ x = 10;
+ }
+ }
}
--
GitLab