[PATCH] Jump scope checker recovery
Alp Toker
alp at nuanti.com
Tue Apr 29 22:24:04 PDT 2014
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.
In particular this improves the interactive editing experience where
jump diagnostics were appearing and disappearing as the user typed.
Alp.
--
http://www.nuanti.com
the browser experts
-------------- next part --------------
diff --git a/lib/Sema/JumpDiagnostics.cpp b/lib/Sema/JumpDiagnostics.cpp
index 1f5d682..27bca15 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 4208421..37663b0 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -10006,8 +10006,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 b410efe..49d6895 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -10549,7 +10549,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 c5fdb09..dc15dc8 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;
+ }
+ }
}
More information about the cfe-commits
mailing list