Add vanishing-this-pointer tutorial - #141
Conversation
f818c77 to
8284c09
Compare
8284c09 to
081fa05
Compare
| void* x_ptr = &a.x; | ||
| auto* a_ptr = static_cast<A*>(x_ptr); |
There was a problem hiding this comment.
| void* x_ptr = &a.x; | |
| auto* a_ptr = static_cast<A*>(x_ptr); | |
| auto* a_ptr = reinterpret_cast<A*>(x_ptr) |
I am not entirely sure if the cast-to-void* is legal by the standard. My understanding is that void* can only be used for strict aliasing. Regardless, I think it's more precise to use reinterpret_cast here.
|
|
||
| class A { | ||
| public: | ||
| Callable fn; |
There was a problem hiding this comment.
I think we may as well introduce the [[no_unique_address]] here. The section is already pretty light, so we could take a slight bit of complexity from the following section.
| int offset = reinterpret_cast<A*>(this)->value_; | ||
| return x + offset; |
There was a problem hiding this comment.
| int offset = reinterpret_cast<A*>(this)->value_; | |
| return x + offset; | |
| int y = reinterpret_cast<A*>(this)->value_; | |
| return x + y; |
The variable name offset may be a little confusing, since we're also using the term offset to refer to member variable offsets above.
| // Multiply::operator() must be defined out of line: the reinterpret_cast and | ||
| // static_cast require `MultiplyBase` and `A` to be complete types. |
There was a problem hiding this comment.
| // Multiply::operator() must be defined out of line: the reinterpret_cast and | |
| // static_cast require `MultiplyBase` and `A` to be complete types. |
I think saying this once is fine. The above comment could be updated to say "Add:operator() and Multiply::operator() must be defined out of line..."
| const auto* base = reinterpret_cast<const AddBase*>(this); | ||
| const auto* owner = static_cast<const A*>(base); |
There was a problem hiding this comment.
Depending how const correct we want the tutorial to be, I think we should either remove const here, since the member function itself is marked non-const, or just add const to all the member function definitions.
| const auto* base = reinterpret_cast<const MultiplyBase*>(this); | ||
| const auto* owner = static_cast<const A*>(base); |
There was a problem hiding this comment.
See comment above about const-ness.
| A a(3); | ||
| EXPECT_EQ(a.add(5), 8); | ||
| EXPECT_EQ(a.multiply(5), 15); |
There was a problem hiding this comment.
Perhaps a little conclusive comment: "add() and multiply() now behave just like member functions on A."
2 participants
No description provided.