Thread overview
foreach + ref bug
Jun 28, 2018
Andrea Fontana
Jun 28, 2018
ketmar
Jun 28, 2018
Andrea Fontana
Jun 28, 2018
ketmar
June 28, 2018
1- Take a range with a non-ref front() method.
2- Use it with a foreach(ref element; Range) (ex: myrange.each!(x => writeln(x)) since "each" use a ref foreach)

(Bug: https://issues.dlang.org/show_bug.cgi?id=11934)

What happens? This code should be rejected but instead it is lowered and each element's ctor is called but not dtor. That means, in my case, that my library never frees a lot of resources (and relative referenced objects).

I think that (invalid!) code should trigger some kind of error/warning.

I tried to put here [1] this code:

if (auto fd = sfront.isFuncDeclaration())
{
  // If front is a function, it must return a ref if used with foreach(ref x; ...)
  if ((p.storageClass & STC.ref_) /* && front is ref */)
  {
    fs.error("`%s.front` isn't a ref", oaggr.toChars());
    goto case Terror;
  }
}

But I can't (and nobody can, apparently) tell how to check if front is returning a ref.

fd.type.toChars() == "pure nothrow @nogc @property ref @safe int() return"

But I still can't understand how this code take "ref" out from fd.type :)
Someone suggests to check fd.type.nextOf but it is simply "int" (return type of front() function)

Any hint?

Andrea

[1] https://github.com/dlang/dmd/blob/master/src/dmd/statementsem.d#L1463
[2] https://github.com/dlang/dmd/blob/master/src/dmd/hdrgen.d#L1883
June 28, 2018
Andrea Fontana wrote:

it is all fairly easy, i fixed (workarounded) this bug in Aliced long time ago[1]. patch is againd an older dmd, but it should put you on the track.


[1] http://dpaste.com/1SC0T6G
June 28, 2018
On Thursday, 28 June 2018 at 10:39:21 UTC, ketmar wrote:
> Andrea Fontana wrote:
>
> it is all fairly easy, i fixed (workarounded) this bug in Aliced long time ago[1]. patch is againd an older dmd, but it should put you on the track.
>
>
> [1] http://dpaste.com/1SC0T6G

Thank you! Not that obvious for me, I've just started watching at dmd code.
Now the question is: should we accept foreach(ref) for non-ref ranges?

IMHO the right thing to do is to reject that code. I think this is going to brake a lot of existing phobos/user code so probably we should go through a deprecation process to have some time to fix all parts. And in this period we can use the workaround above.

What to do?

Andrea

June 28, 2018
Andrea Fontana wrote:

> Thank you! Not that obvious for me, I've just started watching at dmd code.
> Now the question is: should we accept foreach(ref) for non-ref ranges?
>
> IMHO the right thing to do is to reject that code.

i don't see a good reason to do it. the only thing it does is forces a user to write alot of duplicated code: basically, exactly the same boilerplate, but one with ref, and one without ref. which, in turn, leads either to string mixin abuse (and impossibility of debugging/syntax highlighting), or to inevitable bugs from copy/paste.

this problem has no nice solution if we'll add code-generating templates to the picture, but just silently dropping `ref` causes much less headache, i believe.