Add as_c_str method on strings#6729
Conversation
|
I really dislike this API. Can we not make it something like this: HELLO WORLD LOL, THIS IS BEN EDITING PATRICK'S COMMENT TO PROVE THAT GITHUB ALLOWS ME TO EDIT ANY ARBITRARY COMMENT ON ANY PULL REQUEST THAT I'VE OPENED! NOTHING IS SAFE! R+ BSTRIE/REFLECTIONS_ON_TRUSTING_TRUST |
|
@Aatch Good catch, I completely glossed over the extra indirection there. @pcwalton I'm not attached to the current API, but how would you expect to use a CStringBundle for C interop? Right now my Lua interop code looks like this: let ret1 = str::as_c_str("function foo (x,y) return x+y end",
|s| luaL_loadstring(L, s));With this pull I was expecting to change it to this: let ret1 = do "function foo (x,y) return x+y end".as_c_str |s| {
luaL_loadstring(L, s);
}This still seems roundabout, I'd be perfectly happy with something like: let ret1 = luaL_loadstring(L, "function foo (x,y) return x+y end".as_c_str());With your proposed function, would |
|
@pcwalton What I don't like about this API most is that the performance characteristics are not obvious. What if we instead changed the signature to |
|
Redesigning this interface probably doesn't need to block this pull request though. |
|
Oh, what I really like the least about this api is that it is possible to read uninitialized memory when checking for null. |
|
@brson Are you talking about the current as_c_str? how can it read uninitialized memory? |
Formerly this was a free function rather than a method. I've left it in place for now, although redefined it so that it just calls the method.
|
@Kimundi perhaps it can't if we maintain the invariant that all slices are overallocated. There was a bug once because some code didn't and I've had a vendetta for it since. |
Formerly this was a free function rather than a method. I've left it in place for now, although redefined it so that it just calls the method.