Bug 12448 – "in" argument for std.string.toStringz

The signature of the second overload of toStringz is: immutable(char)* toStringz(string s) @trusted pure nothrow If you replace it with: immutable(char)* toStringz(in string s) @trusted pure nothrow Then the compiler gets able to catch some not used result bugs: import std.exception: assumeUnique; import core.stdc.string: memcmp, strlen; import std.array: empty; immutable(char)* toStringz(const(char)[] s) @trusted pure nothrow { auto copy = new char[s.length + 1]; copy[0 .. s.length] = s[]; copy[$ - 1] = '\0'; return assumeUnique(copy).ptr; } immutable(char)* toStringz(in string s) @trusted pure nothrow { if (s.empty) return "".ptr; immutable p = s.ptr + s.length; if ((cast(size_t) p & 3) && *p == 0) return s.ptr; return toStringz(cast(const char[]) s); } void main() { string foo; foo.toStringz; // Warning } test.d(32,8): Warning: Call to strictly pure function test.toStringz discards return value of type immutable(char)*, prepend a cast(void) if intentional

To avoid future possible problems with "scope", using "const string s" is also enough.

no const or in needed the compiler already warns

(In reply to Robert Schadek from comment #2) > no const or in needed the compiler already warns Generally in idiomatic D all arguments have be in/const/immutable, unless they can't :-)

your're properly right. do you want to create the PR or should I?

(In reply to Robert Schadek from comment #4) > your're properly right. > > do you want to create the PR or should I? I think reopening this issue could be enough.

but we should fix that and close it again ;-) It is your fix, but if you don't want to create the PR I will do it for D HackDay 2

(In reply to Robert Schadek from comment #6) > but we should fix that and close it again ;-) > > It is your fix, but if you don't want to create the PR I will do it for D > HackDay 2 It's a similar "problem", I don't even have to change the issue name. If you want to open a new issue you are free to do it, but the purpose of Bugzilla and fixing issues is not to proliferate them :-)

I don't really follow. If this issue is none existing anymore, why should this stay open?

(In reply to Robert Schadek from comment #8) > I don't really follow. > > If this issue is none existing anymore, why should this stay open? The original problem is solved, but I was thinking about keeping this issue open for a different purpose: to ask the signature of toStringz to become toStringz(in string s) anyway. But this is not so important, so I close this issue again.

There is no guarantee that the result of toStringz does not escape the function, so using in for parameter to toStringz would be fundamentally wrong, since in implies scope. And since scope hasn't been fully implemented, the problem wouldn't even be caught. Rather, we risk ending up with such code breaking when scope actually gets fully implemented - which is why I'd argue that using in is almost always bad. We should just avoid using scope and in until they're properly implemented IMHO.

(In reply to Jonathan M Davis from comment #10) > we risk ending up with such code breaking when scope > actually gets fully implemented Implementing scope/in will just disallow some current wrong usages, so it will just break code, it will not introduce actual (silent) bugs, that are the dangerous ones.

(In reply to bearophile_hugs from comment #11) > (In reply to Jonathan M Davis from comment #10) > > we risk ending up with such code breaking when scope > > actually gets fully implemented > > Implementing scope/in will just disallow some current wrong usages, so it > will just break code, it will not introduce actual (silent) bugs, that are > the dangerous ones. Just because code breakage isn't silent doesn't mean that it's good. It isn't even properly defined what scope is supposed to mean, so you can't even know for sure what correct usage is. In theory, it's only supposed to be used with parameters which don't escape a function, but since that's not well-defined, it's arguably impossible to know for sure whether you're using it correctly, and even if we knew for sure exactly what it meant, if you still end up using it in the wrong way, then your code will break when scope is properly implemented, whereas it wouldn't break if you just didn't use it. So, using scope or in before they're properly implemented really doesn't buy you anything, and it risks breaking code once they are implemented. Regardless, putting scope on toStringz is fundamentally wrong, because it's not guaranteed that the pointer being returned doesn't refer to the string being passed in. And even if we don't know exactly what constitutes escaping a function, I think that it's pretty clear that returning a pointer to the contents of the in parameter would mean that the parameter escaped the function.