[cfe-commits] r169571 - /cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp

[Jordan Rose]

On Dec 7, 2012, at 10:26 , Ted Kremenek <kremenek at apple.com> wrote:

> On Dec 7, 2012, at 9:36 AM, Jordan Rose <jordan_rose at apple.com> wrote:
> 
>>> //===----------------------------------------------------------------------===//
>>> // Fine-grained control of RegionStoreManager.
>>> @@ -164,19 +289,15 @@
>>> namespace {
>>> 
>>> class RegionStoreManager : public StoreManager {
>>> +public:
>>>  const RegionStoreFeatures Features;
>>>  RegionBindings::Factory RBFactory;
>>> -  ClusterBindings::Factory CBFactory;
>>> +  mutable ClusterBindings::Factory CBFactory;
>> 
>> Is there a reason this is mutable? When is the RSM ever const? (Left over from an intermediate attempt?)
>> 
> 
> Yes.
> 
> Remove it, and you get:
> 
> /Volumes/Data/Users/kremenek/llvm/tools/clang/lib/StaticAnalyzer/Core/RegionStore.cpp:478:12: error: no matching constructor for initialization of '<anonymous>::RegionBindingsRef'
>     return RegionBindingsRef(CBFactory,
>            ^                 ~~~~~~~~~~
> /Volumes/Data/Users/kremenek/llvm/tools/clang/lib/StaticAnalyzer/Core/RegionStore.cpp:148:3: note: candidate constructor not viable: 1st argument ('const ClusterBindings::Factory') would lose const qualifier
>   RegionBindingsRef(ClusterBindings::Factory &CBFactory,
>   ^
> /Volumes/Data/Users/kremenek/llvm/tools/clang/lib/StaticAnalyzer/Core/RegionStore.cpp:154:3: note: candidate constructor not viable: requires 2 arguments, but 3 were provided
>   RegionBindingsRef(const ParentTy &P, ClusterBindings::Factory &CBFactory)
>   ^
> /Volumes/Data/Users/kremenek/llvm/tools/clang/lib/StaticAnalyzer/Core/RegionStore.cpp:141:7: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 3 were provided
> class RegionBindingsRef : public llvm::ImmutableMapRef<const MemRegion *,
>       ^
> 1 error generated.
> ninja: build stopped: subcommand failed.
> 
> The error is in getRegionBindings(), which is marked 'const'.  We could also use a const-cast, or remove the 'const' from getRegionBindings() and deal with the fallout.

I don't think getRegionBindings() really is const if it returns something that can modify the set of all regions. I think the Right Thing is to make RegionBindingsRef const-correct, then add a const overload to getRegionBindings to handle includedInBindings (the only externally-visible const function in StoreManager). But we can leave this for now.

Jordan

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