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
parent reply "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
parent reply "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
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