[cfe-commits] r110314 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/SemaCXX/warn-for-var-in-else.cpp

[Nick Lewycky]

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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>