[cfe-commits] r60858 - in /cfe/trunk: include/clang/AST/Decl.h include/clang/Basic/DiagnosticKinds.def include/clang/Parse/Scope.h lib/Parse/ParseStmt.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaExprCXX.cpp test/SemaCXX/warn-for-var-in-else.cpp
Douglas Gregor
dgregor at apple.com
Wed Dec 10 15:01:30 PST 2008
Author: dgregor
Date: Wed Dec 10 17:01:14 2008
New Revision: 60858
URL: http://llvm.org/viewvc/llvm-project?rev=60858&view=rev
Log:
Added a warning when referencing an if's condition variable in the
"else" clause, e.g.,
if (int X = foo()) {
} else {
if (X) { // warning: X is always zero in this context
}
}
Fixes rdar://6425550 and lets me think about something other than
DeclContext.
Added:
cfe/trunk/test/SemaCXX/warn-for-var-in-else.cpp
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/Basic/DiagnosticKinds.def
cfe/trunk/include/clang/Parse/Scope.h
cfe/trunk/lib/Parse/ParseStmt.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaExprCXX.cpp
Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=60858&r1=60857&r2=60858&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Wed Dec 10 17:01:14 2008
@@ -331,7 +331,11 @@
unsigned SClass : 3;
bool ThreadSpecified : 1;
bool HasCXXDirectInit : 1;
-
+
+ /// DeclaredInCondition - Whether this variable was declared in a
+ /// condition, e.g., if (int x = foo()) { ... }.
+ bool DeclaredInCondition : 1;
+
// Move to DeclGroup when it is implemented.
SourceLocation TypeSpecStartLoc;
friend class StmtIteratorBase;
@@ -341,7 +345,9 @@
SourceLocation TSSL = SourceLocation())
: ValueDecl(DK, DC, L, Id, T, PrevDecl), Init(0),
ThreadSpecified(false), HasCXXDirectInit(false),
- TypeSpecStartLoc(TSSL) { SClass = SC; }
+ DeclaredInCondition(false), TypeSpecStartLoc(TSSL) {
+ SClass = SC;
+ }
public:
static VarDecl *Create(ASTContext &C, DeclContext *DC,
SourceLocation L, IdentifierInfo *Id,
@@ -373,6 +379,18 @@
return HasCXXDirectInit;
}
+ /// isDeclaredInCondition - Whether this variable was declared as
+ /// part of a condition in an if/switch/while statement, e.g.,
+ /// @code
+ /// if (int x = foo()) { ... }
+ /// @endcode
+ bool isDeclaredInCondition() const {
+ return DeclaredInCondition;
+ }
+ void setDeclaredInCondition(bool InCondition) {
+ DeclaredInCondition = InCondition;
+ }
+
/// hasLocalStorage - Returns true if a variable with function scope
/// is a non-static local variable.
bool hasLocalStorage() const {
Modified: cfe/trunk/include/clang/Basic/DiagnosticKinds.def
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticKinds.def?rev=60858&r1=60857&r2=60858&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticKinds.def (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticKinds.def Wed Dec 10 17:01:14 2008
@@ -1294,6 +1294,10 @@
DIAG(err_not_tag_in_scope, ERROR,
"%0 does not name a tag member in the specified scope")
+DIAG(warn_value_always_zero, WARNING,
+ "%0 is always zero in this context")
+DIAG(warn_value_always_false, WARNING,
+ "%0 is always false in this context")
// assignment related diagnostics (also for argument passing, returning, etc).
// FIXME: %2 is an english string here.
Modified: cfe/trunk/include/clang/Parse/Scope.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Scope.h?rev=60858&r1=60857&r2=60858&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Scope.h (original)
+++ cfe/trunk/include/clang/Parse/Scope.h Wed Dec 10 17:01:14 2008
@@ -75,6 +75,10 @@
/// interrelates with other control flow statements.
unsigned Flags : 8;
+ /// WithinElse - Whether this scope is part of the "else" branch in
+ /// its parent ControlScope.
+ bool WithinElse : 1;
+
/// FnParent - If this scope has a parent scope that is a function body, this
/// pointer is non-null and points to it. This is used for label processing.
Scope *FnParent;
@@ -84,6 +88,11 @@
/// there is no containing break/continue scope.
Scope *BreakParent, *ContinueParent;
+ /// ControlParent - This is a direct link to the immediately
+ /// preceeding ControlParent if this scope is not one, or null if
+ /// there is no containing control scope.
+ Scope *ControlParent;
+
/// BlockParent - This is a direct link to the immediately containing
/// BlockScope if this scope is not one, or null if there is none.
Scope *BlockParent;
@@ -151,6 +160,9 @@
return const_cast<Scope*>(this)->getBreakParent();
}
+ Scope *getControlParent() { return ControlParent; }
+ const Scope *getControlParent() const { return ControlParent; }
+
Scope *getBlockParent() { return BlockParent; }
const Scope *getBlockParent() const { return BlockParent; }
@@ -193,6 +205,12 @@
return getFlags() & Scope::TemplateParamScope;
}
+ /// isWithinElse - Whether we are within the "else" of the
+ /// ControlParent (if any).
+ bool isWithinElse() const { return WithinElse; }
+
+ void setWithinElse(bool WE) { WithinElse = WE; }
+
/// Init - This is used by the parser to implement scope caching.
///
void Init(Scope *Parent, unsigned ScopeFlags) {
@@ -204,17 +222,23 @@
FnParent = AnyParent->FnParent;
BreakParent = AnyParent->BreakParent;
ContinueParent = AnyParent->ContinueParent;
+ ControlParent = AnyParent->ControlParent;
BlockParent = AnyParent->BlockParent;
TemplateParamParent = AnyParent->TemplateParamParent;
+ WithinElse = AnyParent->WithinElse;
+
} else {
FnParent = BreakParent = ContinueParent = BlockParent = 0;
+ ControlParent = 0;
TemplateParamParent = 0;
+ WithinElse = false;
}
// If this scope is a function or contains breaks/continues, remember it.
if (Flags & FnScope) FnParent = this;
if (Flags & BreakScope) BreakParent = this;
if (Flags & ContinueScope) ContinueParent = this;
+ if (Flags & ControlScope) ControlParent = this;
if (Flags & BlockScope) BlockParent = this;
if (Flags & TemplateParamScope) TemplateParamParent = this;
DeclsInScope.clear();
Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=60858&r1=60857&r2=60858&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Wed Dec 10 17:01:14 2008
@@ -509,8 +509,11 @@
ParseScope InnerScope(this, Scope::DeclScope,
C99orCXX && Tok.isNot(tok::l_brace));
+ bool WithinElse = CurScope->isWithinElse();
+ CurScope->setWithinElse(true);
ElseStmtLoc = Tok.getLocation();
ElseStmt = ParseStatement();
+ CurScope->setWithinElse(WithinElse);
// Pop the 'else' scope if needed.
InnerScope.Exit();
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=60858&r1=60857&r2=60858&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Dec 10 17:01:14 2008
@@ -483,6 +483,27 @@
// check if referencing an identifier with __attribute__((deprecated)).
if (VD->getAttr<DeprecatedAttr>())
Diag(Loc, diag::warn_deprecated) << VD->getDeclName();
+
+ if (VarDecl *Var = dyn_cast<VarDecl>(VD)) {
+ if (Var->isDeclaredInCondition() && Var->getType()->isScalarType()) {
+ Scope *CheckS = S;
+ while (CheckS) {
+ if (CheckS->isWithinElse() &&
+ CheckS->getControlParent()->isDeclScope(Var)) {
+ if (Var->getType()->isBooleanType())
+ Diag(Loc, diag::warn_value_always_false) << Var->getDeclName();
+ else
+ Diag(Loc, diag::warn_value_always_zero) << Var->getDeclName();
+ break;
+ }
+
+ // Move up one more control parent to check again.
+ CheckS = CheckS->getControlParent();
+ if (CheckS)
+ CheckS = CheckS->getParent();
+ }
+ }
+ }
// Only create DeclRefExpr's for valid Decl's.
if (VD->isInvalidDecl())
Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=60858&r1=60857&r2=60858&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Dec 10 17:01:14 2008
@@ -648,6 +648,10 @@
return true;
AddInitializerToDecl(Dcl, AssignExprVal);
+ // Mark this variable as one that is declared within a conditional.
+ if (VarDecl *VD = dyn_cast<VarDecl>((Decl *)Dcl))
+ VD->setDeclaredInCondition(true);
+
return new CXXConditionDeclExpr(StartLoc, EqualLoc,
cast<VarDecl>(static_cast<Decl *>(Dcl)));
}
Added: cfe/trunk/test/SemaCXX/warn-for-var-in-else.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-for-var-in-else.cpp?rev=60858&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-for-var-in-else.cpp (added)
+++ cfe/trunk/test/SemaCXX/warn-for-var-in-else.cpp Wed Dec 10 17:01:14 2008
@@ -0,0 +1,30 @@
+// RUN: clang -fsyntax-only -verify %s
+// rdar://6425550
+int bar();
+void do_something(int);
+
+int foo() {
+ if (int X = bar()) {
+ return X;
+ } else {
+ do_something(X); // expected-warning{{'X' is always zero in this context}}
+ }
+}
+
+bool foo2() {
+ if (bool B = bar()) {
+ if (int Y = bar()) {
+ return B;
+ } else {
+ do_something(Y); // expected-warning{{'Y' is always zero in this context}}
+ return B;
+ }
+ } else {
+ if (bool B2 = B) { // expected-warning{{'B' is always false in this context}}
+ do_something(B); // expected-warning{{'B' is always false in this context}}
+ } else if (B2) { // expected-warning{{'B2' is always false in this context}}
+ do_something(B); // expected-warning{{'B' is always false in this context}}
+ }
+ return B; // expected-warning{{'B' is always false in this context}}
+ }
+}
More information about the cfe-commits
mailing list