dmd
dmd copied to clipboard
Implement DIP1029: Add `throw` as Function Attribute
https://github.com/dlang/DIPs/blob/master/DIPs/accepted/DIP1029.md
Signed-off-by: Luís Ferreira [email protected]
- [x] Specification PR (https://github.com/dlang/dlang.org/pull/3082)
- [x] DMD Implementation
- [x] Tests
- [x] Header generation of
throw
- [x] Templated functions: do they infer default or
throw
? how does that interact with toplevelthrow
/nothrow
? - [ ] throw virtual functions that are overriden with
nothrow
- [x] more cases for
throw:
/nothrow:
interacting with function/template declarations - [x] Add tests with
__traits(getFunctionAttributes, func)
- [ ] Cast
nothrow
tothrow
- [ ] Check for forbidden cast of
throw
tonothrow
- [x] Header generation of
CC: @WalterBright
Thanks for your pull request, @ljmf00!
Bugzilla references
Your PR doesn't reference any Bugzilla issue.
If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.
Testing this PR locally
If you don't have a local development environment setup, you can use Digger to test this PR:
dub run digger -- build "master + dmd#12934"
Buildkite seems to fail due to some side effects, not related to this PR. Although, need to write some more tests to cover all this DIP. For now, I would appreciate a review of my implementation approach. This needs an autosquash rebase on my side to get rid of fixup
commits. My initial approach doesn't work as it introduces a breaking change and it also produces a wrong behaviour. The fixup creates a "default" behaviour which is not having either throw
or nothrow
, similar to @safe
, @trusted
and @system
approaches, where no attribute means @system
.
This still requires some tests for:
- header generation of throw
- templated functions: do they infer default or throw? how does that interact with toplevel throw/nothrow?
- throw virtual functions that are overriden with nothrow
- more cases for
throw:
/nothrow:
interacting with function/template declarations
Thanks for the list :+1: . I'm going to do that.
The DIP does not specify whether inference should deduce throw or default for functions that are not nothrow. I would argue that a function has to be throw or nothrow; default is just an intermediary state where the compiler does not know but should assume throw. As a precedent, in the case of safety,
@system
is specifically deduced when inference occurs. In this implementation there seems to be a gap between throw and default. Looking attest/compilable/dip1029.d
line 41 it seems that inference does not deduce throw, but default. This is important for things like header generation, error messages, typeof etc.
I choose to use a default mechanism to prevent breaking changes.
I thought that this was not specified in the DIP because it is implicitly implied by "no breaking changes". It is important to test this behaviour because existing code might get the typeof
a throw
function without specifying any attribute and adding throw
attribute to the returned type may introduce a breaking change.
void foo() {}
void bar() @system {}
Doing a typeof
of foo
and bar
produces different output and if the user compares it with, for example, a .stringof, this may break it.
I also see this as acceptable because this is how the @system
attribute comparison works with the default value.
is(void function() == void function() @system) // true
AFAIK, the intermediate state where the compiler can't assume throw is validated by FUNCFLAG.nothrowInprocess
rather than THROW.default_
.
Tell me if I interpreted your comment/suggestion wrongly.
Doing a typeof of foo and bar produces different output and if the user compares it with, for example, a .stringof, this may break it.
In your example there is no inference. In that case, I agree that typeof should return whatever the explicit annotation is. However, in the case of delegates/lambdas it's not that obvious: these always have a body and their attributes are explicitly embedded in the type (the same as @system
).
Also, for template functions, I think that throw
should be explicitly added as it is the case for system
however that might break some code that uses .stringof the way you mentioned.
To summarize: should function declarations that have their attributes inferred (deletgates, lambdas, function templates) explicitly be marked as throw
or be left unmarked? On the long run, the compiler should explicitly annotate with throw
, however, this might break some code (when generating headers that contain delegates/lambdas, when checking the type of template instances via .stringof etc.)
cc @WalterBright
Doing a typeof of foo and bar produces different output and if the user compares it with, for example, a .stringof, this may break it.
In your example there is no inference. In that case, I agree that typeof should return whatever the explicit annotation is. However, in the case of delegates/lambdas it's not that obvious: these always have a body and their attributes are explicitly embedded in the type (the same as
@system
).Also, for template functions, I think that
throw
should be explicitly added as it is the case forsystem
however that might break some code that uses .stringof the way you mentioned.To summarize: should function declarations that have their attributes inferred (deletgates, lambdas, function templates) explicitly be marked as
throw
or be left unmarked? On the long run, the compiler should explicitly annotate withthrow
, however, this might break some code (when generating headers that contain delegates/lambdas, when checking the type of template instances via .stringof etc.)
I agree. We could add a -preview=dip1029
to express that behaviour. I'm not aware of the preview process, but I assume is the same as the deprecation process but inverted, right?
This code will still have breaking changes. __traits(getFunctionAttributes)
gives you @system
on a function without ~~parameters~~ attributes.
however that might break some code that uses .stringof the way you mentioned.
As far as I know the language spec makes no guarantees whatsoever about the specific contents of .stringof
, so anyone who relies on it in this way is doing so at their own risk.
Needs #13121 for trailing whitespace free tests.
#13121 has been merged.
#13121 has been merged.
Yes, but I still need to commit the header generation tests and fix the logic for it.
I added the rest of the test cases. I just need to add cast() and virtual functions.
This code will still have breaking changes.
__traits(getFunctionAttributes)
gives you@system
on a function without ~parameters~ attributes.
As I suspected, this fails to compile on Phobos. Should I add a flag to the compiler to make the transition or make __traits(getFunctionAttributes)
hide throw
on THROW.default_
? That will make it a special case tho. How can we proceed here @RazvanN7 ?
We have the following solutions:
- omit
throw
when it is not specifically added by the user. This would be the easiest way, however, it will be inconsistent with what happens in the case of@system/@safe
. - issue a deprecation whenever
getFunctionAttributes
is used on athrow
function that is not annotated as such by the user. - fix the breakages and go with it as is.
Either way we choose it's a bit nasty. I, personally, would be inclined to go the latter route since I expect that the way phobos uses getFunctionAttributes
is not a common occurrence. But, I think that there are folks who might not necessarily agree with me, so issuing a deprecation would be a safer alternative, however this is problematic because if you are reflecting on library code there is no way to get rid of the deprecation.
Another alternative would be to deprecate getFunctionAttributes
altogether for a newer getFuncAttrs
which outputs throw
, however, this is not scalable if we intend to implement impure
and gc
- Would this be a precursor for diagnostics akin to:
error: func() throws but is not caught
- Possibility of future extensions to allow specifying which exception is thrown, e.g:
int openFile() throw(FileException) { ... }
- Would this be a precursor for diagnostics akin to:
error: func() throws but is not caught
I don't think so, the default, which is the "hidden" throw
doesn't behave like that, why would with specified throw
? I would argue that makes sense if we specified the Exception class, explicitly, but this DIP doesn't cover that.
- Possibility of future extensions to allow specifying which exception is thrown, e.g:
int openFile() throw(FileException) { ... }
This DIP only creates a contrary attribute to negate nothrow
.
fix the breakages and go with it as is.
I can create an attempt to fix the breakage on Phobos, but that doesn't guarantee the fact that this will break user code, although the user should be prepared for those changes, as any other attribute might get introduced. To do this attempt, should I add a synchronization/transition flag to do that transition? If yes, what flag? My suggestion would be to stick with a -revert=
flag, that way users can revert that behaviour to the old one, preventing breakage and in the long term, deprecate that flag when we have impure
and @gc
?
Ping @RazvanN7 :)
The more I think about this, the more I feel that a deprecation is a more suitable alternative. We added a new function attribute, people need to update their code to take it into account.
The more I think about this, the more I feel that a deprecation is a more suitable alternative. We added a new function attribute, people need to update their code to take it into account.
I can agree with the deprecation if we implement this to the other attributes, I would say.
Yes, definitely, if other attributes will be implemented then the same deprecation path must be taken.
Yes, definitely, if other attributes will be implemented then the same deprecation path must be taken.
Ah, so you mean this?
- issue a deprecation whenever getFunctionAttributes is used on a throw function that is not annotated as such by the user.
I thought you were talking about implementing the new trait and deprecate the older one. But yeah, I share the same opinion, if that's what you mean.
Ah, so you mean this?
Yes.
Ah, so you mean this?
Yes.
Ok, I can go with implementing it that way. I think this is the best way among the other options we have.
ping @ljmf00
@ljmf00 any updates on this? This is so close to completion, it's a pity it got stalled so much.