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

RFC: Reserve throw and fail as keywords in edition 2018 #2441

Closed
wants to merge 3 commits into from

Conversation

Centril
Copy link
Contributor

@Centril Centril commented May 14, 2018

🖼️ Rendered

📝 Summary

Reserve throw and fail in edition 2018.

🎵 Note

This RFC separates out the urgent question of keyword reservation from RFC #2426.
It does not prescribe actually adding a fail expr or throw expr construct to the language.

@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the RFC. label May 14, 2018
@Centril Centril mentioned this pull request May 14, 2018
@scottmcm
Copy link
Member

scottmcm commented May 14, 2018

👍

Thanks for posting this, @Centril. The keyword reservation is definitely the critical part right now, and the overall cost of doing so is tiny, since it can be unreserved against at any time.

While I'm personally in favour of some feature in this space, I'm also perfectly happy to set aside the whole discussion of whether it's needed or what its semantics should be until after the edition.

@ExpHP
Copy link

ExpHP commented May 14, 2018

These keyword reservations are starting to get frighteningly close to home.

$ rg '^[^/]*[^\[/]\bfail\b'
src/tasks/config/lib.rs
329:    #[serde(default = "self::defaults::ev_loop::fail")]
330:    pub fail: bool,
380:        pub(crate) fn fail() -> bool { true }

