http://d.puremagic.com/issues/show_bug.cgi?id=927

           Summary: writefln() is duplicating values for parameters supplied
           Product: D
           Version: 1.00
          Platform: Macintosh
        OS/Version: Mac OS X
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: bugzilla@digitalmars.com
        ReportedBy: rmann-d-lang@latencyzero.com


I'm not sure if it's just something I did wrong, but I'm getting the wrong value output from writefln.

template
MakeFourCharCode(char[] inS)
{
        static assert(inS.length == 4);
        const FourCharCode MakeFourCharCode = (inS[0] << 24)
                                                                        |
(inS[1] << 16)
                                                                        |
(inS[2] << 8)
                                                                        |
inS[3];
}


char[]
FourCharCodeAsString(FourCharCode inVal)
{
        char[4] s;

        s[0] = (inVal >> 24 & 0xFF);
        s[1] = (inVal >> 16 & 0xFF);
        s[2] = (inVal >> 8 & 0xFF);
        s[3] = (inVal >> 0 & 0xFF);

        return s;
}

foo()
{
        FourCharCode val1 = MakeFourCharCode!("cntl");
        uint val2 = 4;
        writefln("SketchViewDrawHandler called. Class: '%s', Kind: %s ('%s')",
                        FourCharCodeAsString(val1),
                        val2,
                        FourCharCodeAsString(MakeFourCharCode!("abcd")));
}

Output of foo:

SketchViewDrawHandler called. Class: 'cntl', Kind: 4 ('cntl')

I expected:

SketchViewDrawHandler called. Class: 'cntl', Kind: 4 ('abcd')

(I originally had put val2 as the argument where "abcd" is, with appropriate casting, but then I switched to "abcd" as an experiment).


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=927





------- Comment #1 from rmann-d-lang@latencyzero.com  2007-02-03 12:55 -------
It seems that the first %s is converting the value of the last parameter. This code

                char[] str = FourCharCodeAsString(MakeFourCharCode!("abcd"));
                char[] str1 = FourCharCodeAsString(MakeFourCharCode!("defg"));
                writefln("SketchViewClickHandler called. Class: '%s', Kind: %s
('%s')",
                                str,
                                eventKind,
                                str1);

Produces this output:

SketchViewClickHandler called. Class: 'defg', Kind: 3 ('defg')




This, however, works as expected:

                writefln("SketchViewClickHandler called. Class: '%s', Kind: %s
('%s')",
                                "abcd",
                                eventKind,
                                "efgh");


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=927


bugzilla@digitalmars.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |INVALID




------- Comment #2 from bugzilla@digitalmars.com  2007-02-03 13:17 -------
char[]
FourCharCodeAsString(FourCharCode inVal)
{
        char[4] s;
        ...
        return s;
              ^^^ this is the bug, returning a reference to a local, try:
        return s.dup;
}

BTW, the next release of the compiler will give a compile time error for this, which should help, as it seems to come up frequently.


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=927





------- Comment #3 from rmann-d-lang@latencyzero.com  2007-02-03 13:33 -------
Ah! Sorry. Force of habit, and mistakenly thinking "char[]" was the same as Java's "String".

Thanks!


-- 

On Sat, 3 Feb 2007 18:43:15 +0000 (UTC), d-bugmail@puremagic.com wrote:


> char[]
> FourCharCodeAsString(FourCharCode inVal)
> {
>         char[4] s;
> 
>         s[0] = (inVal >> 24 & 0xFF);
>         s[1] = (inVal >> 16 & 0xFF);
>         s[2] = (inVal >> 8 & 0xFF);
>         s[3] = (inVal >> 0 & 0xFF);
> 
>         return s;
> }

As Walter pointed out, the return is referencing a local stack variable.

However, another way of resolving this could be ...

char[]
FourCharCodeAsString(FourCharCode inVal)
{
        char[] s;

        s ~= (inVal >> 24 & 0xFF);
        s ~= (inVal >> 16 & 0xFF);
        s ~= (inVal >> 8 & 0xFF);
        s ~= (inVal >> 0 & 0xFF);

        return s;
}

In other words, seeing you are trying to return a char[], you might as well create a char[] (and not char[4]) to return. The char[], being a variable-length array is created as a heap item and so the return statement returns its heap reference and not any stack data.

