www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 927] New: writefln() is duplicating values for parameters supplied

reply d-bugmail puremagic.com writes:
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).


-- 
Feb 03 2007
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=927






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");


-- 
Feb 03 2007
prev sibling next sibling parent reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=927


bugzilla digitalmars.com changed:

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





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.


-- 
Feb 03 2007
parent Frits van Bommel <fvbommel REMwOVExCAPSs.nl> writes:
d-bugmail puremagic.com wrote:

[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.
Feb 03 2007
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=927






Ah! Sorry. Force of habit, and mistakenly thinking "char[]" was the same as
Java's "String".

Thanks!


-- 
Feb 03 2007
prev sibling next sibling parent reply Derek Parnell <derek psych.ward> writes:
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
Feb 03 2007
parent Derek Parnell <derek psych.ward> writes:
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
Feb 03 2007
prev sibling parent reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=927






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.


-- 
Feb 03 2007
parent reply Frits van Bommel <fvbommel REMwOVExCAPSs.nl> writes:
d-bugmail puremagic.com wrote:

 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.
Feb 04 2007
parent Kevin Bealer <kevinbealer gmail.com> writes:
Frits van Bommel wrote:
 d-bugmail puremagic.com wrote:

 -------
 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
Feb 04 2007