[cfe-commits] r170931 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp lib/Sema/SemaExpr.cpp test/SemaObjC/arc.m

[Ted Kremenek]

On Dec 21, 2012, at 2:10 PM, Jordan Rose <jordan_rose at apple.com> wrote:

> On Dec 21, 2012, at 13:59 , Ted Kremenek <kremenek at apple.com> wrote:
> 
>> Author: kremenek
>> Date: Fri Dec 21 15:59:39 2012
>> New Revision: 170931
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=170931&view=rev
>> Log:
>> Change checkUnsafeAssignLiteral() to use the new Sema::CheckLiteralKind().
>> 
>> Along the way, fix a bug in CheckLiteralKind(), previously in diagnoseObjCLiteralComparison, where we didn't ignore parentheses
>> in boxed expressions for purpose of classification.
>> 
>> In other words, both @42 and @(42) should be classified as numeric
>> literals.
> 
> That was sort of deliberate the first time around, but okay. I guess to the ObjC programmer they're the same thing.

Yes, my thoughts exactly.

> 
> 
> 
>> Modified:
>>   cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>   cfe/trunk/lib/Sema/SemaChecking.cpp
>>   cfe/trunk/lib/Sema/SemaExpr.cpp
>>   cfe/trunk/test/SemaObjC/arc.m
>> 
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=170931&r1=170930&r2=170931&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Dec 21 15:59:39 2012
>> @@ -3690,7 +3690,7 @@
>>  "; object will be released after assignment">,
>>  InGroup<ARCUnsafeRetainedAssign>;
>> def warn_arc_literal_assign : Warning<
>> -  "assigning %select{dictionary literal|array literal|block literal|boxed expression}0"
>> +  "assigning %select{block literal|array literal|dictionary literal|numeric literal|boxed expression}0"
>>  " to a weak %select{property|variable}1"
>>  "; object will be released after assignment">,
>>  InGroup<ARCUnsafeRetainedAssign>;
>> 
>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=170931&r1=170930&r2=170931&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Dec 21 15:59:39 2012
>> @@ -5753,33 +5753,24 @@
>>  // immediately zapped in a weak reference.  Note that we explicitly
>>  // allow ObjCStringLiterals, since those are designed to never really die.
>>  RHS = RHS->IgnoreParenImpCasts();
>> -  // This enum needs to match with the 'select' in warn_arc_literal_assign.
>> -  enum Kind { Dictionary = 0, Array, Block, BoxedE, None };
>> -  unsigned kind = None;
>> -  switch (RHS->getStmtClass()) {
>> -    default:
>> -      break;
>> -    case Stmt::ObjCDictionaryLiteralClass:
>> -      kind = Dictionary;
>> -      break;
>> -    case Stmt::ObjCArrayLiteralClass:
>> -      kind = Array;
>> -      break;
>> -    case Stmt::BlockExprClass:
>> -      kind = Block;
>> -      break;
>> -    case Stmt::ObjCBoxedExprClass:
>> -      kind = BoxedE;
>> -      break;
>> -  }
>> -  if (kind != None) {
>> -    S.Diag(Loc, diag::warn_arc_literal_assign)
>> -    << (unsigned) kind
>> +
>> +  // Classification for diagnostic.
>> +  unsigned SelectVal = /* block literal */ 0;
>> +  if (!isa<BlockExpr>(RHS)) {
>> +    // This enum needs to match with the 'select' in
>> +    // warn_objc_arc_literal_assign (off-by-1).
>> +    Sema::ObjCLiteralKind Kind = S.CheckLiteralKind(RHS);
>> +    if (Kind == Sema::LK_String || Kind == Sema::LK_None)
>> +      return false;
>> +    SelectVal = (unsigned) Kind + 1;
>> +  }
> 
> This seems awkward again. Can we just add "Block" to the enum, even though it's not strictly an Objective-C thing? If we want to be pedantic we could call it something like "DynamicLiteralKind" (but, strings) or "OwnershipLiteralKind" (i.e. literals that produce objects managed by ARC).

It's going to be awkward in some way.  The two diagnostics that use this classification care about different things, are a feeding the enum value to the %select{…} in the diagnostic.  In one case we care about the block literal, and in the other case we don't, etc.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121221/06dd9394/attachment.html>