Jump to page: 1 2
Thread overview
[Issue 23175] -preview=in silently adds possible stack memory escape
Jun 10, 2022
Paul Backus
Jun 10, 2022
Paul Backus
Jun 10, 2022
Paul Backus
Jun 10, 2022
Paul Backus
Jun 15, 2022
Dlang Bot
Jun 16, 2022
Mathias LANG
Jun 16, 2022
Mathias LANG
Jun 16, 2022
Richard Cattermole
Jun 16, 2022
Paul Backus
Jun 16, 2022
Mathias LANG
Dec 17, 2022
Iain Buclaw
June 10, 2022
https://issues.dlang.org/show_bug.cgi?id=23175

Paul Backus <snarwin+bugzilla@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |snarwin+bugzilla@gmail.com

--- Comment #1 from Paul Backus <snarwin+bugzilla@gmail.com> ---
Some historical context:

Pn 2018-06-14, in commit 93411bed2 [], the spec was changed

On 2020-08-16, in commit 52e211539 [], the spec was changed to its current wording, which documents `-preview=in` and says that `in` parameters "behave as though" they are `const scope`.

[]: https://github.com/dlang/dlang.org/commit/93411bed2
[]: https://github.com/dlang/dlang.org/commit/52e211539

--
June 10, 2022
https://issues.dlang.org/show_bug.cgi?id=23175

--- Comment #2 from Paul Backus <snarwin+bugzilla@gmail.com> ---
Apologies for the previous comment. It was submitted in error while still incomplete.

--
June 10, 2022
https://issues.dlang.org/show_bug.cgi?id=23175

--- Comment #3 from Paul Backus <snarwin+bugzilla@gmail.com> ---
Some historical context:

Prior to 2013-02-06, the spec defined `in` as a no-op attribute (likely a
holdover from D1).

On 2013-02-06, in commit 26d4aec9c [1], `in` was defined as equivalent to `const scope`.

On 2018-01-25, in commit 71ad1b38d [2], the spec was changed to define `in` as equivalent to `const`. The commit message refers to issue 17928, which contains some relevant discussion.

On 2018-06-14, in commit 93411bed2 [3], the spec was changed to define `in` as `const scope`, with a note that the current implementation treated it as equivalent to `const`.

On 2020-08-16, in commit 52e211539 [4], the spec was changed to its current wording, which documents `-preview=in` and says that `in` parameters "behave as though" they are `const scope`.

[1]: https://github.com/dlang/dlang.org/commit/26d4aec9c [2]: https://github.com/dlang/dlang.org/commit/71ad1b38d [3]: https://github.com/dlang/dlang.org/commit/93411bed2 [4]: https://github.com/dlang/dlang.org/commit/52e211539

--
June 10, 2022
https://issues.dlang.org/show_bug.cgi?id=23175

--- Comment #4 from Paul Backus <snarwin+bugzilla@gmail.com> ---
My summary the historical information above:

* Except for a few months in 2018, `in` has consistently been documented as `const scope` since 2013.

* Until `-preview=in`, the compiler has consistently treated `in` as `const`.

* There has been considerable confusion over whether the spec or the compiler's behavior should be treated as authoritative w.r.t. the meaning of `in`.

In light of this information, I am inclined to agree with Jonathan M Davis's comment from issue 17928:

> If we want in to mean scope const instead of just const, then we're going to need a means of migrating to that without immediately breaking code

--
June 10, 2022
https://issues.dlang.org/show_bug.cgi?id=23175

--- Comment #5 from Steven Schveighoffer <schveiguy@gmail.com> ---
One further update, in Feb 2021, which is still there to this day, notes that the `in` storage class without the preview=in switch means `const`:

https://github.com/dlang/dlang.org/commit/8f05cb00c99c90d88550972199c7941e2ea8ca21

--
June 15, 2022
https://issues.dlang.org/show_bug.cgi?id=23175

Dlang Bot <dlang-bot@dlang.rocks> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull

--- Comment #6 from Dlang Bot <dlang-bot@dlang.rocks> ---
@dkorpel created dlang/dmd pull request #14219 "Fix 23175 - -preview=in silently adds possible stack memory escape" fixing this issue:

- Fix 23175 - -preview=in silently adds possible stack memory escape

https://github.com/dlang/dmd/pull/14219

--
June 16, 2022
https://issues.dlang.org/show_bug.cgi?id=23175

Mathias LANG <pro.mathias.lang@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pro.mathias.lang@gmail.com

--- Comment #7 from Mathias LANG <pro.mathias.lang@gmail.com> ---
As was pointed out in the lengthy forum thread:
- `scope` now leads to stack allocation;
- `-preview=in` is supposed to make `in` means `scope const`;

So I don't see how turning on a switch that is supposed to make `in` have the
semantic of `scope` is "silently breaking code" ??
Hello, *the whole point of `-preview`* is to introduce potentially-breaking
changes!

This is working as intended.

--
June 16, 2022
https://issues.dlang.org/show_bug.cgi?id=23175

--- Comment #8 from Steven Schveighoffer <schveiguy@gmail.com> ---
Let's look at foo written like this:

```d
string foo(const string s)
{
   return s;
}
```

Do you think there is any valid case where the compiler can be changed (even behind a switch) to make this start corrupting memory, yet not give any warning about it?

It is the same here. `in` means `const`. It's that way in the implementation, it says that right in the spec. This is *valid* code, with an *expected* meaning, and you are *changing the meaning* without warning.

You say that it's expected when you use -preview=in, because you specifically used that switch. Does this mean preview in is never meant to be used on existing code? Does this mean that at no time in the future will preview in be turned on by default? I must assume this is a "preview" of what is to come. If we never intend to make it a permanent semantic change, then we shouldn't call it a "preview".

I do not think we need to change the semantics of preview=in, or to disable optimizations based on it. What we need is a warning to tell the user that what he wrote before, while valid with the previous compiler, has memory corruption implications now that the semantics have changed.

--
June 16, 2022
https://issues.dlang.org/show_bug.cgi?id=23175

--- Comment #9 from Mathias LANG <pro.mathias.lang@gmail.com> ---
> I do not think we need to change the semantics of preview=in, or to disable optimizations based on it. What we need is a warning to tell the user that what he wrote before, while valid with the previous compiler, has memory corruption implications now that the semantics have changed.

You should have led with this. That's what `-transition` is for. It lists all instances of `in` usage so you can inspect them.

--
June 16, 2022
https://issues.dlang.org/show_bug.cgi?id=23175

Richard Cattermole <alphaglosined@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |alphaglosined@gmail.com

--- Comment #10 from Richard Cattermole <alphaglosined@gmail.com> ---
(In reply to Mathias LANG from comment #9)
> > I do not think we need to change the semantics of preview=in, or to disable optimizations based on it. What we need is a warning to tell the user that what he wrote before, while valid with the previous compiler, has memory corruption implications now that the semantics have changed.
> 
> You should have led with this. That's what `-transition` is for. It lists all instances of `in` usage so you can inspect them.

```
  -transition=<name>
                    help with language change identified by 'name'
```

That description could do with some improvement.

--
« First   ‹ Prev
1 2