[cfe-commits] r61069 - /cfe/trunk/lib/Analysis/RegionStore.cpp

[Ted Kremenek]

Sounds good.

On Dec 16, 2008, at 5:11 PM, Zhongxing Xu wrote:

>
>
> On Wed, Dec 17, 2008 at 2:40 AM, Ted Kremenek <kremenek at apple.com>  
> wrote:
> Hi Zhongxing,
>
> Thanks for doing this.  A couple comments inline.
>
> On Dec 15, 2008, at 6:36 PM, Zhongxing Xu wrote:
>
>> URL: http://llvm.org/viewvc/llvm-project?rev=61069&view=rev
>> Log:
>> Implement RegionStoreManager::Remove().
>>
>> Modified:
>>    cfe/trunk/lib/Analysis/RegionStore.cpp
>>
>> Modified: cfe/trunk/lib/Analysis/RegionStore.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/RegionStore.cpp?rev=61069&r1=61068&r2=61069&view=diff
>>
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> = 
>> =====================================================================
>> --- cfe/trunk/lib/Analysis/RegionStore.cpp (original)
>> +++ cfe/trunk/lib/Analysis/RegionStore.cpp Mon Dec 15 20:36:30 2008
>> @@ -111,10 +111,7 @@
>>
>> +Store RegionStoreManager::Remove(Store store, Loc L) {
>> +  RegionBindingsTy B = GetRegionBindings(store);
>> +
>> +  const MemRegion* R = cast<loc::MemRegionVal>(L).getRegion();
>> +  assert(R);
>
> This assertion seems bogus.  If Remove should always expect a  
> MemRegion* then we should change its interface to accept that as an  
> argument instead of a Loc value.  Otherwise, Remove should handle  
> the case when 'L' isn't a region.
>
>> +
>> +  return RBFactory.Remove(B, R).getRoot();
>> +}
>
> Do we plan on inserting Unknown into the map to marked the value as  
> killed?  If so, we should only do that if it was in the map in the  
> first place.
>
> I am doing an overhaul on the bind/remove methods of storemanager to  
> implement the lazy binding idea. These are fixed there.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20081216/03024bb0/attachment.html>