Issue 19873 - function should be by default @system even with -preview=dip1000
Summary: function should be by default @system even with -preview=dip1000
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P1 normal
Assignee: No Owner
URL:
Keywords: pull, safe
Depends on:
Blocks:
 
Reported: 2019-05-14 18:08 UTC by Seb
Modified: 2021-11-16 13:16 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Seb 2019-05-14 18:08:47 UTC
---
@system unittest
{
    int i;
    int* p = &i;
    int[]  slice = [0, 1, 2, 3, 4];
    int[5] arr   = [0, 1, 2, 3, 4];
    int*[]  slicep = [p];
}
---

Compiles fine with -unittest -preview=dip1000.
Remove @system:

---
unittest
{
    int i;
    int* p = &i;
    int[]  slice = [0, 1, 2, 3, 4];
    int[5] arr   = [0, 1, 2, 3, 4];
    int*[]  slicep = [p];
}
---

$ dmd -unittest -preview=dip1000 foo.d

And now it errors:

---
foo.d(7): Error: scope variable p may not be copied into allocated memory
---
Comment 1 Jacob Carlborg 2019-05-14 19:29:49 UTC
Would it be a good idea to infer attributes for unittest?
Comment 2 Seb 2019-05-14 23:03:15 UTC
I think this is what's (wrongly?) happening here which is probably why the unittest without @system results in an error.
Comment 3 Jacob Carlborg 2019-05-15 05:38:17 UTC
(In reply to Seb from comment #2)
> I think this is what's (wrongly?) happening here which is probably why the
> unittest without @system results in an error.

Then it sounds like the inference is broken.
Comment 4 Walter Bright 2020-03-12 08:11:51 UTC
unittest has nothing to do with the issue. Calling it `void test()` instead does the same thing.
Comment 5 Walter Bright 2020-03-12 08:40:17 UTC
Here's what's happening:

------- reduced test case ----------
void test() {
    int i;
    int* p = &i; // <= `scope` is inferred for `p`
    int*[]  slicep = [p];
}
----------------------

If it is declared:

    scope int*p = &i;

the error will occur regardless of the application of @system. Looking at the code in escape.d checkAssignEscape():

------------
// Try to infer 'scope' for va if in a function not marked @system
bool inferScope = false;
if (va && sc.func && sc.func.type && sc.func.type.ty == Tfunction)
    inferScope = (cast(TypeFunction)sc.func.type).trust != TRUST.system;
------------
https://github.com/dlang/dmd/blob/master/src/dmd/escape.d#L578

and there it is. It is deliberate. What's going on is there are actually 4 trust levels - system, trusted, safe, and default_. The fourth state default_ is necessary for functions that will have their attributes inferred. I went a little further here to assume a bit more @safety in default_ functions unless the user specifically marked it as @system.

As you know, I'd like to see @safe as the default. While this issue is a bug, it's in the direction we need to go anyway so it shouldn't be "fixed".
Comment 6 Walter Bright 2020-03-12 08:52:33 UTC
https://github.com/dlang/dmd/pull/10898
Comment 7 Mathias LANG 2020-03-12 09:10:34 UTC
> As you know, I'd like to see @safe as the default. While this issue is a bug, it's in the direction we need to go anyway so it shouldn't be "fixed".

What ? It *is* a bug. By the current definition of the language, an error that shouldn't be triggered in `@system` code is triggered in `@system` code.

`@safe` by default is orthogonal and should be handled on its own. Breaking the language because "we're going to do it" is not reasonable, especially when this DIP hasn't been accepted and is so far into the future.
Comment 8 Dlang Bot 2021-07-06 12:10:30 UTC
@dkorpel created dlang/dmd pull request #12821 "Fix issue 19873 - function should be by default @system even with -preview=dip1000" fixing this issue:

- fix issue 19873 - function should be by default @system even with -preview=dip1000

https://github.com/dlang/dmd/pull/12821
Comment 9 Dlang Bot 2021-11-16 13:16:10 UTC
dlang/dmd pull request #12821 "Fix issue 19873 - function should be by default @system even with -preview=dip1000" was merged into master:

- ba1b790238c1941bc78e0e5a80993db85d07a5b2 by dkorpel:
  fix issue 19873 - function should be by default @system even with -preview=dip1000

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