src/tasks/cmd/relaxation.rs
193:                    if self.config.fail {
415:    fail: f64,
423:            sc_token.deconstruct(fail, supercell)?

(I'm surprised I don't have any closures named fail).

Oh, about the [^\[/] in that regex... it's a good thing failure hasn't made its 1.0 release yet. (cc @withoutboats)

@Centril
Copy link
Contributor Author

Centril commented May 14, 2018

it's a good thing failure hasn't made its 1.0 release yet. (cc @withoutboats)

I double checked failure now and it doesn't use fail as an identifier anywhere :)

@ExpHP
Copy link

ExpHP commented May 14, 2018

@Centril #[fail]

Hmm.... then again, #[cfg(if)] is allowed even though that is lexically an identifier. Maybe keywords as attribute names should just be allowed (currently you get expected identifier, found keyword).

(Notably, failure does not actually export an attribute named fail, since those aren't stable yet. It is derive_Fail that actually parses and interprets the attribute, removing it from the output token stream. But rustc does some amount of parsing first to find derives and validate the struct, which I imagine would fail on this silly edge case unless fixed)

@Centril
Copy link
Contributor Author

Centril commented May 14, 2018

@ExpHP Hmm... are attributes affected by keywords?

@ExpHP
Copy link

ExpHP commented May 14, 2018

Okay, so I did a proper test of this.

#[macro_use] extern crate serde_derive;
extern crate serde;

#[derive(Serialize)]
#[if]
struct Foo {
    #[serde(rename_all = "lolol")]
    a: i32,
}

I expect one of two outcomes:

  • If rustc tries too eagerly to parse the attribute name as a path and rejects keywords, this will fail before calling the serde derive.
  • Otherwise, it will call the serde derive, and THAT should fail
   Compiling playground v0.0.1 (file:///playground)
error: expected identifier, found keyword `if`
 --> src/main.rs:5:3
  |
5 | #[if]
  |   ^^ expected identifier, found keyword

error: proc-macro derive panicked
 --> src/main.rs:4:10
  |
4 | #[derive(Serialize)]
  |          ^^^^^^^^^
  |
  = help: message: called `Result::unwrap()` on an `Err` value: ParseError(Some("failed to parse derive input: failed to parse"))

error: aborting due to 2 previous errors

Er, they both failed? ...okay, that's not what I expected to see, but due to the first error I'd say this falls under the first bullet. My guess is keywords are currently forbidden here to simplify proc_macro path resolution, but that this limitation might be more or less arbitrary since technically I don't think attribute proc_macros can run before derives?

@est31
Copy link
Member

est31 commented May 15, 2018

The throw RFC seems to be quite unpopular:

screenshot

Is there any reason that this unpopularity will change within the next three years?

Also, reserving throw would make a throw! macro impossible which centril himself suggested (unless it is contextual for some definition of that term).

@scottmcm
Copy link
Member

The throw RFC seems to be quite unpopular [...] Is there any reason that this unpopularity will change within the next three years?

I have every confidence some people who dislike the existing error sugar or would rather not use concurrency sugar either will remain opposed to sugar in this area in the future too.

But there are plenty of things that could change. There may be people who

  • 👎'd it as throw but like fail
  • Dislike the particular semantics proposed there, but like a different version
  • Want to see a more holistic proposal than just adding the one construct
  • Change their mind as use of the language evolves
  • Etc.

And, of course, RFCs aren't a popularity contest anyway.

Also, reserving throw would make a throw! macro impossible

I disagree, just like reserving try doesn't necessarily block try!, and #2394 reserved await while still having await!.

@est31
Copy link
Member

est31 commented May 16, 2018

RFCs aren't a popularity contest anyway.

Surely, the merge descision of an RFC shouldn't sway if there as 30 upvotes and 29 downvotes and now two more people downvote the RFC. But here the number of downvotes is five times larger than the upvotes. That's plenty of room for a sizeable amount of people to change their mind or who are only 👎 because of the keyword choice.

The statement "RFCs aren't a popularity contest" is technically correct, but doesn't imply that popularity, especially unpopularity, doesn't play any role at all.

Note that for most RFCs, even those which get refused by the language team, there is a "popularity bonus": the number of 👍 is often larger than the number of 👎. So if there is an RFC where the number of 👎 is so much bigger than the number of 👍 , it is a clear sign that the community dislikes it.

I disagree, just like reserving try doesn't necessarily block try!

Right after the try {} RFC was merged @Centril didn't even list that question as unresolved in the tracking issue, he simply assumed that try! would be broken. So why should I believe you when you claim that you want to keep throw! a legal macro? Maybe you don't want that at all and just want the RFC to be merged? Edit: I redact this.

@scottmcm
Copy link
Member

he simply assumed [...]

This is uncharitable. He's also the one who updated the tracking issue to include it on the same day it was opened. I don't believe there was any malice or subterfuge or anything at all like that involved.

@est31
Copy link
Member

est31 commented May 16, 2018

@scottmcm okay I've redacted that particular statement.

@scottmcm
Copy link
Member

Note that for most RFCs, there is a "popularity bonus"

I think that dyn makes an interesting case here:

At the core, those are the same feature with the same syntax. But over those 15 months, experience and tweaking took dyn from one of the most heavily 👎'd PRs in this repo to one with broad support.

Will that happen for throw? Maybe, maybe not. I've made no secret that I'd be happy if it did. But I also acknowledge that right now it's contentious, and maybe it will remain that way.

@est31
Copy link
Member

est31 commented May 16, 2018

@scottmcm that difference is obviously only because of my early and strong support for #2113 (I was the first to 👍 it) :p. But seriously, that's a good point.

@Centril
Copy link
Contributor Author

Centril commented May 16, 2018

I don't particularly like to debate 👍 and 👎s. I find there are more important things to do.
But since it was brought up, I'd like to point out that the numbers cited by @est31 re. the first throw are not necessarily accurate.

There were a number of people who were in favor of the proposal who didn't use GitHub's "voting" system.
There were others who did 👎 who have said they'd be OK with using fail since it is less evocative of exceptional terminology.

Honestly, at this point, I'd like to revisit @nikomatsakis's point about letting @rfcbot remove 👎s from RFCs with the core team's blessing since these "vote tallies" are being used as arguments from time to time.
But to not fill this RFC with too much off-topic material, I'll raise it in another avenue.

@burdges
Copy link

burdges commented May 16, 2018

Anyway, there are currently no arguments against reserving fail, only arguments against reserving throw, right? And some concerns about ecosystem breakage due to reserving fail.

@iago-lito
Copy link

Hey, here is a naive question from a naive, Rust-excited reader. I am aware that I do not understand everything that these considerations imply.

Considering discussions here and there, and since it has come to a (legitimate) questionning about "popularity contests" and what to decide when one gets further off from unanimity, I keep wondering about the reasons we are willing to cling to something that feels fundamentally splitting off.

My basic feeling here is that |53 👍 44 👎 16 😕| can either mean that:

  • spambots have trolled the topic (I'll rule this out if you're okay with that ;)

  • OR the proposal is so eclectic / offers so many possibilites that everyone can think its own way and see the good or the bad in it.

    • imho, this is a case where discussing every alternatives and sorting them out is really useful, or the project will either stagnate or fall apart
  • OR the proposal is extremely separating, and it makes two underlying philosophies emerge, which were hidden by the common base until then.

    • imho, this is a different case (both frightening and beautiful) that deserves a special treatment. I think the open source community knows this situation well since it has given it a legitimity, a name, and tools to deal with it. So the project can branch.

So, do you think that all these arguments in favour or defavour of a try catch error fail Rust style reveals a "split-into-two" situation rather than an "radiative-explosive" situation?

How likely is the project to branch, so that some could explore the try catch error fail-style while others prefer keeping away from those to enjoy new horizons?
Is there, somewhere deep inside the roots of Rust project, anything prepared for this situation?

Is there, somewhere deep inside the roots of Rust project, anything prepared for the day that two strong, separated branches will be willing to merge again? :')

@audreyality
Copy link

Several comments in #2426 are concerned that, by developing try functionality incrementally, we're disregarding the error-handling feature at large. This RFC continues this approach, insofar as it splits the throw RFC into a keyword reservation and a language feature. The reasons for doing this are sound--the 2018 epoch gives us an opportunity to modify keywords, and it should be capitalized on. Continuing this approach, however, perpetuates a practice that has been questioned and not adequately addressed.

Several people (myself included) have proposed a from_ok variant of throw/fail (most commonly called pass). I'd like to widen this RFC to include the from_ok variant. There are a few reasons for this:

  • from_ok and from_err are intrinsically related. They both model early-exits, implicitly convert, and reinforce the "It's just data" nature of result propagation.
  • The symmetry of these concepts can assist learnability. One of the more compelling arguments for fail, in this regard, is that pass (or another antonym) is clearly related to both the try and fail concepts.
  • Reserving a from_ok keyword acknowledges that early-exiting keywords are open for debate. It avoids giving preferential treatment to one side of the discussion.

@scottmcm
Copy link
Member

@rfcbot fcp merge

This was discussed in the meeting today; let's move forward pending a concern I'll add...

@rfcbot
Copy link
Collaborator

rfcbot commented May 17, 2018

Team member @scottmcm has proposed to merge this. The next step is review by the rest of the tagged teams:

Concerns:

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels May 17, 2018
@scottmcm
Copy link
Member

@rfcbot concern use in macros and attributes

To ensure that this doesn't effect failure, please update the RFC to, like try!, allow these keywords to be used in macro calls (like throw!) and in attributes (like [fail]).

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels May 17, 2018
@scottmcm scottmcm self-assigned this May 17, 2018
@aturon
Copy link
Member

aturon commented Jun 7, 2018

Someone is supposed to leave a summary comment but I'm not sure who

That would be me, and apologies for not getting to it sooner!

The overwhelming sentiment during the meeting was that we are all tired of having heated arguments about keywords for not-yet-existent features, and are eager to put this issue to bed. That said, feelings about how to do that varied...

  • Drop all reservations except for accepted features (so: try, async, await)
  • Reserve liberally (so: merge this RFC, the delegate RFC, possibly toss in a few more)
  • Various options in between

A key point is how confident we feel that we can add some reasonable mechanism for within-edition reservations (like I proposed earlier). I'm not aware of strong opposition to an idea along these lines, but as always it's hard to predict how things will play out.

@nikomatsakis and I felt particularly strongly that up-front reservations are wrong and a mistake in the initial Edition proposal, basically for the reasons I've already outlined in the thread: they force up-front decisions about surface issues of features that are not yet fully proposed, let alone accepted or implemented. That just seems totally backwards and is going to keep leading to unworkable discussions. We both feel that the role of Editions here is that they can absorb any keyword-flags that have accumulated in the meantime.

In all, there is certainly no consensus to merge this RFC as-is, and I think there are no objections to instead closing it, under the assumption that we'll add a keyword-flag mechanism (or something like it) as needed later.

Given all that:

@rfcbot fcp cancel

@rfcbot
Copy link
Collaborator

rfcbot commented Jun 7, 2018

@aturon proposal cancelled.

@rfcbot rfcbot removed the disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. label Jun 7, 2018
@aturon
Copy link
Member

aturon commented Jun 7, 2018

@rfcbot fcp close

@rfcbot
Copy link
Collaborator

rfcbot commented Jun 7, 2018

Team member @aturon has proposed to close this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-close This RFC is in PFCP or FCP with a disposition to close it. labels Jun 7, 2018
@steveklabnik
Copy link
Member

I'm not aware of strong opposition to an idea along these lines,

I feel that up-front keyword reservations are the correct call, but I can appreciate that I may be in the minority here. I'd much prefer a liberal reservation of keywords.

We'll see how it goes :)

@nrc nrc removed the finished-final-comment-period The final comment period is finished for this RFC. label Jun 8, 2018
@scottmcm
Copy link
Member

scottmcm commented Jun 8, 2018

I don't feel strongly enough to block, but

  • I'd like to see a concrete new plan, not just "not this one".
  • Does this imply we should also immediately un-reserve existing keywords for unapproved features?
  • I rather like Steve's suggestion to just reserve a whole bunch of common keywords

@rfcbot reviewed

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-close This RFC is in PFCP or FCP with a disposition to close it. final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. labels Jun 8, 2018
@rfcbot
Copy link
Collaborator

rfcbot commented Jun 8, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@joshtriplett
Copy link
Member

I'd like to see a concrete new plan, not just "not this one".

That's likely to occur after the edition. And it'll likely be "plans", plural, since there's a great deal of discussion still to be had before we get anywhere close to a consensus.

@scottmcm scottmcm assigned aturon and unassigned scottmcm Jun 8, 2018
@scottmcm
Copy link
Member

scottmcm commented Jun 8, 2018

And it'll likely be "plans", plural, since there's a great deal of discussion still to be had before we get anywhere close to a consensus.

I meant a plan for an alternative to keyword reservations, not about any particular feature. Is there some lexical space we have available to use until a new edition shows up? Or maybe a plan to schedule regular editions, to help get some of the "it's ok, I'll just catch the next train" we have for normal stable?

@audreyality
Copy link

@scottmcm:

I meant a plan for an alternative to keyword reservations, not about any particular feature.

A good place to start may be "Idea"/"Pre-Pre-RRC" discussions for @aturon's keyword flags on internals. I question whether it will distract too much from 2018, though. Closing this RFC isn't beneficial if it would merely move the conflict around.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Jun 18, 2018
@rfcbot
Copy link
Collaborator

rfcbot commented Jun 18, 2018

The final comment period, with a disposition to close, as per the review above, is now complete.

By the power vested in me by Rust, I hereby close this RFC.

@rfcbot rfcbot added closed This FCP has been closed (as opposed to postponed) and removed disposition-close This RFC is in PFCP or FCP with a disposition to close it. labels Jun 18, 2018
@rfcbot rfcbot closed this Jun 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed This FCP has been closed (as opposed to postponed) finished-final-comment-period The final comment period is finished for this RFC. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.