Jump to page: 1 26  
Page
Thread overview
January 14
This is the feedback thread for the first round of Community Review for DIP 1029, "Add throw as Function Attribute":

https://github.com/dlang/DIPs/blob/8c48c98a0495f73db9a2d5c4aef502b9febe9673/DIPs/DIP1029.md

All review-related feedback on and discussion of the DIP should occur in this thread. The review period will end at 11:59 PM ET on January 28, or when I make a post declaring it complete.

At the end of Round 1, if further review is deemed necessary, the DIP will be scheduled for another round of Community Review. Otherwise, it will be queued for the Final Review and Formal Assessment.

Anyone intending to post feedback in this thread is expected to be familiar with the reviewer guidelines:

https://github.com/dlang/DIPs/blob/master/docs/guidelines-reviewers.md

*Please stay on topic!*

Thanks in advance to all who participate.

#DIP1029
#throw
January 14
On Tuesday, 14 January 2020 at 10:44:17 UTC, Mike Parker wrote:
> This is the feedback thread for the first round of Community Review for DIP 1029, "Add throw as Function Attribute":
>
> https://github.com/dlang/DIPs/blob/8c48c98a0495f73db9a2d5c4aef502b9febe9673/DIPs/DIP1029.md
>
> All review-related feedback on and discussion of the DIP should occur in this thread. The review period will end at 11:59 PM ET on January 28, or when I make a post declaring it complete.
>
> At the end of Round 1, if further review is deemed necessary, the DIP will be scheduled for another round of Community Review. Otherwise, it will be queued for the Final Review and Formal Assessment.
>
> Anyone intending to post feedback in this thread is expected to be familiar with the reviewer guidelines:
>
> https://github.com/dlang/DIPs/blob/master/docs/guidelines-reviewers.md
>
> *Please stay on topic!*
>
> Thanks in advance to all who participate.
>
> #DIP1029
> #throw

The idea is great, inverse attributes are very powerful in practice and are definitely very good in usability for a lot of code for example applying attributes to the entire D source file.

I don't think making the attribute "throw" is the best way to go though. Currently most attributes use the at-attribute syntax which makes them very unambiguous and easier to (visually) parse. I would suggest instead of having throw as attribute, the new attribute should be @throws instead. `throws` can also be argued for that it is "better" to read like: "function foo throws" than "function foo throw"

Additionally if we make this an at-attribute, it can very easily be extended in the future to have arguments what kind of exceptions are being thrown by this function, for example using template argument syntax to be the easiest and most consistent to parse. This also exists in other languages like Java and helps both with linting for try-catch, but also extremely helps with documentation.

For symmetry with nothrow it might be worth looking into providing nothrow as @nothrow attribute instead too.
January 14
I agree with this change. The suggested solution will have very little or no impact on the current codebase while adding a useful feature.

I was originally for that we should use the "throws" keyword in order to make it more searchable. However, now when I see the solution and that "nothrow" will still remain I think that "throw" is better for symmetry with "nothrow".

Correct me if I'm wrong, what I've understood in the text is that making "nothrow" the default is also a goal which is not covered in this DIP but perhaps in a future DIP.


January 14
On Tuesday, 14 January 2020 at 10:44:17 UTC, Mike Parker wrote:
>
> https://github.com/dlang/DIPs/blob/master/docs/guidelines-reviewers.md
>
> *Please stay on topic!*

This is good.


January 14
This is good. I like how Walter defined the overriding enclosing scope rule as well, and the rationale about aggregates inheriting it is also correct and I like that.

Unambiguous yes from me to this particular step and DIP.
January 14
On Tuesday, 14 January 2020 at 11:53:17 UTC, IGotD- wrote:
> I was originally for that we should use the "throws" keyword in order to make it more searchable. However, now when I see the solution and that "nothrow" will still remain I think that "throw" is better for symmetry with "nothrow".

On that specific note, perhaps the DIP author can include some code search strategies to differentiate between (i) functions marked `throw` and (ii) lines of code where an exception is thrown ... ?

