Issue 12622 - Purity, @safe not checked for pointers to functions

[9rnsr]

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

[9rnsr]

In order to invoke nothrow check later, don't return ErrorExp.

[WalterBright]

Why invoke nothrow check later? Once there's an error detected, no need for further checking. And errors need to be propagated, not ignored.

[9rnsr]

Because of the consistency.

Currently, for direct function calls, the failure of checkPurity and checkSafety is not translated to ErrorExp.

        if (f)
        {
            checkPurity(sc, f);   // purity check failure is ignored
            checkSafety(sc, f);  // safety check failure is ignored
            f->checkNestedReference(sc, loc);
        }

Because of that, nothrow attribute violation is properly checked later.

I think that function pointer calls should follow the behavior.

[WalterBright]

I believe it is a bug that the purity/safety checks ignore the error rather than propagate it. Those errors are not warnings or suggestions, they need to be propagated. I want to get to the point where we can remove the global.errors checks.

[9rnsr]

I also think propagating error would be better. But, current nothrow check is separated from purity/safety check, and if we return ErrorExp against purity/safety violation, it will hide nothrow error.

fail12622.d(23): Error: pure function 'fail12622.foo' cannot call impure function pointer 'fp'
fail12622.d(23): Error: safe function 'fail12622.foo' cannot call system function pointer 'fp'
fail12622.d(25): Error: pure function 'fail12622.foo' cannot call impure function pointer 'fp'
fail12622.d(25): Error: safe function 'fail12622.foo' cannot call system function pointer 'fp'
fail12622.d(27): Error: pure function 'fail12622.foo' cannot call impure function 'fail12622.bar'
fail12622.d(27): Error: safe function 'fail12622.foo' cannot call system function 'fail12622.bar'
fail12622.d(23): Error: 'fp' is not nothrow                                 // These errors will disappear
fail12622.d(25): Error: 'fp' is not nothrow                                 // if we return ErrorExp
fail12622.d(27): Error: 'fail12622.bar' is not nothrow                      // against purity or safety
fail12622.d(19): Error: function 'fail12622.foo' is nothrow yet may throw   // violations.

I'd keep this code, because it would generate better diagnostic. And we can improve nothrow check mechanism in the future.

[WalterBright]

Then I suggest combining the safety/purity/nothrow checks so they all done together, rather than not propagating the error.

[9rnsr]

Yes, we should do it. But we would need more work to do it.

[dnadlinger]

I'm not sure whether a debatable, slight improvement in diagnostics is worth compromising the internal structure of the frontend.

(Debatable, as unfortunately still quite often only the first error message is reliable, especially when compiling multiple modules at once. If anything, we should try to make error handling more methodical at this point, even if some special (test) cases possibly suffer a slight reduction in diagnostics quality.)

[9rnsr]

Internal structure improvement is still work in progress - more much work is necessary. So this trivial compromising won't become serious.

On the other hand, the small diagnostic improvement can be fixed with the test case. We can improve internals without downgrade the compiler output.

I want to get the small, but reliable improvement.

[9rnsr]

OK... I gave up the better diagnostic. It should be fixed later.

[WalterBright]

That was quick!

[9rnsr]

Rebased onto the new @nogc feature.

[dnadlinger]

Auto-merge toggled on

[9rnsr]

Thanks.