Issue 2127 - inliner turns struct "return *this" from by-value into by-ref
Summary: inliner turns struct "return *this" from by-value into by-ref
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D1 (retired)
Hardware: x86 All
: P2 critical
Assignee: No Owner
URL:
Keywords: patch, wrong-code
Depends on:
Blocks: 3761
  Show dependency treegraph
 
Reported: 2008-05-24 06:08 UTC by Frits van Bommel
Modified: 2014-02-24 15:33 UTC (History)
3 users (show)

See Also:


Attachments
tentative fix for this for the d2 branch (might work on d1 as well, untested) (1.53 KB, patch)
2010-05-25 01:59 UTC, Brad Roberts
Details | Diff
d2 fix, version 2 (1.49 KB, patch)
2010-05-25 03:05 UTC, Brad Roberts
Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description Frits van Bommel 2008-05-24 06:08:16 UTC
The following code:
-----
module evbug;
import std.stdio;

struct S {
  int last = 0;
  S opCall(int i) {
    writefln("%s %s", last, i);
    last = i;
    return *this;
  }
}

void main() {
  S t;
  t(1)(2);
  t(3);
}
-----
should output
=====
0 1
1 2
1 3
=====
(Note that opCall returns a copy of the original struct, so the first and last calls will be passed &t as 'this', but the second is passed a pointer to a copy)
With "dmd -run test.d", this all works fine.

However, when run with "dmd -inline -run test.d":
=====
0 1
1 2
2 3
=====
(note the last line)
The "return *this" seems suddenly to have compiled as a reference return instead of a value return. (This can be verified by making opCall print the address as well; the uninlined version will only report the same address for the first and last call, while the inlined version reports the same address every time)

GDC doesn't exhibit this problem, presumably because it uses the GCC inliner.
Comment 1 Brad Roberts 2010-05-25 01:59:48 UTC
Created attachment 642 [details]
tentative fix for this for the d2 branch (might work on d1 as well, untested)

This bug exists in the d2 code base as well.  This patch fixes the test case and passes the test suite.
Comment 2 Brad Roberts 2010-05-25 03:05:16 UTC
Created attachment 643 [details]
d2 fix, version 2

Oops.. the version I attached previously was an older copy.  This is the newer version that moves the changes down from FuncDeclaration::inlineScan to doInline and fixes a problem with the first version.
Comment 3 Walter Bright 2010-05-30 11:03:31 UTC
http://www.dsource.org/projects/dmd/changeset/505