www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Bug in readln interface ?

reply "monarch_dodra" <monarchdodra gmail.com> writes:
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
Jun 30 2013
next sibling parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
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
Jun 30 2013
prev sibling next sibling parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
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
Jun 30 2013
prev sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
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.
Jun 30 2013