Issue 14781 - [REG2.067] impure delegate to pure function context should be able to modify context
Summary: [REG2.067] impure delegate to pure function context should be able to modify ...
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P1 regression
Assignee: No Owner
URL:
Keywords: pull, rejects-valid
Depends on:
Blocks:
 
Reported: 2015-07-07 15:56 UTC by Steven Schveighoffer
Modified: 2015-09-02 04:10 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Steven Schveighoffer 2015-07-07 15:56:10 UTC
In a followup to issue 9148, which made this illegal:

int impureFuncCall() { static int g; return g; }
auto foo(out int delegate() pure pureDg) pure {
    int x;
    auto bar()() {
        impureFuncCall();
            x = 1;  // Error: impure function 'bar' cannot access variable 'x'
        // declared in enclosing pure function 'foo'
    }

    pureDg = &(bar!());

    int dg() pure { return x;}
    return &dg;
}


This should be allowed. This doesn't violate purity of the returned delegate because the context pointer is mutable -- it's weak pure.

This is similar to how a class can have pure and impure functions both accessing the same data.
Comment 1 Kenji Hara 2015-07-07 16:55:43 UTC
Thanks.
Comment 3 Denis Shelomovskii 2015-07-08 16:54:02 UTC
(In reply to Steven Schveighoffer from comment #0)
> In a followup to issue 9148, which made this illegal:
> 
> int impureFuncCall() { static int g; return g; }
> auto foo(out int delegate() pure pureDg) pure {
>     int x;
>     auto bar()() {
>         impureFuncCall();
>             x = 1;  // Error: impure function 'bar' cannot access variable
> 'x'
>         // declared in enclosing pure function 'foo'
>     }
> 
>     pureDg = &(bar!());
> 
>     int dg() pure { return x;}
>     return &dg;
> }
> 
> 
> This should be allowed.
> ...

Do you mean this code should compile? If so, either pure function `bar` can call an impure function or impure delegate is implicitly convertible to a pure one.
If you are opening an issue with an enhancement please provide a clear and concise testcase. Thanks.
Comment 4 Steven Schveighoffer 2015-07-08 18:22:05 UTC
(In reply to Denis Shelomovskij from comment #3)

> Do you mean this code should compile?

Yes.

> If so, either pure function `bar` can
> call an impure function or impure delegate is implicitly convertible to a
> pure one.

No, but I made an error in the example from what I meant.

should be:

auto foo(out int delegate() impureDg) pure {

...

impureDg = &(bar!());

> If you are opening an issue with an enhancement please provide a clear and
> concise testcase. Thanks.

It's hard to make a test case that "should compile" without making an error with no compiler to help you :D

Really, the issue is that bar doesn't compile, not that the impure delegate can't be assigned.
Comment 5 Denis Shelomovskii 2015-07-08 18:58:55 UTC
(In reply to Steven Schveighoffer from comment #4)
> (In reply to Denis Shelomovskij from comment #3)
> 
> > Do you mean this code should compile?
> 
> Yes.
> 
> > If so, either pure function `bar` can
> > call an impure function or impure delegate is implicitly convertible to a
> > pure one.
> 
> No, but I made an error in the example from what I meant.
> 
> should be:
> 
> auto foo(out int delegate() impureDg) pure {
> 
> ...
> 
> impureDg = &(bar!());
> 
> > If you are opening an issue with an enhancement please provide a clear and
> > concise testcase. Thanks.
> 
> It's hard to make a test case that "should compile" without making an error
> with no compiler to help you :D
> 
> Really, the issue is that bar doesn't compile, not that the impure delegate
> can't be assigned.

Then this function isn't strongly pure anymore:
---
T f() pure;
---
if `T` is `int delegate()`.

Is everybody OK with this?
Comment 6 ag0aep6g 2015-07-08 19:33:23 UTC
(In reply to Denis Shelomovskij from comment #5)
> Then this function isn't strongly pure anymore:
> ---
> T f() pure;
> ---
> if `T` is `int delegate()`.
> 
> Is everybody OK with this?

I think that's already not strongly pure. `int delegate()` has a mutable indirection in the context pointer. So a function that returns such a delegate can't be strongly pure.

See http://klickverbot.at/blog/2012/05/purity-in-d/#indirections_in_the_return_type

An example with the delegate type:
----
struct S
{
    int x = 1;
    int method() {return x++;}
}

int delegate() f() pure
{
    return &(new S).method;
}

void main()
{
   auto dg1 = f();
   version(all) auto dg2 = f();
   else auto dg2 = dg1; /* This would be equivalent to the above if f were strongly pure. */
   
   import std.stdio;
   writeln(dg1()); /* "1" */
   writeln(dg1()); /* "2" */
   writeln(dg2()); /* "1" or "3", depending on the version above */
}
----
Comment 7 Kenji Hara 2015-08-28 03:48:43 UTC
I change the importance to regression, because the introduced behavior since 2.067 rejects valid code.
Comment 8 Kenji Hara 2015-08-28 04:17:02 UTC
New PR targeting 2.068.1 release:
https://github.com/D-Programming-Language/dmd/pull/4970
Comment 9 github-bugzilla 2015-08-30 15:16:49 UTC
Commits pushed to stable at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/922d79a43ff60ed324e32e2f4be1430de209746f
fix Issue 14781 - impure delegate to pure function context should be able to modify context

https://github.com/D-Programming-Language/dmd/commit/b30556bf2ce1b4e49fdba51ea759725c2fc6e48f
Merge pull request #4970 from 9rnsr/fix14781

[REG2.067/2.068] Issue 14781 & 14962 - fix problematic purity inference introduced in #3626
Comment 10 github-bugzilla 2015-09-02 04:10:18 UTC
Commits pushed to master at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/922d79a43ff60ed324e32e2f4be1430de209746f
fix Issue 14781 - impure delegate to pure function context should be able to modify context

https://github.com/D-Programming-Language/dmd/commit/b30556bf2ce1b4e49fdba51ea759725c2fc6e48f
Merge pull request #4970 from 9rnsr/fix14781