Compiler error starting in v3.2.2 with non-copyable user type #1070
Comments
|
I would have given a godbolt link, but v3.2.2 does not seem to appear there yet. |
|
This was causing problems in another issue, where This is likely drilling down to the fact that we don't have a "const reference" table: everything in Lua is either a plain, non-const reference or a value. We should probably fix this. |
|
Thanks for the response, and yes fixing would be great (how hard is it?), since it is currently blocking me from updating my sol version. |
|
By the way -- the fact that this broke (or at least changed behavior) would seem to imply that there are no unit tests covering this case... perhaps sol2 needs some better unit test coverage. |
|
Pull requests welcome. |
|
I am happy to look into making a unit test for this -- but I think fixing it would be a bit tough since I don't have any familiarity with the code base. Unless you think it is simple and could give me a sketch of how to do it... |
|
This can't be fixed without breaking a fairly fundamental tenent that: Arguments:
Returns:
The old behavior where
Folks could override these 2 to change the behavior. A greater change of "add a const reference table" is overkill and adds to the "too much runtime data" problem, albeit that's solved with an orthogonal feature to "pin the set of allowed operations at compile time to completely avoid issues of needing to grow or add things at runtime". So, we might just add a The extension points are probably a better fundamental fix for this issue, that goes beyond the already-in- |
|
Here is probably a dumb question: why does sol associate const ref return types with copying? Shouldn't it only copy (or move) if a function returns the type by value? What was the original reasoning for wanted to copy const ref return types? In any case... I guess for now I will just use non-const refs for return types and deal with it. |
#include <string>
const std::string& jekw (const std::string& xD) {
return xD;
}
int main () {
const std::string* ptr = &jekw(":)");
return ptr->size();
} |
|
(In other words, |
|
Right, but do you think we could say something like, generally in C++, a function should not return a reference unless it is sure that it is not referring to a temporary. In other words, the above example would seem to be more the fault of the person who wrote Is this something that we can/should change with sol? I know that it might break backward compatibility, but it seems like the right thing to do if I understand correctly what's going on here. |
|
This is, unfortunately, not how people write code in the wild and not how legacy code behaves. People have been returning
Compilers warn on more direct examples but protracted ones still break, without warning. These bugs would be hard to find and hard to track down because the nature of their brokenness means they go into the Lua system and would demand quite the suite of runtime tools to catch. (Or just a lucky break with AddressSanitizer and UBSanitizer.) |
|
I hear you. I guess a counter could be that, if sol is "silently" copying types that are returned by const&, that could introduce bugs as well, where a user thinks they're operating on the "original" object in Lua when they are in fact just using a copy. It sounds like for the time being I will just have to return non-const refs in my code. But thanks for your prompt responses, I appreciate it. P.S. when are we getting your Unicode stuff in C++? |
|
Still working on this. P.S.: Unicode stuff is coming. We had some wins in the C Committee - https://soasis.org/posts/planted-seeds-unicode-c-c++-2021/ |
|
So I've spent some time on this trying to think about making a it just doesn't work. It's easy to detect for The problem gets more interesting (read: more annoying) when you start doing things like this: sol::state lua;
lua["f"] = [] (int count, std::string& value) {
// ...
};
static const std::string my_string = "hi";
lua["f"](1, std::cref(my_string)); // Uh???What am I supposed to do here? Using
The approach of "checking if the qualifiers match and performing C++-style resolution for each argument" is bad enough; doing it to also make things like So, unfortunately, I can't really implement this one. I think we'll just have to stay with the copies, and in the new docs make sure it's noted pretty clearly what happens with arguments vs. returns vs. function calls. |
|
@ThePhD Thanks for taking a detailed look at this. Given that it looks like things need to stay the way they are, I am wondering if you could just confirm my understanding so that I can better use the sol2 API:
Are those correct? (second one seems unlikely, but just making sure) And one other thing as a last attempt to make something work: I noticed above in your analysis that you are consider function parameters. For my use case I happen to only be concerned with |
Hi,
After upgrading my sol2 version from v3.2.1 to v3.2.2 I started seeing a compile error which I traced it to commit 0e1aafe. It is preventing me from registering a callable whose return type is a
const &to a non-copyable type (although the problem may well be more general than that). Below I provided all of the info that you might need, but let me know if you need anything else.Environment:
-std=c++20Standalone Reproducer
This is impacting me because some of my types are movable but not copyable, and I used to be able to register callables that return
const &of one of those non-copyable types but can't seem to anymore as of v3.2.2. Am I doing something wrong?Compiler Error
The text was updated successfully, but these errors were encountered: