Jump to page: 1 2 3
Thread overview
[Issue 19183] DIP1000 defeated if auto used instead of scope in variable declaration with template this member function
Aug 20, 2018
Atila Neves
Aug 20, 2018
ag0aep6g
Aug 21, 2018
Atila Neves
Aug 21, 2018
ZombineDev
Aug 21, 2018
Atila Neves
Aug 21, 2018
ag0aep6g
Aug 21, 2018
Atila Neves
Aug 21, 2018
ag0aep6g
Aug 21, 2018
Atila Neves
Aug 21, 2018
ag0aep6g
Aug 22, 2018
anonymous4
Aug 22, 2018
Atila Neves
Aug 22, 2018
ag0aep6g
Aug 23, 2018
Atila Neves
Aug 23, 2018
Mike Franklin
Aug 23, 2018
ag0aep6g
Sep 21, 2019
Mike Franklin
Sep 23, 2019
Atila Neves
Sep 23, 2019
Mike Franklin
Sep 23, 2019
Atila Neves
Mar 04, 2020
Walter Bright
August 20, 2018
https://issues.dlang.org/show_bug.cgi?id=19183

Atila Neves <atila.neves@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |safe

--
August 20, 2018
https://issues.dlang.org/show_bug.cgi?id=19183

ag0aep6g <ag0aep6g@gmail.com> changed:

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

--- Comment #1 from ag0aep6g <ag0aep6g@gmail.com> ---
Doesn't compile for me (2.081.2). I get:

    test.d(7): Error: scope variable s assigned to gInts with longer lifetime

https://run.dlang.io/is/74B2qq

--
August 21, 2018
https://issues.dlang.org/show_bug.cgi?id=19183

--- Comment #2 from Atila Neves <atila.neves@gmail.com> ---
I used dmd 2.081.2. I just tried it again and it compiles. Just in case there's something wrong with the Arch Linux package, I used dmd.081.2 from install.sh. Same result. I also tried the 2.082.0 beta 1. Same thing.

--
August 21, 2018
https://issues.dlang.org/show_bug.cgi?id=19183

ZombineDev <petar.p.kirov@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |petar.p.kirov@gmail.com

--- Comment #3 from ZombineDev <petar.p.kirov@gmail.com> ---
It seems that you're not passing -dip1000 on the command-line. I'm also on Arch Linux. Here's what I get:

~/dlang/install.sh dmd
source ~/dlang/dmd-2.081.2/activate
dmd -dip1000 scope.d

  scope.d(7): Error: scope variable s assigned to gInts with longer lifetime

--
August 21, 2018
https://issues.dlang.org/show_bug.cgi?id=19183

--- Comment #4 from Atila Neves <atila.neves@gmail.com> ---
I made a mistake when I posted the code. This compiles but shouldn't (the only difference is removing `scope` from the destructor).

I also surrounded the call to `free` with @trusted instead of the whole constructor.



@safe:

const(int)* gInts;

void main() {
    auto s = MyStruct(10);
    gInts = s.ints;
}

struct MyStruct
{
    import core.stdc.stdlib;
    int* ints;
    this(int size) @trusted { ints = cast(int*) malloc(size); }
    ~this() { () @trusted { free(ints); }(); }
    scope ptr(this This)() { return ints; }
}

--
August 21, 2018
https://issues.dlang.org/show_bug.cgi?id=19183

--- Comment #5 from ag0aep6g <ag0aep6g@gmail.com> ---
(In reply to Atila Neves from comment #4)
> I made a mistake when I posted the code. This compiles but shouldn't (the only difference is removing `scope` from the destructor).
[...]
> @safe:
> 
> const(int)* gInts;
> 
> void main() {
>     auto s = MyStruct(10);
>     gInts = s.ints;
> }
> 
> struct MyStruct
> {
>     import core.stdc.stdlib;
>     int* ints;
>     this(int size) @trusted { ints = cast(int*) malloc(size); }
>     ~this() { () @trusted { free(ints); }(); }
>     scope ptr(this This)() { return ints; }
> }

You're not calling `ptr`. `main` reads the `ints` field directly. That's allowed, of course. D has no way of preventing @safe code from accessing a local struct's field.

If the code is changed to actually call `ptr`, it still compiles. Maybe that shouldn't be allowed, but it's not obvious to me from the code. If this is a safety violation, we should be able to show something like memory corruption, mutating immutable data, stuff like that.

As it is, the only obvious safety violation is in the @trusted destructor. It assumes that the `ints` field is not accessible from @safe code, but that's wrong. Unfortunately, misusing @trusted like that is necessary for stuff like this, but it still means breaking the @trusted promise. The consequence is that (at least) the whole module must be checked manually to uphold the assumption of the badly @trusted method. One cannot rely on @safe to catch mistakes in that area. So if none of MyStruct's methods can be allowed to return `ints`, because the destructor relies on that, then that must be checked manually.

--
August 21, 2018
https://issues.dlang.org/show_bug.cgi?id=19183

--- Comment #6 from Atila Neves <atila.neves@gmail.com> ---
I typed the code from memory and made a mistake. Yes, it's supposed to be `.ptr`. There's no way I can call core.stdc.stdlib.free without @trusted _somewhere_, since `free` isn't @safe - free(42) is obviously going to crash.

What's @trusted is that I'm calling `free` on the same pointer I allocated in the constructor, and the other thing needing checking is the postblit.

This all is besides the point, given the code below has no @trusted anywhere, compiles, and shouldn't:

-------------------------------
@safe:

const(int)* gInts;

void main() {
    auto s = MyStruct();
    gInts = s.ptr;
}

struct MyStruct {
    int* _ints;
    auto ptr(this This)() { return _ints; }
}
-------------------------------


As in the original report:


* Adding a no-op destructor does nothing (i.e. the code still compiles):

~this() {} // ok but shouldn't be


* Adding a no-op destructor annotate with scope causes a compile-time error:

~this() scope { }

baz.d(7): Error: scope variable s assigned to gInts with longer lifetime


* With no destructor at all but replacing `auto` with `scope` also fails to compile:

scope s = MyStruct();

baz.d(7): Error: scope variable s assigned to gInts with longer lifetime




Weirdly enough, changing `gInts` to a static array and returning &_ints[0] in ptr also fails to compile;

--
August 21, 2018
https://issues.dlang.org/show_bug.cgi?id=19183

--- Comment #7 from ag0aep6g <ag0aep6g@gmail.com> ---
(In reply to Atila Neves from comment #6)
> What's @trusted is that I'm calling `free` on the same pointer I allocated in the constructor, and the other thing needing checking is the postblit.

If you want to keep the @trusted promise, you can't rely on the pointer being the same. @safe code can mess with it. There's no way around that. Maybe there should be.

So what you do is this: You mark the `free` call as @trusted anyway, because there's no other way. And then you have to check manually that your @safe code doesn't mess with the pointer. Ideally, manual checks wouldn't be needed for @safe code, but that's the price you pay for breaking the @trusted promise.


> This all is besides the point, given the code below has no @trusted anywhere, compiles, and shouldn't:
> 
> -------------------------------
> @safe:
> 
> const(int)* gInts;
> 
> void main() {
>     auto s = MyStruct();
>     gInts = s.ptr;
> }
> 
> struct MyStruct {
>     int* _ints;
>     auto ptr(this This)() { return _ints; }
> }
> -------------------------------

Why shouldn't this compile? There's no `scope` anywhere now (except maybe an inferred one). You're just copying an `int*` around. That's not against any rules.


> As in the original report:
> 
> 
> * Adding a no-op destructor does nothing (i.e. the code still compiles):
> 
> ~this() {} // ok but shouldn't be

Why shouldn't it compile with the destructor? The destructor is marked as @safe and does nothing. So it's the same as not having a destructor at all, no?


> * Adding a no-op destructor annotate with scope causes a compile-time error:
> 
> ~this() scope { }
> 
> baz.d(7): Error: scope variable s assigned to gInts with longer lifetime

Interesting. Looks like a `scope` destructor also makes any instances implicitly `scope`. I don't think I understand DIP 1000 or its implementation well enough to this. DIP 1000 doesn't seem to mention it, as far as I see.


> * With no destructor at all but replacing `auto` with `scope` also fails to compile:
> 
> scope s = MyStruct();
> 
> baz.d(7): Error: scope variable s assigned to gInts with longer lifetime

This makes sense to me. With `s` being `scope`, the compiler now checks it and its contents don't leave `main`.

If `s` is not `scope`, there's no indication that its contents shouldn't be allowed to leave `main`.


> Weirdly enough, changing `gInts` to a static array and returning &_ints[0] in ptr also fails to compile;

Do you mean making `_ints` a static array (e.g., `int[1] _ints;`)? If so, the code effectively becomes:

----
const(int)* gInts;
void main() @safe
{
    int _ints;
    gInts = &_ints;
}
----

Leaking a reference to a local variable is not allowed, of course. It's very different from copying a pointer.

If you really mean making `gInts` a static array, could you post the full code? I don't understand what you mean in that case.

--
August 21, 2018
https://issues.dlang.org/show_bug.cgi?id=19183

--- Comment #8 from Atila Neves <atila.neves@gmail.com> ---
> @safe code can mess with it

No it can't, that's the point of @safe. Mess with it how?

> Why shouldn't this compile? There's no `scope` anywhere now (except maybe an inferred one)

Function template => inferred attributes => scope. But fine, this compiles and shouldn't:

----------
@safe:

const(int)* gInts;

void main() {
    auto s = MyStruct();
    gInts = s.ptr;
}

struct MyStruct {
    int* ints;
    scope ptr(this This)() { return ints; }
}
----------

> This makes sense to me. With `s` being `scope`, the compiler now checks it and its contents don't leave `main`.


Because, from DIP1000:

A variable is inferred to be scope if it is initialized with a value that has a non-∞ lifetime.


This very same code without a template this and instead manual instantiations of the three versions (mutable, const, immutable) doesn't compile _even_ if `auto` is used in the variable declaration.

Furthermore, if `auto` was enough to get away from compiler checks, DIP1000 would be useless since nobody is going to remember to write `scope`.

--
August 21, 2018
https://issues.dlang.org/show_bug.cgi?id=19183

--- Comment #9 from ag0aep6g <ag0aep6g@gmail.com> ---
(In reply to Atila Neves from comment #8)
> > @safe code can mess with it
> 
> No it can't, that's the point of @safe. Mess with it how?

@safe applies to functions/methods, not variables/fields. You can't forbid @safe code from accessing a visible variable.

Example that's similar to your code:

----
int x = 41;
int y = 42;

struct S
{
    int* p;
    void t() @trusted
    {
        import core.stdc.stdlib: malloc;
        if (p is null) p = cast(int*) malloc(2 * int.sizeof);
        p[1] = 13;
    }
    void s() @safe { p = &x; }
}

void main() @safe
{
    S s;
    s.s();
    s.t();
    assert(y == 42); /* Fails. */
}
----

Here, `t` assumes that `p` will be the result of `malloc`. But there is no way in the language to force @safe code to respect that assumption. You can always add an @safe method like `s` that messes with `p`, and then `t` will violate safety.

DIP 1000 doesn't help here. It doesn't add a mechanism with which we could make `p` inaccessible from `s`.

We're drifting off-topic topic here, though. Maybe we should move it to the forum if we're not on the same page by now?


[...]
> ----------
> @safe:
> 
> const(int)* gInts;
> 
> void main() {
>     auto s = MyStruct();
>     gInts = s.ptr;
> }
> 
> struct MyStruct {
>     int* ints;
>     scope ptr(this This)() { return ints; }
> }
> ----------

You're still just copying an `int*` around, which isn't unsafe. `ptr` is inferred as `return` so it's allowed to return `ints`. I suppose your point is that `return` shouldn't be inferred? Maybe, but it's not obvious from the code. A test case that shows actual damage (memory corruption) would go a long way.


[...]
> Because, from DIP1000:
> 
> A variable is inferred to be scope if it is initialized with a value that has a non-∞ lifetime.

Without `scope` on the variable and without a destructor, there is no indication that `s.ints` has a non-infinite lifetime.


> This very same code without a template this and instead manual instantiations of the three versions (mutable, const, immutable) doesn't compile _even_ if `auto` is used in the variable declaration.

I'm not sure if I understand that correctly, but this compiles just fine:

----
@safe:

const(int)* gInts;

void main() {
    auto s = MyStruct();
    gInts = s.ptr;
}

struct MyStruct {
    int* _ints;
    auto ptr() { return _ints; }
    auto ptr() const { return _ints; }
    auto ptr() immutable { return _ints; }
}
----


> Furthermore, if `auto` was enough to get away from compiler checks, DIP1000 would be useless since nobody is going to remember to write `scope`.

Copying a pointer isn't unsafe. There's no reason for the compiler to reject it unless you add further restrictions by using `scope`.

--
« First   ‹ Prev
1 2 3