Implement DIP1029: Add throw as Function Attribute

[ljmf00]

https://github.com/dlang/DIPs/blob/master/DIPs/accepted/DIP1029.md

Signed-off-by: Luís Ferreira contact@lsferreira.net

---

• Specification PR (spec: DIP1029: Add throw as Function Attribute dlang.org#3082)
• DMD Implementation
• Tests
  • 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
  • Add tests with __traits(getFunctionAttributes, func)
  • Cast nothrow to throw
  • Check for forbidden cast of throw to nothrow

CC: @WalterBright

[dlang-bot]

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"

[ljmf00]

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.

[RazvanN7]

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

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 at test/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.

[RazvanN7]

It seems that this line is not covered by tests.

[RazvanN7]

This if and the following one could be put in the same block, i.e.:

if (tf.throw_ != THROW.default_)
{
    sc.stc &= ~STC.throwGroup;
    if (tf.throw == THROW.nothrow_)
           ...
    if (tf.throw == THROW.throw)
        ...
}

[ljmf00]

Done.

[RazvanN7]

I think that this line is unnecessary. this.throw_ should be 0 by default.

[ljmf00]

Done. Going to do that for trust too.

[RazvanN7]

Suggested change

	if (stc & STC.throw_)
	else if (stc & STC.throw_)

[ljmf00]

Done

[RazvanN7]

I don't think this deprecation is necessary.

[ljmf00]

I thought that in case someone uses dmd as a library. I hear that was a thing, so I didn't remove it all to prevent breaking user code, although I don't know its state.

[RazvanN7]

Technically, it could be used, but I doubt that. I guess there's no harm in using deprecatins here.

[ljmf00]

Removed.

[RazvanN7]

Is this right? Shouldn't it be return throw == THROW.nothrow ? Isn't the default throw ?

[ljmf00]

You are right. Need to add a test case for this, if isnothrow is kept. The compiler itself no longer uses isnothrow function. It is only to give backwards compatibility and prevent breaking code, as I described above.

[ljmf00]

Removed.

[RazvanN7]

Note that the isnothrow function is not covered by tests.

[ljmf00]

The compiler itself is no longer using it.

[ljmf00]

Removed.

[RazvanN7]

Lines 4372-4374 Not covered by tests.

[ljmf00]

I still need to check which section this code belongs. I arranged the code based on existing attributes. I will try to write some more tests to cover all these warnings.

[RazvanN7]

why is this not inferred as throw wheres the subsequent one is?

[ljmf00]

This is inferred as throw, void function() pure throw @safe is the same as void function() pure @safe.

[ljmf00]

I added a check on line 45 to prove it.

[ljmf00]

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 👍 . 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 at test/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.

[RazvanN7]

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

[ljmf00]

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.)

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?

[ljmf00]

This code will still have breaking changes. __traits(getFunctionAttributes) gives you @system on a function without parameters attributes.

[pbackus]

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.

[ljmf00]

Needs #13121 for trailing whitespace free tests.

[thewilsonator]

#13121 has been merged.

[ljmf00]

#13121 has been merged.

Yes, but I still need to commit the header generation tests and fix the logic for it.

[ljmf00]

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 ?

[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 a throw 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.

[RazvanN7]

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

[ibuclaw]

1. Would this be a precursor for diagnostics akin to: error: func() throws but is not caught
2. Possibility of future extensions to allow specifying which exception is thrown, e.g: int openFile() throw(FileException) { ... }

[ljmf00]

1. 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.

2. 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.

[ljmf00]

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?

[ljmf00]

Ping @RazvanN7 :)

[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.

[ljmf00]

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.

[RazvanN7]

Yes, definitely, if other attributes will be implemented then the same deprecation path must be taken.

[ljmf00]

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.

[RazvanN7]

Ah, so you mean this?

Yes.

[ljmf00]

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.

[RazvanN7]

ping @ljmf00

[RazvanN7]

@ljmf00 any updates on this? This is so close to completion, it's a pity it got stalled so much.