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

suggest const_in_array_repeat_expression flag #65858

Merged
merged 1 commit into from
Oct 29, 2019

Conversation

davidtwco
Copy link
Member

This PR adds a suggestion to add the #![feature(const_in_array_repeat_expression)] attribute to the crate when a promotable expression is used in a repeat expression and the feature gate is not enabled.

Unfortunately, this ended up being a little bit more complex than I anticipated, which may not have been worth it given that this would all be removed when the feature is stabilized. However, with #65732 and #65737 being open, and the feature gate having not been being suggested to potential users, the feature might not be stabilized in a while, so maybe this is worth landing.

cc @Centril (addresses this comment)
r? @ecstatic-morse (opened issues related to RFC 2203 recently)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 26, 2019
@ecstatic-morse
Copy link
Contributor

ecstatic-morse commented Oct 27, 2019

There's a newly merged, top-down way to compute promotability in transform::promote_consts that would make this a few lines in borrow_check. You need to run collect_temps_and_candidates and pass the vector of TempStates to a Validator with explicit set to false. Then call validate_operand on the operand to the array repeat expression which will tell you if it is implicitly promotable. You might want to wait until #65839 gets merged (it's currently in the bors queue), which will make initializing a Validator a lot simpler.

We should be able to remove all the infrastructure in qualify_consts soon, so I think we should try to avoid adding new features that depend on it. That said, we could merge this as is, and I could port it to use eddyb's new stuff later.

@davidtwco davidtwco added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 27, 2019
@davidtwco
Copy link
Member Author

@ecstatic-morse I've marked this as S-blocked to wait until #65839 is merged, but if you want to merge this as-is and then port it to the new stuff later, feel free to.

@ecstatic-morse
Copy link
Contributor

@davidtwco #65839 has been merged. Let me know on Zulip if you have any questions. We may be able to similarly improve error messages when rvalue static promotion fails and 'static is required, but I don't know enough about lifetime inference to do this.

Sorry about the timing :/

@davidtwco davidtwco force-pushed the rfc-2203-feature-gate-in-error branch from 393c606 to a726578 Compare October 28, 2019 16:14
@davidtwco
Copy link
Member Author

@ecstatic-morse I've rebased atop that PR and implemented the suggested approach.

@davidtwco davidtwco added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Oct 28, 2019
@ecstatic-morse
Copy link
Contributor

Thanks @davidtwco!

r=me after we discuss the help message.

Note that this will be my first review approval as a new member of the rust-lang org. This change is low impact and in code that I am very familiar with, so I think this is fine.

@davidtwco davidtwco force-pushed the rfc-2203-feature-gate-in-error branch from a726578 to cfc387d Compare October 28, 2019 18:28
This commit adds a suggestion to add the
`#![feature(const_in_array_repeat_expression)]` attribute to the crate
when a promotable expression is used in a repeat expression.

Signed-off-by: David Wood <david@davidtw.co>
@davidtwco davidtwco force-pushed the rfc-2203-feature-gate-in-error branch from cfc387d to 92b1512 Compare October 28, 2019 18:38
@ecstatic-morse
Copy link
Contributor

Looks good to me as well.

@bors r+

@bors
Copy link
Contributor

bors commented Oct 28, 2019

📌 Commit 92b1512 has been approved by ecstatic-morse

@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 Oct 28, 2019
Centril added a commit to Centril/rust that referenced this pull request Oct 28, 2019
…error, r=ecstatic-morse

suggest `const_in_array_repeat_expression` flag

This PR adds a suggestion to add the `#![feature(const_in_array_repeat_expression)]` attribute to the crate when a promotable expression is used in a repeat expression and the feature gate is not enabled.

Unfortunately, this ended up being a little bit more complex than I anticipated, which may not have been worth it given that this would all be removed when the feature is stabilized. However, with rust-lang#65732 and rust-lang#65737 being open, and the feature gate having not been being suggested to potential users, the feature might not be stabilized in a while, so maybe this is worth landing.

cc @Centril (addresses [this comment](rust-lang#61749 (comment)))
r? @ecstatic-morse (opened issues related to RFC 2203 recently)
bors added a commit that referenced this pull request Oct 28, 2019
Rollup of 9 pull requests

Successful merges:

 - #65563 (Add long error explanation for E0587)
 - #65640 (Use heuristics to recover parsing of missing `;`)
 - #65643 (Correct handling of type flags with `ConstValue::Placeholder`)
 - #65825 (rustc: use IndexVec<DefIndex, T> instead of Vec<T>.)
 - #65858 (suggest `const_in_array_repeat_expression` flag)
 - #65877 (doc: introduce `once` in `iter::chain` document)
 - #65887 (doc: mention `get(_mut)` in Vec)
 - #65891 (self-profiling: Record something more useful for crate metadata generation event.)
 - #65893 (Output previous stable  error messaging when using stable build.)

Failed merges:

r? @ghost
@bors bors merged commit 92b1512 into rust-lang:master Oct 29, 2019
@davidtwco davidtwco deleted the rfc-2203-feature-gate-in-error branch October 29, 2019 08:28
@likebike
Copy link

There is a critical spelling mistake in the suggested attribute. Right now, the compiler suggests this:

= help: add #![feature(const_in_array_repeat_expression)] to the crate attributes to enable

...but the real attribute name is const_in_array_repeat_expressions (with an 's' at the end).

This mistake confused me for many hours. Please correct it ASAP.

@estebank
Copy link
Contributor

@likebike, could you create a new ticket for this linking to your comment here? I'm currently on mobile so I can't create it myself. It is easier to keep track of things that need to be done when there's a ticket for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants