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.

Sign up for GitHub

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

Jump to bottom

Update E0744 about control flow in const contexts to accurately describe when the error is triggered and why #79087

Merged
merged 1 commit into from
Nov 17, 2020
Merged

Update E0744 about control flow in const contexts to accurately describe when the error is triggered and why #79087

merged 1 commit into from
Nov 17, 2020

Conversation

ThePuzzlemaker
Copy link
Contributor

@ThePuzzlemaker ThePuzzlemaker commented Nov 16, 2020 •
edited
Loading

This PR fixes #79083. const fn currently supports if, match, loop, and while in terms of control flow. The error relating to control flow in const contexts currently states that those control flow constructs are not allowed in const contexts. That is no longer true, as RFC 2342 and 2344 were stabilized. for loops, however, as well as ? and .await are still not allowed, so I changed the error message to be more descriptive of the error as it is not just control flow that could trigger this error. I also added links to tracking issues that mark things that are blocking the usage of these expressions.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @petrochenkov (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 16, 2020
@jyn514 jyn514 added A-diagnostics Area: Messages for errors, warnings, and lints A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels Nov 16, 2020
@jyn514
Copy link
Member

jyn514 commented Nov 16, 2020

I changed the error message to be more descriptive of the error as it is currently only for loops (please confirm!)

You can check by searching for E0774 in compiler/.

@ThePuzzlemaker
Copy link
Contributor Author

It seems like it is also triggered for basically any unsupported expression that does not have a corresponding feature gate, so I'll update that to say that.

@ThePuzzlemaker
Copy link
Contributor Author

I'll have to look at its usages to see if there are any other examples.

@ThePuzzlemaker
Copy link
Contributor Author

ThePuzzlemaker commented Nov 16, 2020 •
edited
Loading

I'm going to add more links to issues that block various things related to ? and .await then you can merge if you're fine with the changes.

@petrochenkov
Copy link
Contributor

r? @RalfJung

@RalfJung
Copy link
Member

RalfJung commented Nov 16, 2020 •
edited
Loading

so I changed the error message to be more descriptive of the error as it is currently only for loops (please confirm!) that trigger this error

I don't know if this is true, hopefully @oli-obk or @ecstatic-morse can confirm.

EDIT: Oh, the PR description is outdated. Could you please update it? It will also appear in the git history after merging, so it is a good idea to make sure it stays up-to-date.

RalfJung
RalfJung approved these changes Nov 16, 2020
View reviewed changes
Copy link
Member

@RalfJung RalfJung left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good. :)

Can you squash the changes to a single commit? Let me know if you need help with handling git for that.

@jyn514
Copy link
Member

jyn514 commented Nov 16, 2020

https://rustc-dev-guide.rust-lang.org/git.html#advanced-rebasing

@ThePuzzlemaker ThePuzzlemaker changed the title Update E0744 about control flow in const contexts to reflect the current status of control flow in const fn Update E0744 about control flow in const contexts to accurately describe when the error is triggered and why Nov 16, 2020
@ThePuzzlemaker
Copy link
Contributor Author

ThePuzzlemaker commented Nov 16, 2020 •
edited
Loading

https://rustc-dev-guide.rust-lang.org/git.html#advanced-rebasing

Why did you send this link? (Also, the first force push was cause I'm an idiot at Git, and the second is following the instructions in the rebasing section of the git guide on the rustc dev guide) Ah, nvm, just saw RalfJung's comment

@ThePuzzlemaker
Update E0744 about control flow in const contexts to reflect the cu…
0bc04e3 
…rrent status of `const fn`.

This is a squash of these commits:
- Update E0744 about control flow in `const` contexts to reflect current status of `const fn`.
- E0744 isn't just about `for` loops or control flow
- Fix formatting on E0744 cause my editor decided to not copy it well
- Improve wording
- Fix a markdown formatting error
- Fix E0744's description as I interpreted some code wrong
- Fix a minor wording issue again
- Add a few more links to blocking issues
- Improve links to tracking issues
@RalfJung
Copy link
Member

Thanks. :)
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 16, 2020

📌 Commit 0bc04e3 has been approved by RalfJung

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 16, 2020
@ThePuzzlemaker
Copy link
Contributor Author

ThePuzzlemaker commented Nov 16, 2020 •
edited
Loading

Thank you all for being so supportive to a new contributor 😄 The Rust community truly is great!

@m-ou-se m-ou-se mentioned this pull request Nov 16, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 16, 2020
@bors
Auto merge of rust-lang#79104 - m-ou-se:rollup-v74492y, r=m-ou-se
c6a6105 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#74989 (Implement `Index` and `IndexMut` for arrays)
 - rust-lang#76339 (Test structural matching for all range types)
 - rust-lang#77691 (Rename/Deprecate LayoutErr in favor of LayoutError)
 - rust-lang#78364 (Update RELEASES.md for 1.48.0)
 - rust-lang#78678 (Add tests and improve rendering of cfgs on traits)
 - rust-lang#78714 (Simplify output capturing)
 - rust-lang#78769 (Remove unneeded lifetimes in array/mod.rs)
 - rust-lang#78903 (BTreeMap: test chaotic ordering & other bits & bobs)
 - rust-lang#79032 (improve type const mismatch errors)
 - rust-lang#79061 (Make all rustdoc functions and structs crate-private)
 - rust-lang#79087 (Update E0744 about control flow in `const` contexts to accurately describe when the error is triggered and why)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e6b6c8e into rust-lang:master Nov 17, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 17, 2020
@ThePuzzlemaker ThePuzzlemaker deleted the issue-79083-docfix branch November 17, 2020 01:46
@RalfJung RalfJung mentioned this pull request Nov 26, 2020
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung approved these changes

@jyn514 jyn514 jyn514 left review comments

Assignees

@RalfJung RalfJung

Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.50.0
Development

Successfully merging this pull request may close these issues.

rustc --explain E0744 says "while" is forbidden but accepts it nontheless
7 participants
@ThePuzzlemaker @rust-highfive @jyn514 @petrochenkov @RalfJung @bors @rustbot