Szelethus added a comment.

In D79704#2034571 <https://reviews.llvm.org/D79704#2034571>, @NoQ wrote:

> In D79704#2032947 <https://reviews.llvm.org/D79704#2032947>, @Szelethus wrote:
>
> > In D79704#2032271 <https://reviews.llvm.org/D79704#2032271>, @NoQ wrote:
> >
> > > Blanket reply! `ParamRegion` is not a `DeclRegion` because it does not 
> > > necessarily have a corresponding `Decl`. For instance, when calling a 
> > > function through an unknown function pointer, you don't have the 
> > > declaration of a function, let alone its parameters.
> >
> >
> > Well hold on for a second then -- how is this, if it is, different for 
> > member pointers? Aren't they represented with a `VarRegion`?
>
>
> They aren't even `Loc`!
>
> We currently don't have a representation for an unknown pointer-to-member. A 
> known pointer-to-member is represented by a `FieldDecl` (not sure about 
> static members) but it's still a `NonLoc` and as such isn't a region at all. 
> Because, well, you can't dereference a pointer-to-member; you can only apply 
> it to a base pointer like an offset. The mental model is "an offset". 
> Therefore it's `NonLoc`.
>
> Pointers to member functions are a different thing; they're function pointers 
> so they're kinda regions.
>
> >> But for parameters of C++ object type you still need your `ParamRegion` 
> >> because arguments are constructed into it.
> > 
> > Could you give a specific code example?
>
>
>
>   struct S {
>     S() {
>       this; // What region does 'this' point to...
>     }
>   };
>  
>   void foo(void (*bar)(S)) {
>     bar(S()); // ...in this invocation?
>   }
>


I remember this coming up so many times, and I'm an inch away from getting it 
but I'm not there just yet ^-^ Sorry if you have to repeat yourself too many 
times.

The example you have shown sounds like a case we could (should?) abstract away. 
If we model in accordance to the AST as closely as possible, `ParamRegion` 
should totally be a `VarRegion`. If something tricky like this came up, the 
`getDecl` function should just return with a nullptr, shouldn't it? The code 
changes make me feel like we're doing a lot of chore (and make it super easy to 
forget checking for parameters explicitly). The gain, which is I guess 
correctness, seems negligible, and I'm not sure this is actually correct, as I 
mentioned in D79704#inline-730731 
<https://reviews.llvm.org/D79704#inline-730731>. I don't see why the most 
defining feature of a `DeclRegion` is supposed to be an always existing `Decl`, 
rather than a mirror of the AST.


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

https://reviews.llvm.org/D79704



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to