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

Issue 9755 once fns feature directive #9863

Conversation

csainty
Copy link
Contributor

@csainty csainty commented Oct 15, 2013

Hello,

First time rust contributor here, please let me know if I need to sort out the contribution agreement for this.

I picked issue #9755 to dip my toe in the water, this pull request isn't quite complete though as I have not updated the documentation. The reason for this is that I haven't tracked down why this feature is gated so I don't feel I can write a justification of the same quality as the other features have been documented.
If someone would like to explain or point me at a mail thread I am happy to update with this change.

Hopefully I have understood the process of converting the old flag into a directive correctly.

Also just to call out what I am sure if a known quirk when adding feature directives, you can't build this code unless you have a snapshot of the compiler which knows about the feature directive. Chicken and the egg. I split the change into two commits, the first should be able to build a snapshot that can compile the second.

bccx.span_err(
cmt0.span,
format!("cannot move out of {}{}", bccx.cmt_to_str(cmt), once_hint));
format!("cannot move out of {} (unless the destination closure \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should still only mention once fn if the #[feature(once_fn)] directive is enabled; since once fns are extremely experimental/undecided.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I could work out, feature_gate.rs does not expose the state of features to other pieces of code. I agree it would be better to keep the message conditional as it was before but didn't see a way to do that without making changes to the surface of feature_gate.rs

@alexcrichton
Copy link
Member

Thanks for the pull request, and nice work! I think that @huonw has a valid point about the default error message for closures including a gated feature is probably not the best idea. Feature gating could be modified to return what features were enabled (and possibly stored in a side table somewhere), but I think that it's also possibly OK to not have the clause in the error message regardless. Additionally, once fns will probably be removed once the closure reform has happened.

Additionally, right now the commit process is that the pull request as a whole is landed against master, passes all tests, and then bakes for awhile. We create snapshots periodically, allowing us to update code which needed the snapshot. To land this, it may need to allow once fns by default for a window of time. I can make a snapshot right after this lands on master, but doing this in stages through multiple commits would be a little difficult.

@nikomatsakis
Copy link
Contributor

(Note that the current plans for closures do not include once fns)

@alexcrichton
Copy link
Member

Would you mind squashing these commits into just one? Otherwise looks good to me, thanks!

…tive of the same name to replace it.

Changed the frame_address intrinsic to no longer be a once fn.
This removes the dependency on once_fns from std.
@csainty
Copy link
Contributor Author

csainty commented Oct 17, 2013

Squashed and rebased, the branch point was a few days old by now.

bors added a commit that referenced this pull request Oct 17, 2013
…ive, r=alexcrichton

Hello,

First time rust contributor here, please let me know if I need to sort out the contribution agreement for this.

I picked issue #9755 to dip my toe in the water, this pull request isn't quite complete though as I have not updated the documentation. The reason for this is that I haven't tracked down why this feature is gated so I don't feel I can write a justification of the same quality as the other features have been documented.
If someone would like to explain or point me at a mail thread I am happy to update with this change.

Hopefully I have understood the process of converting the old flag into a directive correctly.

Also just to call out what I am sure if a known quirk when adding feature directives, you can't build this code unless you have a snapshot of the compiler which knows about the feature directive. Chicken and the egg. I split the change into two commits, the first should be able to build a snapshot that can compile the second.
@bors bors closed this Oct 17, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 21, 2022
…ip1995

Fix typo in `expect_used` and `unwrap_used` warning messages

"\`an Option\`" -> "an \`Option\`" and "\`a Result\`" -> "a \`Result\`".

changelog: fix typo in `expect_used` and `unwrap_used` warning messages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants