Bug 927 – writefln() is duplicating values for parameters supplied

Status
RESOLVED
Resolution
INVALID
Severity
normal
Priority
P2
Component
phobos
Product
D
Version
D1 (retired)
Platform
PowerPC
OS
Mac OS X
Creation time
2007-02-03T12:43:00Z
Last change time
2014-02-15T13:12:50Z
Assigned to
bugzilla
Creator
rmann-d-lang

Comments

Comment #0 by rmann-d-lang — 2007-02-03T12:43:09Z
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).
Comment #1 by rmann-d-lang — 2007-02-03T12:55:09Z
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");
Comment #2 by bugzilla — 2007-02-03T13:17:47Z
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.
Comment #3 by rmann-d-lang — 2007-02-03T13:33:28Z
Ah! Sorry. Force of habit, and mistakenly thinking "char[]" was the same as Java's "String". Thanks!
Comment #4 by fvbommel — 2007-02-03T19:15:18Z
[email protected] wrote: > ------- Comment #2 from [email protected] 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.
Comment #5 by rmann-d-lang — 2007-02-03T21:41:45Z
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.
Comment #6 by fvbommel — 2007-02-04T04:30:43Z
[email protected] wrote: > ------- Comment #5 from [email protected] 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.
Comment #7 by kevinbealer — 2007-02-04T17:05:19Z
Frits van Bommel wrote: > [email protected] wrote: >> ------- Comment #5 from [email protected] 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