Thread overview
Bug in readln interface ?
Jun 30, 2013
monarch_dodra
Jul 01, 2013
monarch_dodra
June 30, 2013
The global readln has a signature that looks like this:
size_t readln(ref char[] buf, dchar terminator = '\n')

File.readln's is:
size_t readln(C)(ref C[] buf, dchar terminator = '\n')
if (isSomeChar!C && !is(C == enum))

You might think "Oh: Global readline isn't templated, it should". Yes it should, but that's minor and trivial to fix.

The problem is that "C[]" can mean things like "const(char)[]", which means, basically, "string". This means that the following code is legal:

string reuseableBuffer; (!)
myFile.readln(reuseableBuffer); //Okey Dokey.

Note: This works perfectly fine, there is no illegal mutation or anything. It's just that a brand new value is always assigned to the (not so reuseable) reuseableBuffer slice.

The code handles this, but:
a) It Accepts code this makes little sense, and is most probably an error that silently passes.
b) While the *code* accepts this, *reading it*, it feels more like luck then explicitly handled.
c) This can be replaced just as well by:
 c.1) "s = myFile.readln();"
 c.2) "(s = myFile.readln()).size;" if you want the return value

----------------

So my question is: Do we *want* to keep this? Should we deprecate it? I think we should deprecated it.

Thoughts?

PS: I got dibs on the fix :p
June 30, 2013
On Sun, 30 Jun 2013 15:12:40 -0400, monarch_dodra <monarchdodra@gmail.com> wrote:

>
> So my question is: Do we *want* to keep this? Should we deprecate it? I think we should deprecated it.
>
> Thoughts?

Fully agree.  The fact that the condition explicitly disallows enums should clue us in that it was not intended to simply play nice with types, it really wants a mutable buffer.

If you want an immutable result, use the version that gives you the result as a return value.

-Steve
June 30, 2013
On Sun, 30 Jun 2013 19:08:35 -0400, Steven Schveighoffer <schveiguy@yahoo.com> wrote:

> On Sun, 30 Jun 2013 15:12:40 -0400, monarch_dodra <monarchdodra@gmail.com> wrote:
>
>>
>> So my question is: Do we *want* to keep this? Should we deprecate it? I think we should deprecated it.
>>
>> Thoughts?
>
> Fully agree.  The fact that the condition explicitly disallows enums should clue us in that it was not intended to simply play nice with types, it really wants a mutable buffer.
>
> If you want an immutable result, use the version that gives you the result as a return value.

BTW, this should simply be changed, not deprectated IMO.  It is an "accepts invalid" bug.  The function description specifically says it reuses the buffer and extends as necessary.  Since it can't possibly reuse, that means this is truly a bug.

-Steve
July 01, 2013
On Sunday, 30 June 2013 at 23:10:07 UTC, Steven Schveighoffer wrote:
> On Sun, 30 Jun 2013 19:08:35 -0400, Steven Schveighoffer <schveiguy@yahoo.com> wrote:
>
>> On Sun, 30 Jun 2013 15:12:40 -0400, monarch_dodra <monarchdodra@gmail.com> wrote:
>>
>>>
>>> So my question is: Do we *want* to keep this? Should we deprecate it? I think we should deprecated it.
>>>
>>> Thoughts?
>>
>> Fully agree.  The fact that the condition explicitly disallows enums should clue us in that it was not intended to simply play nice with types, it really wants a mutable buffer.
>>
>> If you want an immutable result, use the version that gives you the result as a return value.
>
> BTW, this should simply be changed, not deprectated IMO.  It is an "accepts invalid" bug.  The function description specifically says it reuses the buffer and extends as necessary.  Since it can't possibly reuse, that means this is truly a bug.
>
> -Steve

Alright, thanks. Officially Filed:
http://d.puremagic.com/issues/show_bug.cgi?id=10517

And under correction.