Thread overview
Compiler bug or incorrect usage for pointer of Struct?
Jan 13
Heromyth
Jan 13
Temtaime
Jan 14
Heromyth
Jan 14
Temtaime
Jan 14
Heromyth
Jan 14
Temtaime
Jan 15
Heromyth
January 13
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);
    }
}
January 13
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.

January 13
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
January 14
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);
	}
}

January 14
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
January 14
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?

January 14
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
January 15
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!