Support for string_view slots #17
Conversation
| @@ -255,6 +260,11 @@ namespace wrenbind17 { | |||
| return std::string(wrenGetSlotString(vm, idx)); | |||
| } | |||
|
|
|||
| template <> inline std::string_view getSlot(WrenVM* vm, int idx) { | |||
| validate<WrenType::WREN_TYPE_STRING>(vm, idx); | |||
| return std::string_view(wrenGetSlotString(vm, idx)); | |||
There was a problem hiding this comment.
This may be very dangerous. The string_view would not own the memory.
Therefore, if you have a C++ function that accepts string_view, and stores the string_view for a future use, then that may cause a segmentation fault. As far as I know Wren only guarantees that the raw bytes of that slot (string) is valid only during the call of the C++ function. The garbage collection may run after calling the function and the string_view points to an unknown memory.
This is fine the other way around. Wren makes a copy of the string/string_view you give it.
I am fine with accepting the risk, but could you update the documentation here? -> https://github.com/matusnovak/wrenbind17/blob/master/docs/content/Tutorial/types.md Maybe adding some section explaining this danger. I don't mind the exact paragraphs, I can leave that to you. Plus a brief comment on this line that yes it is a risk.
There was a problem hiding this comment.
Sorry for the late response, but i got distracted with other projects and completly forgot about this PR, until i checked out my old project.
I can update the paragraph, but i think std::string_view does never own memory does it?
So the risk should probably be known to the user.
This adresses #16
These changes do work for me, however i have no idea if it includes all places required (there's too much template magic going on for me :P )