Bug 22221 – [dip1000] pure function can escape parameters through Exception

Follow up to issue 20150. The fix for that issue prevented escaping through the return value, but had the known limitation that even when escaping through parameters or the return value is not possible, parameters of pure functions can still be put into an Exception and thrown: ``` void f_throw(string x) @safe pure { throw new Exception(x); } void escape_throw_20150() @safe { immutable(char)[4] str; f_throw(str[]); } ``` If the function isn't `nothrow`, the pure function -> scope parameters implication shouldn't be done.

@dkorpel created dlang/dmd pull request #12989 "Fix issue 22221 - pure function can escape parameters through Exception" fixing this issue: - fix issue 22221 - pure function can escape parameters through Exception https://github.com/dlang/dmd/pull/12989

The problem here comes from the compiler automatically inferring `scope` for the parameter `x` because it's a pure function, assuming that `x` cannot escape it because `pure` disallows storing things in global variables. The trouble is that escaping via a thrown exception was not taken into account.

Atila and I discussed this, and we think that it should just be an error for a pure function to throw its parameters. The rationale is too much existing code relies on the scope inference, and there is (likely) very little that tries to throw its parameters. Therefore, this fix will be both correct and break the least amount of code. For code that does break, the solution is to make a copy of the parameter, and throw the copy. This is workable since scope is not transitive.

(In reply to Walter Bright from comment #3) > Therefore, this fix will be both correct and break the least amount of code. I'm not sure, what's left blocking the fix PR is 1 function in excel-D (shouldEqual) and some Phobos unittests. It's unknown how much breakage results from your proposal. > For code that does break, the solution is to make a copy of the parameter, > and throw the copy. This is workable since scope is not transitive. How would you explain that special case to D users? Let's consider the most obvious example: ``` void enforce(bool x, string msg) @safe pure { if (!x) throw new Exception(msg); } ``` The compiler will ask you to make a copy of the `msg` string for no good reason. Now consider this variant that returns the first argument (like in Phobos): ``` int* enforce(int* x, string msg) @safe pure { if (!x) throw new Exception(msg); return x; } ``` Because the return value has a pointer now, `msg` will not infer `scope` from `pure` anyway, so the compiler either: - allows it, meaning that changing the return type causes a weird unrelated error about throwing `msg` - still give an error, meaning you have to make a copy of `msg`, and still cannot assign a `scope` string to the parameter! Finally, consider giving an auto return type: ``` auto enforce(bool x, string msg) { if (!x) throw new Exception(msg); } ``` How are the attributes inferred? Possibilities are: - impure @system - impure @safe - pure @system Will this pick the `@safe` one? What if I explicitly specify `pure`, will it infer `@system` then? This looks like a special case that has a ripple effect of complexity, I can't see this being a good idea.

> I'm not sure, what's left blocking the fix PR is 1 function in excel-D (shouldEqual) and some Phobos unittests. It's unknown how much breakage results from your proposal. Are you sure about that? I checked out your branch some weeks ago and started adding scope to a *lot* of Phobos functions before I gave up. I think the real issue is illustrated by the fact that this correctly doesn't compile: ----- void f_throw(scope /*explicit scope here*/ string x) @safe pure { throw new Exception(x); } ----- In the original example, the compiler is looking at the signature of the function and the fact that it's strongly pure and adding scope to the parameter when it's being called from `escape_throw_20150`, but not when compiling the function itself! I don't think we can consider it to be scope in one context but not in the other. My proposal is that we decide to implicitly add scope to the parameters of strongly pure functions. The escape hatch would be to add a dummy parameter with mutable indirection to make it weakly pure. This is ugly but it's for a niche use-case and other than introducing a keyword or some way to mark the function as "pure but don't scope my params" I'm not sure what to do.

(In reply to Atila Neves from comment #5) > Are you sure about that? I checked out your branch some weeks ago and > started adding scope to a *lot* of Phobos functions before I gave up. Did you check out the latest Phobos? As of Dec 24, 2021 (https://github.com/dlang/phobos/pull/8214) the non-unittest build oh Phobos compiles. > I don't think we can consider it to be scope in one context but not in the other. It's not that the language defines a `pure` function to have `scope` parameters, it's a shortcut. The compiler thinks "`scope` inference failed, but it's a `pure` function and can't return its parameters, so I'll let you pass `scope` variables to it anyway". It's similar to pure factory functions returning a mutable type but still allowing `@safe` conversion to immutable. That also doesn't mean the language considers `pure` functions to always return `immutable` data. > My proposal is that we decide to implicitly add scope to the parameters of > strongly pure functions. An attribute should not introduce unrelated restrictions, that makes attribute inference intractable like I mentioned in my previous reply. How are you deciding between `pure` and `@safe` when you can only pick one, because `pure` would invalidate `@safe` and vice versa? > The escape hatch would be to add a dummy parameter > with mutable indirection to make it weakly pure. How are you going to explain to D users why `enforce` now has an extra `null` argument? "Okay, so the function would be strongly pure, and we decided that this makes the parameters scope. Why? Well, because our scope inference is very weak, so we made this shortcut using pure, which turns out doesn't work, but we wanted to keep it anyway, so we added this weird language rule so it kind of works again, but this now requires a `null` parameter here to break the shortcut". I think fixing issue 20674 will work out better. > This is ugly but it's for a niche use-case and other than introducing a > keyword or some way to mark the function as "pure but don't scope my params" If you want to mark the function as "pure but don't scope my params", then mark it `pure` and add `scope` to the parameters.

> The compiler thinks "`scope` inference failed On a non templated function? > How are you going to explain to D users why `enforce` now has an extra `null` argument? With a comment on enforce. It's not ideal, but I think it's better than the current situation. >> This is ugly but it's for a niche use-case and other than introducing a >> keyword or some way to mark the function as "pure but don't scope my params" > If you want to mark the function as "pure but don't scope my params", then mark it > `pure` and add `scope` to the parameters. That doesn't make any sense to me. What we want is to rely on inference as much as possible instead of explicit `scope` on parameters.

(In reply to Atila Neves from comment #7) > > The compiler thinks "`scope` inference failed > > On a non templated function? I assume a function with attribute inference here, but it's the same without inference. The point is: the language doesn't define `pure` to imply `scope` in any way, neither is this expected behavior. > With a comment on enforce. It's not ideal, but I think it's better than the > current situation. What about attribute inference becoming intractable like I mentioned? > > If you want to mark the function as "pure but don't scope my params", then mark it > `pure` and add `scope` to the parameters. > > That doesn't make any sense to me. What we want is to rely on inference as > much as possible instead of explicit `scope` on parameters. I meant "mark it `pure` and *don't* add `scope` parameters". The point here is: non-scope is the default, there's no need for a `nonscope` keyword or workaround when you don't introduce a special case where `scope` becomes the default.

dlang/dmd pull request #12989 "Fix issue 22221 - pure function can escape parameters through Exception" was merged into master: - 3439511d7df3d6c85c57b878406d170be7bdaef3 by dkorpel: fix issue 22221 - pure function can escape parameters through Exception https://github.com/dlang/dmd/pull/12989