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<[email protected]>  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
>> [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

Reply via email to