[PATCH] D141472: [clang][Interp] Add function pointers

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

In D141472#4107008 <https://reviews.llvm.org/D141472#4107008>, @aaron.ballman wrote:

> In D141472#4106091 <https://reviews.llvm.org/D141472#4106091>, @tbaeder wrote:
>
>>> Member pointers (for functions or for data) are weird in that they're not the typical pointer width. They're actually a pointer and between one-to-three other fields in a trenchcoat, depending on the circumstances. You generally need the function pointer, but you also may need various offsets (to this, to the vtable, etc). There's some more information about how it's done in MSVC (which is different from Itanium ABI, but we can do what we want for the constant expression interpreter): https://rants.vastheman.com/2021/09/21/msvc/
>>>
>>> I don't think there's a problem with `FunctionPointer` per se, I'm more wondering are you planning to also add a `MemberPointer` type or are you planning to reuse `FunctionPointer` to handle function members (and presumably something else for data members)?
>>>
>>> As for virtual functions in general, the standard has rules: http://eel.is/c++draft/expr.const#5.6 and http://eel.is/c++draft/expr.const#7
>>
>> I was thinking that the `dynamicDispatch` in https://godbolt.org/z/rf9Ks77Wo would be a good reproducer since the actual function to call is only known when calling `dynamicDispatch()`, but that example already works when doing a few changes to `classify()` the right types and adding a `if (BO->isPtrMemOP()) { return visit(RHS); }` to `VisitBinaryOperator()`.
>
> That is a reasonable test, but probably not sufficient as nothing is really testing the layout of those objects (the calls return a constant). How about a test like: https://godbolt.org/z/rhhhvxYxf where there are offsets to member variables involved?

Ah I get what you mean now.

The pointer we have on the stack is of type `S` but then we end up calling a function expecting a `S2` pointer, and which one we're calling is not known at compile time.

I'm not sure what to do about this right now. I was wondering about restructuring pointers so all the metadata is before all the actual data, but that would be a large refactoring.


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

https://reviews.llvm.org/D141472