The signature for functions like std.file.copy() takes "in char[]" so it can
be called with both mutable and immutable strings. If an exception gets
thrown, the conservative assumption is that the source may get changed
so the string is duplicated.
The problem, again, is bloat.
The solution is to add two overloads for FileException, one taking string and one taking const(char)[].
Comment #1 by hsteoh — 2014-09-27T21:53:21Z
A better solution is to use to!string, which is a no-op if the source and destination types are identical.
Comment #2 by dlang-bugzilla — 2014-09-28T02:24:38Z
(In reply to Walter Bright from comment #0)
> The signature for functions like std.file.copy() takes "in char[]" so it can
> be called with both mutable and immutable strings. If an exception gets
> thrown, the conservative assumption is that the source may get changed
> so the string is duplicated.
>
> The problem, again, is bloat.
>
> The solution is to add two overloads for FileException, one taking string
> and one taking const(char)[].
Doesn't that mean that we'd also need to add overloads to std.file.copy (and many other std.file functions)?
(In reply to hsteoh from comment #1)
> A better solution is to use to!string, which is a no-op if the source and
> destination types are identical.
That won't work for "in char[]", will it? It would only work by making the constructor templated.
Comment #3 by code — 2014-09-29T02:48:51Z
Well is that really a problem for hopefully rare case that an exception is thrown?
If you add an overload you duplicate the function and the source code.
When you templatize it instead you'll add template code bloat to every user of that function.
Comment #4 by bugzilla — 2014-09-29T09:31:13Z
(In reply to hsteoh from comment #1)
> A better solution is to use to!string, which is a no-op if the source and
> destination types are identical.
It has the same problem - the copying is done at the call site, when it should be done by the callee (in order to reduce code bloat).
Comment #5 by bugzilla — 2014-09-29T09:32:33Z
(In reply to Vladimir Panteleev from comment #2)
> Doesn't that mean that we'd also need to add overloads to std.file.copy (and
> many other std.file functions)?
No, the idea is to get the copy operation into the callee, not the caller.
Comment #6 by dlang-bugzilla — 2014-09-29T09:53:06Z
(In reply to Walter Bright from comment #5)
> (In reply to Vladimir Panteleev from comment #2)
> > Doesn't that mean that we'd also need to add overloads to std.file.copy (and
> > many other std.file functions)?
>
> No, the idea is to get the copy operation into the callee, not the caller.
So I understand that you want to force all callees to pass in an immutable string, even though an immutable string is needed only for the rare case of when an exception occurs. I don't see how that would be an improvement.
Comment #7 by bugzilla — 2014-10-04T09:58:26Z
(In reply to Vladimir Panteleev from comment #6)
> (In reply to Walter Bright from comment #5)
> > (In reply to Vladimir Panteleev from comment #2)
> > > Doesn't that mean that we'd also need to add overloads to std.file.copy (and
> > > many other std.file functions)?
> >
> > No, the idea is to get the copy operation into the callee, not the caller.
>
> So I understand that you want to force all callees to pass in an immutable
> string, even though an immutable string is needed only for the rare case of
> when an exception occurs. I don't see how that would be an improvement.
No, I suggested adding two overloads for FileException, one taking string and one taking const(char)[]. Only the latter would ever call .idup, and the .idup would only happen when an exception was thrown, and would only have code generated once for it in the entire code base.
Comment #8 by dlang-bugzilla — 2014-10-04T18:37:24Z
Oh, sorry, I finally get it now. For some reason I kept thinking you were trying to get rid of the .idup when the user passes a string to copy. Thanks for elaborating.
Comment #9 by dlang-bugzilla — 2014-10-05T17:31:47Z
BTW, std.file.copy is not a very good example, because really it should be fixed as to what exception message it's using. Right now it's doing:
if (!result)
throw new FileException(to.idup);
However, if the source file or path does not exist, this generates an entirely misleading message. The exception message should contain both the "from" and "to" parameters, because we can't know which path the problem occurred with.
Yes, in this case, this means more bloat. But it needs to be fixed.
Comment #10 by robert.schadek — 2024-12-01T16:22:33Z