[cfe-commits] r161821 - /cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

[Ted Kremenek]

Let me clarify my comment.

If the point is to not track this information anymore, it should just be 
removed, not #if'ed out.

If the goal is to do something better, such as approach the same task 
differently, then the code should be removed, not #if'ed out.

If the goal is to fix what is already there, then I'd expect us to fix 
it, not to #if'ed the code out.

Any of these are reasonable directions; I just don't understand why the 
code is getting #if'ed out.

Ted Kremenek wrote:
> What is the motivation for doing this?  Don't we want to track the autorelease symbols in the state (and correctly use them)?  Removing the overhead doesn't add much value if we are just going to add it back later, and now the code has extra #if guards that makes it harder to read.
>
> On Aug 13, 2012, at 5:36 PM, Anna Zaks<ganna at apple.com>  wrote:
>
>> Author: zaks
>> Date: Mon Aug 13 19:36:17 2012
>> New Revision: 161821
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=161821&view=rev
>> Log:
>> [analyzer] Disable autorelease pool tracking.
>>
>> The autorelease pool has not been implemented completely: we were adding
>> the autoreleased symbols to the state, but never looking at them. Until
>> we have a complete implementation, remove the overhead and comment out
>> the unused code.
>>
>> Modified:
>>     cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
>>
>> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=161821&r1=161820&r2=161821&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Mon Aug 13 19:36:17 2012
>> @@ -1638,6 +1638,10 @@
>> //===----------------------------------------------------------------------===//
>> // AutoreleaseBindings - State used to track objects in autorelease pools.
>> //===----------------------------------------------------------------------===//
>> +#define AUTORELEASE_POOL_MODELING (0)
>> +// We do not currently have complete modeling of autorelease pools.
>> +
>> +#if AUTORELEASE_POOL_MODELING
>>
>> typedef llvm::ImmutableMap<SymbolRef, unsigned>  ARCounts;
>> typedef llvm::ImmutableMap<SymbolRef, ARCounts>  ARPoolContents;
>> @@ -1685,6 +1689,7 @@
>>
>>    return state->set<AutoreleasePoolContents>(pool, newCnts);
>> }
>> +#endif
>>
>> //===----------------------------------------------------------------------===//
>> // Error reporting.
>> @@ -2427,7 +2432,9 @@
>>    mutable OwningPtr<RetainSummaryManager>  Summaries;
>>    mutable OwningPtr<RetainSummaryManager>  SummariesGC;
>>
>> +#if AUTORELEASE_POOL_MODELING
>>    mutable ARCounts::Factory ARCountFactory;
>> +#endif
>>
>>    mutable SummaryLogTy SummaryLog;
>>    mutable bool ShouldResetSummaryLog;
>> @@ -3004,7 +3011,10 @@
>>
>>      case NewAutoreleasePool:
>>        assert(!C.isObjCGCEnabled());
>> -      return state->add<AutoreleaseStack>(sym);
>> +#if AUTORELEASE_POOL_MODELING
>> +      state = state->add<AutoreleaseStack>(sym);
>> +#endif
>> +      return state;
>>
>>      case MayEscape:
>>        if (V.getKind() == RefVal::Owned) {
>> @@ -3022,7 +3032,11 @@
>>          return state;
>>
>>        // Update the autorelease counts.
>> +      // TODO: AutoreleasePoolContents are not currently used. We will need to
>> +      // call SendAutorelease after it's wired up.
>> +#if AUTORELEASE_POOL_MODELING
>>        state = SendAutorelease(state, ARCountFactory, sym);
>> +#endif
>>        V = V.autorelease();
>>        break;
>>
>> @@ -3718,20 +3732,23 @@
>>      Out<<  "<pool>";
>>    Out<<  ":{";
>>
>> +#if AUTORELEASE_POOL_MODELING
>>    // Get the contents of the pool.
>>    if (const ARCounts *Cnts = State->get<AutoreleasePoolContents>(Sym))
>>      for (ARCounts::iterator I = Cnts->begin(), E = Cnts->end(); I != E; ++I)
>>        Out<<  '('<<  I.getKey()<<  ','<<  I.getData()<<  ')';
>> -
>> +#endif
>>    Out<<  '}';
>> }
>>
>> +#if AUTORELEASE_POOL_MODELING
>> static bool UsesAutorelease(ProgramStateRef state) {
>>    // A state uses autorelease if it allocated an autorelease pool or if it has
>>    // objects in the caller's autorelease pool.
>>    return !state->get<AutoreleaseStack>().isEmpty() ||
>>            state->get<AutoreleasePoolContents>(SymbolRef());
>> }
>> +#endif
>>
>> void RetainCountChecker::printState(raw_ostream&Out, ProgramStateRef State,
>>                                      const char *NL, const char *Sep) const {
>> @@ -3747,6 +3764,7 @@
>>      Out<<  NL;
>>    }
>>
>> +#if AUTORELEASE_POOL_MODELING
>>    // Print the autorelease stack.
>>    if (UsesAutorelease(State)) {
>>      Out<<  Sep<<  NL<<  "AR pool stack:";
>> @@ -3758,6 +3776,7 @@
>>
>>      Out<<  NL;
>>    }
>> +#endif
>> }
>>
>> //===----------------------------------------------------------------------===//
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>