Issue 9148 - 'pure' is broken

[9rnsr]

https://issues.dlang.org/show_bug.cgi?id=9148

A nested function inside pure function should also be typed as pure, and considered as weak purity function.

[tgehr]

Why should this pass?

[9rnsr]

foo1 is implicitly marked as pure, so accessing global data g will be disallowed.

[tgehr]

This is new behaviour introduced by this pull request, right? I guess this makes some sense, but it is a breaking language change. Also, I'd prefer to just have attribute inference for local functions instead because that is more general and wouldn't be a possible cause of annoyance.

[9rnsr]

This is not new behavior. In 2.065 and git-head, foo1() is not explicitly typed as pure but its purity violation is correctly checked by Expression::checkPurity.

void test9148a() pure
{
    static int g;
    void foo1() { ++g; } // pure function 'foo1' cannot access mutable static data 'g'
}

[tgehr]

I think this is broken behaviour. I would suggest to indeed stop incorrectly reporting the purity violation, but this can break code unless this is taken care of by e.g. adding attribute inference for local functions. I guess it is ok to merge this as is for now, because it can be fixed later.

[tgehr]

@9rnsr: That example does not show broken behaviour. The return type of test() would be int delegate()[]. Then the calls f[0]() and f[1]() in main are impure and the behaviour is not at all surprising. I am just saying that banning ++g categorically is not the right thing to do here. It is perfectly valid for a pure function to compute a result that contains impure delegates. It just may not call those itself. Purity for nested functions should be inferred based on the nested function body and not based on whether the enclosing function is pure. However, I am not opposed to merging this pull request, this restriction can be lifted later. I'll file some issues as soon as this is merged. (Another thing that is still missing is inference of the 'immutable' attribute.) The pull fixes the main concerns of issue 9148 though. Thanks!

@jmdavis: There is no reason to infer purity if the function is not actually pure and this is just going to cause a compile time error.

[jmdavis]

There is no reason to infer purity if the function is not actually pure and this is just going to cause a compile time error.

Well, foo1 is within a pure function, so inferring purity makes some sense. What doesn't make sense IMHO is that the outer function be legally pure given that it has a mutable static variable in it. But assume for a moment that foo1 is trying to access a global variable rather than a local static which shouldn't even be legal. If that were the case, then the outer function is properly pure, and it then becomes a question of whether it makes more sense for foo1 to be inferred as pure (resulting in an error if it's not), or if it makes more sense for foo1 to be inferred as impure and then be an error for being inside of a pure function. I'm not sure that it matters much which (though given what's going on with the outer function having a mutable static variable in it that it doesn't use, I suspect that the compiler would unfortunately only complain about an impure foo1 being called rather than it being declared in the first place).

[tgehr]

The scope of a static variable is completely unimportant. As long as a pure function does not access a mutable static variable, then everything is fine. Also, besides being called, an impure nested function can be passed around as a value. The following code is fine:

int delegate() getCounterIncrementor()pure{
    static int counter = 0;
    return ()=>counter++;
}

getCounterIncrementor does not access any mutable static variables nor does it call any functions that are not pure, hence it can be pure. The following must be (and is already) illegal though:

int foo()pure{
    return getCounterIncrementor()();
}

I.e. there are already enough type checks to make sure that static variables are never accessed within a pure function. Computing using impure functions is not impure as long as those functions are not called. A function that composes two non-pure functions, for example, is still a pure function.

[9rnsr]

The return type of test() would be int delegate()[]. Then the calls f0 and f1 in main are impure and the behaviour is not at all surprising.

By returning a tuple instead of an array, you can see the behavior that pure delegate call f[0]() will be violated by the impure f[1]() call.

Ok @tghr, now I'm probably understanding what you saying. With simple case, declaring impure nested function inside pure function would be possible.
But I'm really not sure that is possible with complex cases (and useful or not). So today, I cannot say anything more than that.

[tgehr]

I do not understand these concerns at all. f[0]() is a weakly pure call. There simply is no possible issue here.

[yebblies]

So should I reopen #2790 ?

[9rnsr]

@yebblies No, by fixing 9148, the error message issue will also be naturally fixed.

I added diagnostic test case for 9620.

[yebblies]

No, by fixing 9148, the error message issue will also be naturally fixed.

Even better!

[9rnsr]

I also fixed a lambda attribute inference case (test9148e() in compilable/testInference.d).
For that, now phobos fix is necessary: dlang/phobos#2493

[9rnsr]

PR is re-enableb by the Phobos tweaking.

[denis-sh]

What happened here? Was it finally accepted or just accidentally merged with #3956?

[denis-sh]

@9rnsr, sorry for bothering but could you confirm this is really accepted?

[9rnsr]

I don't know about any discussion and approval over this PR so I had kept a distance from dmd development for a while.

[denis-sh]

Thanks.

@WalterBright, could you comment on this pull state please?