Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non default constructible object is created and returned, somehow #42

Open
fabiorossetto opened this issue Apr 17, 2024 · 6 comments
Open

Comments

@fabiorossetto
Copy link

Expected Behavior

See the attached minimal reproducible example. SimpleCall returns an object of type NoDefaultCtor. Somehow, the code compiles. This doesn't make sense because how can NoDefaultCtor be constructed. value, which should be 7, is 0 instead.

Actual Behavior

Steps to Reproduce the Problem

#include <cstdio>
#include <https://raw.githubusercontent.com/boost-experimental/te/master/include/boost/te.hpp>
#include <cassert>

namespace te = boost::te;

struct NoDefaultCtor {
    NoDefaultCtor() = delete;

    int value{7};
};

struct SimpleCall {
  constexpr inline auto run() {
    return te::call<NoDefaultCtor>([](auto &self) {}, *this);
  }
};

struct Test {};

int main() {
  // Option 1 or 2 is required on some compilers
  te::poly<SimpleCall> f{Test{}};
  NoDefaultCtor s = f.run();
  return s.value;
}

Specifications

Tried on clang 16.0.0 and GCC 10

@fabiorossetto
Copy link
Author

fabiorossetto commented Apr 27, 2024

I think I found the problem. In call_impl, the function stored in the vtable is reinterpret casted like this: reinterpret_cast<R (*)(void *, Ts...)>(self.vptr[N - 1]).
So I can actually return whatever:

struct SimpleCall {
  constexpr inline int run() {
    return te::call<int>([](auto &self) { return std::string{"not an int"}; }, *this);
  }
};

That seems dangerous to me...

I also don't understand why call takes the return type as argument. Can't the return type be inferred by TExpr using std::invoke_result?

@kris-jusiak
Copy link
Contributor

Thanks for your investigation @fabiorossetto and for sharing your findings. I'm pretty sure there should be static assert there to ensure that the cast is valid, likely should be a bit_cast in c++20 too. I will add it as indeed its dangerous as it is. Regarding the result type in call, that's required as the lambda takes template arguments which are known later on, otherwise the deduction could fail if poked with wrong types similary to std::invoke.

@fabiorossetto
Copy link
Author

@krzysztof-jusiak I'd be happy to contribute to the project. I can understand almost all the code, but there are certain parts I cannot follow. In order for me to push a fix I'd need some support with understanding certain parts of the code.

Let me know if you'd be interested in receiving external support with the project!

@kris-jusiak
Copy link
Contributor

Hi @fabiorossetto . Would absolutely love it and will do my best to help. Thank you!

@fabiorossetto
Copy link
Author

@krzysztof-jusiak Thank you Krys. Let me know if there is any way I can write you in private with my questions

@kris-jusiak
Copy link
Contributor

Sure, feel free to drop me a line at [email protected]. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants