[PATCH] D79704: [Analyzer] [NFC] Parameter Regions

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D79704#2029305 <https://reviews.llvm.org/D79704#2029305>, @baloghadamsoftware wrote:

> Thank you for quickly looking into this.
>
> In D79704#2029230 <https://reviews.llvm.org/D79704#2029230>, @Szelethus wrote:
>
> > - What identifies a `MemRegion`, `TypedValueRegion`s in particular? Why are parameters so special that they deserve their own region? Do you have a concrete, problematic example?
>
>
> `ParamRegion` contains different data: mainly the index of the parameter. This makes it independent of the actual `Decl` of the parameter.
>
> > - Why is it an issue that we don't always use the same declaration? Does this result in a crash, incorrect modeling?
>
> See D49443#1193290 <https://reviews.llvm.org/D49443#1193290>.
>
> > - Why are we making `ParamRegion` **not** a subclass of `VarRegion`?
>
> Because `VarRegion` is a `DeclRegion` which stores the `Decl` of the variable. Here the main purpose is to not to store it.


To me that sounds like a technicality. Sure, inheriting from `VarRegion` would make you carry around a field or two you may not need/use, but is that a big deal? To me it feels like all the problems you're solving should be an implementation detail. The interface of `ParamRegion` looks like a superset of `VarRegion`'s. The code changes suggests that every time we're interested in a `VarRegion`, we're also interested in parameters.

I may not see the obvious just yet, but I invite you to correct me! :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79704/new/

https://reviews.llvm.org/D79704