It's minor, but it would be a nice consideration to folks whose search rules-of-thumb might be thrown out of whack by this change.
January 14
I really don't like how small these DIPs are. It seems they don't have direction on their own.

This is given as part of the rationale:

    void bar(); // may throw

    struct S1 {
        nothrow void foo() { bar(); } // error, bar() throws
    }

    nothrow {
        struct S2 {
            void foo() { bar(); } // no error, as the nothrow block does not apply
        }
    }


Why isn't this being fixed so that `nothrow{}` then applies to `S2.foo()`? Why is it even part of the rationale if there's no intention to fix this?

If you are going to add `throw` why aren't the rest of the attributes getting inverses? Pure? @nogc? etc...

> Although this DIP does not propose making exceptions opt-in, the throw attribute is a key requirement for it.

It seems this DIP's sole purpose is as a stepping stone for the next DIP to change the default of nothrow to be the default. On its' own it doesn't make sense, if your are trying to fix the disparity of being able to turn off attributes there's no reason to not include the rest of the attributes as well.

Otherwise this shouldn't be it's own DIP. If the intention is this is only required for another DIP, this should just be included in that DIP. If the intention is the fix the problem that attributes don't have inverses to be able to turn them on/off, then I don't see why you need a separate DIP for each and every attribute. Include them all in the same DIP as well as address the strange behavior with structs/classes that was included as part of the rationale.

If this is really just a stepping-stone DIP, what if the next DIP is rejected, but this one is accepted? What purpose would it have served? Just adding `throw` isn't a significant/meaningful enough change on it's own, if you look solely at this DIP.
January 14
On Tuesday, 14 January 2020 at 15:18:41 UTC, Arine wrote:
> If you are going to add `throw` why aren't the rest of the attributes getting inverses? Pure? @nogc? etc...

Because those changes are not contingent on one another.  One can make this change without impacting any of the other attributes.

Where changes don't _need_ to be coupled, let's make the case for each of them on their own merits.  It makes for a much simpler change management process.

(Or if you really think it's worthwhile in its own right, write up your own DIP for a comprehensive set of invertible attributes.)
January 14
On Tuesday, 14 January 2020 at 15:18:41 UTC, Arine wrote:
> If this is really just a stepping-stone DIP, what if the next DIP is rejected, but this one is accepted?

This is still a positive step.. I think we have a bad habit of rejecting small good steps in favor of larger better steps that don't come to pass.

nothrow/throws itself is not amazing, but it would make it more likely to be used even by itself.
January 14
On Tuesday, 14 January 2020 at 15:28:29 UTC, Joseph Rushton Wakeling wrote:
> On Tuesday, 14 January 2020 at 15:18:41 UTC, Arine wrote:
>> If you are going to add `throw` why aren't the rest of the attributes getting inverses? Pure? @nogc? etc...
>
> Because those changes are not contingent on one another.  One can make this change without impacting any of the other attributes.
>
> Where changes don't _need_ to be coupled, let's make the case for each of them on their own merits.  It makes for a much simpler change management process.

I disagree. That's how your language turns into C++. The tough part of language design is making decisions conditional on what the language as a whole looks like, and what it's going to be in ten years.

My opinion is that it's an ugly inconsistency to give only one attribute an inverse. Maybe others don't agree, but lately there's been an attempt to stifle discussion on major changes. That's fine, but then get rid of the DIP process, as it doesn't serve a purpose. Walter can push changes to master and then there can be announcement that the change has been made. No point pretending there's a process in place if you're not going to allow relevant discussion.

> (Or if you really think it's worthwhile in its own right, write up your own DIP for a comprehensive set of invertible attributes.)

This is a terrible idea. You don't need a DIP to counter a DIP. The DIP author has to write a document that can be defended. The ultimate goal is to make the right decision, not "best DIP wins".
« First   ‹ Prev
1 2 3 4 5 6