Thread overview
Compiler bug or incorrect usage for pointer of Struct?
Jan 13
Heromyth
Jan 13
Temtaime
6 days ago
Heromyth
6 days ago
Temtaime
6 days ago
Heromyth
6 days ago
Temtaime
5 days ago
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
6 days ago
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);
	}
}

6 days ago
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
6 days ago
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?

6 days ago
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
5 days ago
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!