www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Compiler bug or incorrect usage for pointer of Struct?

reply Heromyth <bitworld qq.com> writes:
When executing the test code, it will exit abnormally. It seems 
*this.output* is pointing a free memory when executing 
*writer.write(dom)*.

I'm not sure whether there is a bug in the compiler. If it is, I 
can file a bug. If not, somebody can tell me how to fix this.

Thanks!

Here is my test code (You can test it in 
https://run.dlang.io/is/0OPBVI):

==============================
import std.stdio;

void main()
{
     bugTest();
}

// This code is stripped from 
https://github.com/Kozzi11/experimental.xml/blob/master/source/std/experimental/xml/writer.d
void bugTest()
{
     string dom = "XML DOM";

     auto file = File("catalogue.xml", "w");

     // It's OK
     // auto textWriter = file.lockingTextWriter;
     // textWriter.writerFor.write(dom);

     // There's a bug
     auto writer = writerFor(file.lockingTextWriter);
     writer.write(dom);

     file.close();
}

auto writerFor(OutRange)(auto ref OutRange outRange)
{
     auto res = Writer!(OutRange)();
     res.setSink(outRange);
     return res;
}

struct Writer(OutRange)
{
     private OutRange* output;

     void setSink(ref OutRange output)
     {
         this.output = &output;
         writeln("in setSink: ", this.output);
     }

     // void setSink(typeof(output) output)
     // {
     // 	this.output = output;
     // }

     void write(string s)
     {
         writeln("in write: ", output);
         output.put(s);
     }
}
Jan 13
next sibling parent Temtaime <temtaime gmail.com> writes:
On Saturday, 13 January 2018 at 12:22:17 UTC, Heromyth wrote:
 When executing the test code, it will exit abnormally. It seems 
 *this.output* is pointing a free memory when executing 
 *writer.write(dom)*.

 I'm not sure whether there is a bug in the compiler. If it is, 
 I can file a bug. If not, somebody can tell me how to fix this.

 Thanks!

 Here is my test code (You can test it in 
 https://run.dlang.io/is/0OPBVI):

 ==============================
 import std.stdio;

 void main()
 {
     bugTest();
 }

 // This code is stripped from 
 https://github.com/Kozzi11/experimental.xml/blob/master/source/std/experimental/xml/writer.d
 void bugTest()
 {
     string dom = "XML DOM";

     auto file = File("catalogue.xml", "w");

     // It's OK
     // auto textWriter = file.lockingTextWriter;
     // textWriter.writerFor.write(dom);

     // There's a bug
     auto writer = writerFor(file.lockingTextWriter);
     writer.write(dom);

     file.close();
 }

 auto writerFor(OutRange)(auto ref OutRange outRange)
 {
     auto res = Writer!(OutRange)();
     res.setSink(outRange);
     return res;
 }

 struct Writer(OutRange)
 {
     private OutRange* output;

     void setSink(ref OutRange output)
     {
         this.output = &output;
         writeln("in setSink: ", this.output);
     }

     // void setSink(typeof(output) output)
     // {
     // 	this.output = output;
     // }

     void write(string s)
     {
         writeln("in write: ", output);
         output.put(s);
     }
 }
It's your bug. file.lockingTextWriter does not return a reference. It is passed to writerFor as a copy on the stack and in setSink you get a pointer of a stack variable that gets killed after writerFor returns.
Jan 13
prev sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Sat, Jan 13, 2018 at 12:22:17PM +0000, Heromyth via Digitalmars-d wrote:
[...]
 auto writerFor(OutRange)(auto ref OutRange outRange)
 {
     auto res = Writer!(OutRange)();
     res.setSink(outRange);
     return res;
 }
 
 struct Writer(OutRange)
 {
     private OutRange* output;
 
     void setSink(ref OutRange output)
     {
         this.output = &output;
[...] Here's the bug. `output` refers to a local variable (parameter) in writerFor(), which goes out of scope after writerFor() exits, so this.output becomes a dangling pointer. T -- Public parking: euphemism for paid parking. -- Flora
Jan 13
parent reply Heromyth <bitworld qq.com> writes:
On Saturday, 13 January 2018 at 14:11:23 UTC, H. S. Teoh wrote:
 On Sat, Jan 13, 2018 at 12:22:17PM +0000, Heromyth via 
 Digitalmars-d wrote: [...]
 auto writerFor(OutRange)(auto ref OutRange outRange)
 {
     auto res = Writer!(OutRange)();
     res.setSink(outRange);
     return res;
 }
 
 struct Writer(OutRange)
 {
     private OutRange* output;
 
     void setSink(ref OutRange output)
     {
         this.output = &output;
[...] Here's the bug. `output` refers to a local variable (parameter) in writerFor(), which goes out of scope after writerFor() exits, so this.output becomes a dangling pointer. T
I have another test. It runs whithout any error. Here it is: import std.stdio; void main() { Tester tester = new Tester(buildWriter()); tester.run("It's OK"); } struct StringWriter { void put(string s) { writeln(s); } } StringWriter buildWriter() { return StringWriter(); } class Tester { private StringWriter* writer; this(StringWriter w) { writer = &w; writer.put("ok"); } void run(string m) { writer.put(m); } }
Jan 13
parent reply Temtaime <temtaime gmail.com> writes:
On Sunday, 14 January 2018 at 04:02:09 UTC, Heromyth wrote:
 On Saturday, 13 January 2018 at 14:11:23 UTC, H. S. Teoh wrote:
 On Sat, Jan 13, 2018 at 12:22:17PM +0000, Heromyth via 
 Digitalmars-d wrote: [...]
 auto writerFor(OutRange)(auto ref OutRange outRange)
 {
     auto res = Writer!(OutRange)();
     res.setSink(outRange);
     return res;
 }
 
 struct Writer(OutRange)
 {
     private OutRange* output;
 
     void setSink(ref OutRange output)
     {
         this.output = &output;
[...] Here's the bug. `output` refers to a local variable (parameter) in writerFor(), which goes out of scope after writerFor() exits, so this.output becomes a dangling pointer. T
I have another test. It runs whithout any error. Here it is: import std.stdio; void main() { Tester tester = new Tester(buildWriter()); tester.run("It's OK"); } struct StringWriter { void put(string s) { writeln(s); } } StringWriter buildWriter() { return StringWriter(); } class Tester { private StringWriter* writer; this(StringWriter w) { writer = &w; writer.put("ok"); } void run(string m) { writer.put(m); } }
https://run.dlang.io/is/RUHtqK It's not ok dude It runs because you don't use any variable inside the struct and because struct members are simple functions with hidden parameter
Jan 14
parent reply Heromyth <bitworld qq.com> writes:
On Sunday, 14 January 2018 at 08:05:34 UTC, Temtaime wrote:
 On Sunday, 14 January 2018 at 04:02:09 UTC, Heromyth wrote:
 On Saturday, 13 January 2018 at 14:11:23 UTC, H. S. Teoh wrote:
 On Sat, Jan 13, 2018 at 12:22:17PM +0000, Heromyth via 
 Digitalmars-d wrote: [...]
 https://run.dlang.io/is/RUHtqK
 It's not ok dude
 It runs because you don't use any variable inside the struct 
 and because struct members are simple functions with hidden 
 parameter
Thanks. It's really dangerous to use a pointer to struct! I have created another test based on your code. See here https://run.dlang.io/is/LOeMKG I add *ref* in the constructor and add a new template fucntion writerFor. So, it goes back the scenario in my first post. Tester tester = new Tester(buildWriter()); // can't compile. The compiler does the right thing. Tester tester = writerFor(buildWriter()); // Here is a bug, because the compiler takes this! Am I right?
Jan 14
parent reply Temtaime <temtaime gmail.com> writes:
On Sunday, 14 January 2018 at 13:24:14 UTC, Heromyth wrote:
 On Sunday, 14 January 2018 at 08:05:34 UTC, Temtaime wrote:
 On Sunday, 14 January 2018 at 04:02:09 UTC, Heromyth wrote:
 On Saturday, 13 January 2018 at 14:11:23 UTC, H. S. Teoh 
 wrote:
 On Sat, Jan 13, 2018 at 12:22:17PM +0000, Heromyth via 
 Digitalmars-d wrote: [...]
 https://run.dlang.io/is/RUHtqK
 It's not ok dude
 It runs because you don't use any variable inside the struct 
 and because struct members are simple functions with hidden 
 parameter
Thanks. It's really dangerous to use a pointer to struct! I have created another test based on your code. See here https://run.dlang.io/is/LOeMKG I add *ref* in the constructor and add a new template fucntion writerFor. So, it goes back the scenario in my first post. Tester tester = new Tester(buildWriter()); // can't compile. The compiler does the right thing. Tester tester = writerFor(buildWriter()); // Here is a bug, because the compiler takes this! Am I right?
There's no bug in compiler. "auto ref" can be NOT a reference. It depends on its parameter. RValues are passed by value. given writerFor(buildWriter()); it becomes auto writerFor(OutRange)(OutRange outRange) // NO REF, parameter is on the stack { auto res = new Tester(outRange); return res; } and after writerFor the returned object points on a variable inside writerFor which already died
Jan 14
parent Heromyth <bitworld qq.com> writes:
On Sunday, 14 January 2018 at 14:12:55 UTC, Temtaime wrote:
 On Sunday, 14 January 2018 at 13:24:14 UTC, Heromyth wrote:
 On Sunday, 14 January 2018 at 08:05:34 UTC, Temtaime wrote:
 On Sunday, 14 January 2018 at 04:02:09 UTC, Heromyth wrote:
 On Saturday, 13 January 2018 at 14:11:23 UTC, H. S. Teoh 
 wrote:
 On Sat, Jan 13, 2018 at 12:22:17PM +0000, Heromyth via 
 Digitalmars-d wrote: [...]
 https://run.dlang.io/is/RUHtqK
 It's not ok dude
 It runs because you don't use any variable inside the struct 
 and because struct members are simple functions with hidden 
 parameter
Thanks. It's really dangerous to use a pointer to struct! I have created another test based on your code. See here https://run.dlang.io/is/LOeMKG I add *ref* in the constructor and add a new template fucntion writerFor. So, it goes back the scenario in my first post. Tester tester = new Tester(buildWriter()); // can't compile. The compiler does the right thing. Tester tester = writerFor(buildWriter()); // Here is a bug, because the compiler takes this! Am I right?
There's no bug in compiler. "auto ref" can be NOT a reference. It depends on its parameter. RValues are passed by value. given writerFor(buildWriter()); it becomes auto writerFor(OutRange)(OutRange outRange) // NO REF, parameter is on the stack { auto res = new Tester(outRange); return res; } and after writerFor the returned object points on a variable inside writerFor which already died
I see. Thanks!
Jan 14