-- 
Derek Parnell

On Sun, 4 Feb 2007 10:32:02 +1100, Derek Parnell wrote:

And if the concatenation is a performance issue for it, then ...

 char[]
 FourCharCodeAsString(FourCharCode inVal)
 {
         char[] s;

         s.length = 4;
         s[0] = (inVal >> 24 & 0xFF);
         s[1] = (inVal >> 16 & 0xFF);
         s[2] = (inVal >> 8 & 0xFF);
         s[3] = (inVal >> 0 & 0xFF);

         return s;
 }

-- 
Derek Parnell

d-bugmail@puremagic.com wrote:
> ------- Comment #2 from bugzilla@digitalmars.com  2007-02-03 13:17 -------
[snip return-local-variable bug]
> 
> BTW, the next release of the compiler will give a compile time error for this,
> which should help, as it seems to come up frequently.

This is good news. Will it do that for delegates[1] as well?


[1] I mean both anonymous delegates and delegates pointing to nested functions.

http://d.puremagic.com/issues/show_bug.cgi?id=927





------- Comment #5 from rmann-d-lang@latencyzero.com  2007-02-03 21:41 -------
I was reading somewhere that the compiler could put the stack frame on the heap in situations like that, so that it would actually work to reference local variables.


-- 

d-bugmail@puremagic.com wrote:
> ------- Comment #5 from rmann-d-lang@latencyzero.com  2007-02-03 21:41 -------
> I was reading somewhere that the compiler could put the stack frame on the heap
> in situations like that, so that it would actually work to reference local
> variables.

Perhaps, but until it does this should be an error. Or at least a warning (I know, Walter doesn't like those).
Returning local arrays & delegates are probably two of the most common bugs seen on d.D.learn.

Frits van Bommel wrote:
> d-bugmail@puremagic.com wrote:
>> ------- Comment #5 from rmann-d-lang@latencyzero.com  2007-02-03 21:41 -------
>> I was reading somewhere that the compiler could put the stack frame on the heap
>> in situations like that, so that it would actually work to reference local
>> variables.
> 
> Perhaps, but until it does this should be an error. Or at least a warning (I know, Walter doesn't like those).
> Returning local arrays & delegates are probably two of the most common bugs seen on d.D.learn.

I'm not against producing an error in the cases where the compiler can detect this cleanly.  I expect there to be some cases which are easy and some which are thorny (but I don't really know).

I think compilers for languages like ruby put all the stack frames on the heap, and this issue is solved completely, but it's one of those features that causes the performance to drop sharply when used.  I seem to remember that Perl allows more of this than D, and Ruby does more than Perl.  Java falls somewhere in this spectrum.  This was the explanation for why you can't build a Ruby (or was it perl?) interpreter that works under the JVM, because the JVM doesn't do stacks this way and can't be made to.  But I don't have the link to this and don't remember the exact details.

I guess the implied next step is to allocate a copy of the relevant portions of the stack to wrap up the delegate in question.  In other words, implement a partial version of the closure mechanism for these cases.  I don't know how well this would work, but these are my thoughts:

1. It would absolutely slaughter performance to do it every time -- the foreach() mechanism uses a delegate, so looping from 1 to 1_000_000,
would incur 1 million allocations.  So it'd have to be selective.

2. In a lot of cases you want the delegate to modify stuff in the stack frame (most foreach loops compute a value that goes in a variable that outlives the foreach), so it would have to be a copy of the stack frame when returning the delegate but use the *real* stack frame when not returning it.

If you return the delegate up the call stack *and* use it in a downward direction, then the entire function would need to work with a garbage collected stack frame (or a gc portion of a stack frame).  This is probably rare.

All of this can be done now, if you put the important stuff in a class and return a delegate to a method of that class.  But it requires programmer intervention of course.

I think that to make this both (as) efficient (as possible) *and* work as expected everywhere, would require a level of analysis similar to checking const correctness in C++ (you need to know where every delegate might go and what stuff it uses from the stack).  And when you are done you have a feature that is going to be slow enough to use that I suspect most people will shy away from it most of the time.

The same issues apply to returning a reference to any local variable, which I think is generally permitted in C (though gcc does warn about this with -Wall).

Kevin