Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DIP: Add throw as Function Attribute #167

Merged
merged 1 commit into from
Jan 14, 2020
Merged

Conversation

WalterBright
Copy link
Member

No description provided.

@wilzbach
Copy link
Member

What about the inverse of nogc and pure?

@dkorpel
Copy link
Contributor

dkorpel commented Sep 15, 2019

What about the inverse of nogc and pure?

This is important to consider. While I like throw in this case, pure isn't fortunate enough to have an opposite keyword for it. If we end up going with e.g. pure(false) or !pure, then you'll end up with either inconsistently not allowing nothrow(false) or !nothrow, or multiple ways to specify a throw function.

@WalterBright
Copy link
Member Author

@wilzbach I haven't decided how to do that. But with throw the solution is straightforward.

@WalterBright WalterBright changed the title Add throw as Function Attribute DIP: Add throw as Function Attribute Sep 16, 2019
@dkorpel
Copy link
Contributor

dkorpel commented Jan 4, 2020

The DIP doesn't specify whether "throw" will be included in __traits(getFunctionAttributes, func).

It also doesn't specify the grammar changes.

any change that requires adjusting the language grammar must include a description of the change in the same format as the existing grammar spec.

https://github.com/dlang/DIPs/blob/master/docs/guidelines-authors.md#technical-details

@PetarKirov
Copy link
Member

PetarKirov commented Jan 4, 2020

I believe that this solution is the wrong way to go as it doesn't scale or integrate well with D's metaprogramming abilities. There's no way toggling an attribute on or off depending on a template parameter, short of using mixins or repeating the whole function definition.

The right way to go is to add compiler recognized UDAs (like we already did for extern (Objective-C) selector UDA, or LDC's @ldc.attributes.*) alternatives to @safe, pure, etc. That way they can be easily manipulated via functions that operate on AliasSeqs.

@WalterBright
Copy link
Member Author

The DIP doesn't specify whether "throw" will be included in __traits(getFunctionAttributes, func).

No need to, since it is a function attribute it is implicitly included.

It also doesn't specify the grammar changes.

The first sentence after "Description".

@dkorpel
Copy link
Contributor

dkorpel commented Jan 8, 2020

The first sentence after "Description".

It wasn't clear to me that that specifies the grammar change, since the DIP doesn't mention the word 'grammar' once, and it links to:
https://dlang.org/spec/attribute.html#Attribute

Instead of:
https://dlang.org/spec/grammar.html#Attribute

Also if I understand correctly, that change only allows throw to appear before the function name, and not after it, since you didn't add it to 'FunctionAttribute':
https://dlang.org/spec/grammar.html#FunctionAttribute

Usually DIPs have an explicit section of grammar changes with a markdown diff of the grammar before and after the DIP. I suggesting doing this too.

No need to, since it is a function attribute it is implicitly included.

A DIP should be explicit, such that (like Andrei put it) even a vengeful ex would implement it correctly. The DIP doesn't say it's a "function attribute", just that it's an attribute that only has an effect on functions. I don't think the spec defines what a "function attribute" is, the part about getFunctionAttribute mentions an explicit list of supported attributes, which (obviously) excludes throw currently.

It's unspecified whether throw is added to the function attributes by default like @system currently is.

void t0() nothrow; // tuple("nothrow", "@system")
void t1() throw; // tuple("throw", "@system")
void t2(); // tuple("@system") or tuple("throw", "@system")?

@WalterBright
Copy link
Member Author

I added it to FunctionAttribute. I had overlooked that.

The DIP doesn't say it's a "function attribute"

Adding it to FunctionAttribute seems clear enough.

I don't think the spec defines what a "function attribute" is

The spec could certainly be improved, but that's a separate issue.

@mdparker mdparker merged commit fc72b92 into dlang:master Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants