Chris Lattner wrote: > > On Aug 4, 2010, at 11:27 PM, Nick Lewycky wrote: > >> Author: nicholas >> Date: Thu Aug 5 01:27:49 2010 >> New Revision: 110314 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=110314&view=rev >> Log: >> Remove the warning for variables declared in the if-expression being used in >> the else clause. The problem is that it's overly zealous and will respond to >> uses in assignments, or after assignments. We should bring this back once we >> can do it right. Fixes PR7100. > > This is an unfortunate thing to remove, but if you're going to do it, please > remove isDeclaredInCondition and the other code that supports this warning > but is now dead.
Great point, done. Also, I filed PR7823 for the useful parts of this warning, with cautions about how to implement it to avoid the same problems. Nick > -Chris > >> >> Removed: >> cfe/trunk/test/SemaCXX/warn-for-var-in-else.cpp >> Modified: >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> cfe/trunk/lib/Sema/SemaExpr.cpp >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=110314&r1=110313&r2=110314&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Aug 5 01:27:49 >> 2010 >> @@ -2508,9 +2508,6 @@ >> def note_condition_assign_silence : Note< >> "place parentheses around the assignment to silence this warning">; >> >> -def warn_value_always_zero : Warning< >> - "%0 is always %select{zero|false|NULL}1 in this context">; >> - >> def warn_ivar_variable_conflict : Warning< >> "%0 lookup will access the property ivar in nonfragile-abi2 mode">, >> InGroup<NonfragileAbi2>; >> >> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=110314&r1=110313&r2=110314&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Aug 5 01:27:49 2010 >> @@ -1191,26 +1191,6 @@ >> Diag(Property->getLocation(), diag::note_property_declare); >> } >> } >> - >> - // Warn about constructs like: >> - // if (void *X = foo()) { ... } else { X }. >> - // In the else block, the pointer is always false. >> - if (Var->isDeclaredInCondition()&& Var->getType()->isScalarType()) { >> - Scope *CheckS = S; >> - while (CheckS&& CheckS->getControlParent()) { >> - if ((CheckS->getFlags()& Scope::ElseScope)&& >> - CheckS->getControlParent()->isDeclScope(DeclPtrTy::make(Var))) { >> - ExprError(Diag(NameLoc, diag::warn_value_always_zero) >> -<< Var->getDeclName() >> -<< (Var->getType()->isPointerType() ? 2 : >> - Var->getType()->isBooleanType() ? 1 : 0)); >> - break; >> - } >> - >> - // Move to the parent of this scope. >> - CheckS = CheckS->getParent(); >> - } >> - } >> } else if (FunctionDecl *Func = R.getAsSingle<FunctionDecl>()) { >> if (!getLangOptions().CPlusPlus&& !Func->hasPrototype()) { >> // C99 DR 316 says that, if a function type comes from a >> @@ -4973,7 +4953,7 @@ >> // This check seems unnatural, however it is necessary to ensure the >> proper >> // conversion of functions/arrays. If the conversion were done for all >> // DeclExpr's (created by ActOnIdExpression), it would mess up the unary >> - // expressions that surpress this implicit conversion (&, sizeof). >> + // expressions that suppress this implicit conversion (&, sizeof). >> // >> // Suppress this for references: C++ 8.5.3p5. >> if (!lhsType->isReferenceType()) >> >> Removed: 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=110313&view=auto >> ============================================================================== >> --- cfe/trunk/test/SemaCXX/warn-for-var-in-else.cpp (original) >> +++ cfe/trunk/test/SemaCXX/warn-for-var-in-else.cpp (removed) >> @@ -1,45 +0,0 @@ >> -// RUN: %clang_cc1 -fsyntax-only -verify %s >> -// rdar://6425550 >> -int bar(); >> -void do_something(int); >> -int *get_ptr(); >> - >> -int foo() { >> - if (int X = bar()) { >> - return X; >> - } else { >> - do_something(X); // expected-warning{{'X' is always zero in this >> context}} >> - return 0; >> - } >> -} >> - >> -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}} >> - do_something(B2); // expected-warning{{'B2' is always false in this >> context}} >> - } >> - return B; // expected-warning{{'B' is always false in this context}} >> - } >> -} >> - >> -void foo3() { >> - if (int *P1 = get_ptr()) >> - do_something(*P1); >> - else if (int *P2 = get_ptr()) { >> - do_something(*P1); // expected-warning{{'P1' is always NULL in this >> context}} >> - do_something(*P2); >> - } else { >> - do_something(*P1); // expected-warning{{'P1' is always NULL in this >> context}} >> - do_something(*P2); // expected-warning{{'P2' is always NULL in this >> context}} >> - } >> -} >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
