rustc/rustc_mir: Implement RFC 2203. #61749

davidtwco · 2019-06-11T20:56:35Z

This PR implements RFC 2203, allowing constants in array repeat
expressions. Part of #49147.

r? @eddyb

src/librustc/mir/mod.rs

src/test/ui/consts/rfc-2203-const-array-repeat-exprs/migrate-borrowck.rs

src/librustc_mir/borrow_check/nll/type_check/mod.rs

src/librustc_mir/transform/promote_consts.rs

src/librustc_typeck/check/mod.rs

eddyb · 2019-06-12T05:59:57Z

src/librustc_mir/borrow_check/nll/type_check/mod.rs

+                            .span_label(span, &format!(
+                                "the trait `{}` is not implemented for `{}`",
+                                copy_path, ty,
+                            ))


cc @nikomatsakis @matthewjasper This is not a proper trait error, so it would miss the "backtrace" that one might get otherwise - any suggestions on how to fix that?

I've started re-using some existing functions for diagnostics in this PR. It appears to work correctly but it might not produce the full "backtrace" that you want.

eddyb · 2019-06-12T06:02:47Z

To expand on #61749 (comment), this example:

#[derive(Copy, Clone)]
struct Foo<T>(T);
fn main() {
    [Foo(String::new()); 4];
}

Currently errors with:

error[E0277]: the trait bound `std::string::String: std::marker::Copy` is not satisfied
 --> src/main.rs:4:5
  |
4 |     [Foo(String::new()); 4];
  |     ^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `std::string::String`
  |
  = note: required because of the requirements on the impl of `std::marker::Copy` for `Foo<std::string::String>`
  = note: the `Copy` trait is required because the repeated element will be copied

The "required because of the requirements on the impl of ..." part would now be missing, I think.
There might be a relatively easy way to produce a proper trait error, even from the NLL type_check.
Either way, it would be good to have tests like this to properly compare the effect.

davidtwco · 2019-06-12T07:55:37Z

Either way, it would be good to have tests like this to properly compare the effect.

I've added this example as a test.

eddyb · 2019-06-12T07:57:03Z

@davidtwco Sadly, without adding a test like this (or a more complex one) on master first, it's hard to see the changes in diagnostics.

davidtwco · 2019-06-12T09:16:07Z

@eddyb I've ran that example with another working directory (at most 24 hours old behind master) and this is the current error:

error[E0277]: the trait bound `std::string::String: std::marker::Copy` is not satisfied
 --> ../rust0/src/test/ui/consts/rfc-2203-const-array-repeat-exprs/trait-error.rs:8:5
  |
8 |     [Foo(String::new()); 4];
  |     ^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `std::string::String`
  |
  = note: required because of the requirements on the impl of `std::marker::Copy` for `Foo<std::string::String>`
  = note: the `Copy` trait is required because the repeated element will be copied

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.

src/test/ui/consts/rfc-2203-const-array-repeat-exprs/migrate-pass.rs

src/libsyntax/feature_gate.rs

src/librustc_mir/transform/qualify_consts.rs

src/test/ui/consts/rfc-2203-const-array-repeat-exprs/migrate-const-fn-pass.rs

eddyb · 2019-06-25T12:08:23Z

What is this blocked on, if anything?

davidtwco · 2019-06-25T12:53:27Z

I'm not aware of any outstanding concerns, only that it isn't a proper trait error with a backtrace being emitted.

eddyb · 2019-07-11T12:59:40Z

r? @matthewjasper or @nikomatsakis - We need a way to report proper trait errors, with their cause/origin backtraces.

eddyb · 2019-07-18T15:28:58Z

@davidtwco I'm sorry for blocking this, I don't think the diagnostic regressions are worth the wait.
Especially given situations like #53491 (comment).

eddyb · 2019-07-18T15:30:46Z

src/test/ui/const-generics/issue-61336-2.rs

@@ -3,11 +3,12 @@

 fn f<T: Copy, const N: usize>(x: T) -> [T; N] {
    [x; {N}]
+    //~^ ERROR array lengths can't depend on generic parameters


Huh I don't remember this error.

Oh it's for [x; n] expressions, not types. Makes sense (although it should use a ty::Const instead of hardcoding an integer, cc @varkor @oli-obk ).

eddyb · 2019-07-18T15:32:57Z

@bors r+ p=2 rollup=never

bors · 2019-07-18T15:32:58Z

📌 Commit 4b1bc2d has been approved by eddyb

bors · 2019-07-18T20:41:46Z

⌛ Testing commit 4b1bc2d with merge a336998...

@eddyb

…=eddyb rustc/rustc_mir: Implement RFC 2203. This PR implements RFC 2203, allowing constants in array repeat expressions. Part of #49147. r? @eddyb

bors · 2019-07-19T00:24:30Z

☀️ Test successful - checks-azure
Approved by: eddyb
Pushing a336998 to master...

Centril · 2019-07-26T18:35:23Z

src/test/ui/feature-gates/feature-gate-const_in_array_repeat_expressions.stderr

+   |
+   = help: the following implementations were found:
+             <std::option::Option<T> as std::marker::Copy>
+   = note: the `Copy` trait is required because the repeated element will be copied


This doesn't mention the feature gate :(

eddyb · 2019-08-14T10:13:42Z

src/librustc_mir/transform/qualify_consts.rs

+                    )) = self.body[bb].statements[stmt_idx].kind {
+                        promoted_temps.insert(index);
+                    }
+                }


I just realized, this is unnecessary, only Candidate::Ref cares (it removes StorageDead from the temp that would need to be 'static).

eddyb · 2019-08-14T10:16:01Z

src/librustc_mir/transform/qualify_consts.rs

+                        _,
+                        box Rvalue::Ref(_, _, Place::Base(PlaceBase::Local(index)))
+                    ) = self.body[bb].statements[stmt_idx].kind {
+                        promoted_temps.insert(index);


@oli-obk Wow this is wrong?! const FOO: &i32 = { let x = &(5, false).0; x }; fails because of it, lol.

@Centril

…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)

RalfJung · 2020-11-13T14:33:29Z

src/test/ui/feature-gates/feature-gate-const_in_array_repeat_expressions.rs

+    let arr: [Option<String>; 2] = [None::<String>; 2];
+    //~^ ERROR the trait bound `std::option::Option<std::string::String>: std::marker::Copy` is not satisfied [E0277]
+}
+


Turns out this test was not thorough enough, and some other cases sneaked past the feature gate.

Acknowledge that `[CONST; N]` is stable When `const_in_array_repeat_expressions` (RFC 2203) got unstably implemented as part of rust-lang#61749, accidentally, the special case of repeating a *constant* got stabilized immediately. That is why the following code works on stable: ```rust const EMPTY: Vec<i32> = Vec::new(); pub const fn bar() -> [Vec<i32>; 2] { [EMPTY; 2] } fn main() { let x = bar(); } ``` In contrast, if we had written `[expr; 2]` for some expression that is not *literally* a constant but could be evaluated at compile-time (e.g. `(EMPTY,).0`), this would have failed. We could take back this stabilization as it was clearly accidental. However, I propose we instead just officially accept this and stabilize a small subset of RFC 2203, while leaving the more complex case of general expressions that could be evaluated at compile-time unstable. Making that case work well is pretty much blocked on inline `const` expressions (to avoid relying too much on [implicit promotion](https://github.com/rust-lang/const-eval/blob/master/promotion.md)), so it could take a bit until it comes to full fruition. `[CONST; N]` is an uncontroversial subset of this feature that has no semantic ambiguities, does not rely on promotion, and basically provides the full expressive power of RFC 2203 but without the convenience (people have to define constants to repeat them, possibly using associated consts if generics are involved). Well, I said "no semantic ambiguities", that is only almost true... the one point I am not sure about is `[CONST; 0]`. There are two possible behaviors here: either this is equivalent to `let x = CONST; [x; 0]`, or it is a NOP (if we argue that the constant is never actually instantiated). The difference between the two is that if `CONST` has a destructor, it should run in the former case (but currently doesn't, due to rust-lang#74836); but should not run if it is considered a NOP. For regular `[x; 0]` there seems to be consensus on running drop (there isn't really an alternative); any opinions for the `CONST` special case? Should this instantiate the const only to immediately run its destructors? That seems somewhat silly to me. After all, the `let`-expansion does *not* work in general, for `N > 1`. Cc `@rust-lang/lang` `@rust-lang/wg-const-eval` Cc rust-lang#49147

rust-highfive assigned eddyb Jun 11, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 11, 2019

eddyb reviewed Jun 11, 2019

View reviewed changes

src/librustc/mir/mod.rs Outdated Show resolved Hide resolved

Centril reviewed Jun 11, 2019

View reviewed changes

davidtwco force-pushed the rfc-2203-const-array-repeat-exprs branch from 0ec77d9 to fd8fc14 Compare June 11, 2019 21:32

This comment has been minimized.

Sign in to view

eddyb reviewed Jun 12, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

davidtwco force-pushed the rfc-2203-const-array-repeat-exprs branch from 3c7ee48 to 99ebd9a Compare June 12, 2019 09:48

This comment has been minimized.

Sign in to view

davidtwco force-pushed the rfc-2203-const-array-repeat-exprs branch 2 times, most recently from 55546ac to fbf884b Compare June 12, 2019 12:52

Centril reviewed Jun 12, 2019

View reviewed changes

src/test/ui/consts/rfc-2203-const-array-repeat-exprs/migrate-pass.rs Show resolved Hide resolved

src/libsyntax/feature_gate.rs Outdated Show resolved Hide resolved

src/librustc_mir/transform/qualify_consts.rs Outdated Show resolved Hide resolved

davidtwco force-pushed the rfc-2203-const-array-repeat-exprs branch from fbf884b to 72ec9d7 Compare June 12, 2019 16:06

Centril reviewed Jun 15, 2019

View reviewed changes

src/test/ui/consts/rfc-2203-const-array-repeat-exprs/migrate-const-fn-pass.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

davidtwco force-pushed the rfc-2203-const-array-repeat-exprs branch from b9cae48 to 9d0ce71 Compare June 17, 2019 17:49

This comment has been minimized.

Sign in to view

davidtwco force-pushed the rfc-2203-const-array-repeat-exprs branch from 9d0ce71 to 2c10ded Compare June 19, 2019 17:32

This comment has been minimized.

Sign in to view

davidtwco force-pushed the rfc-2203-const-array-repeat-exprs branch from 2c10ded to 4a185c0 Compare June 23, 2019 18:28

This comment has been minimized.

Sign in to view

davidtwco force-pushed the rfc-2203-const-array-repeat-exprs branch from 4a185c0 to a357743 Compare June 25, 2019 19:23

This comment has been minimized.

Sign in to view

tests: Add tests that use const fns.

4b1bc2d

davidtwco force-pushed the rfc-2203-const-array-repeat-exprs branch from 8168d22 to 4b1bc2d Compare July 7, 2019 19:23

rust-highfive assigned matthewjasper and unassigned eddyb Jul 11, 2019

eddyb reviewed Jul 18, 2019

View reviewed changes

eddyb approved these changes Jul 18, 2019

View reviewed changes

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 Jul 18, 2019

eddyb mentioned this pull request Jul 18, 2019

Tracking issue for RFC 1892, "Deprecate uninitialized in favor of a new MaybeUninit type" #53491

Closed

3 tasks

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 19, 2019

bors merged commit 4b1bc2d into rust-lang:master Jul 19, 2019

bors mentioned this pull request Jul 19, 2019

Initial implementation of or-patterns #61708

Merged

davidtwco deleted the rfc-2203-const-array-repeat-exprs branch July 19, 2019 11:43

Centril reviewed Jul 26, 2019

View reviewed changes

eddyb reviewed Aug 14, 2019

View reviewed changes

davidtwco mentioned this pull request Oct 26, 2019

suggest const_in_array_repeat_expression flag #65858

Merged

RalfJung mentioned this pull request Nov 13, 2020

Tracking issue for RFC 2203, "Constants in array repeat expressions" #49147

Closed

3 tasks

RalfJung reviewed Nov 13, 2020

View reviewed changes

RalfJung mentioned this pull request Nov 21, 2020

Acknowledge that [CONST; N] is stable #79270

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc/rustc_mir: Implement RFC 2203. #61749

rustc/rustc_mir: Implement RFC 2203. #61749

davidtwco commented Jun 11, 2019

This comment has been minimized.

eddyb Jun 12, 2019

davidtwco Jun 12, 2019

eddyb commented Jun 12, 2019

davidtwco commented Jun 12, 2019

eddyb commented Jun 12, 2019

This comment has been minimized.

davidtwco commented Jun 12, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

eddyb commented Jun 25, 2019

davidtwco commented Jun 25, 2019

This comment has been minimized.

This comment has been minimized.

eddyb commented Jul 11, 2019

eddyb commented Jul 18, 2019

eddyb Jul 18, 2019

eddyb Jul 18, 2019

eddyb commented Jul 18, 2019

bors commented Jul 18, 2019

bors commented Jul 18, 2019

bors commented Jul 19, 2019

Centril Jul 26, 2019

eddyb Aug 14, 2019

eddyb Aug 14, 2019

RalfJung Nov 13, 2020

rustc/rustc_mir: Implement RFC 2203. #61749

rustc/rustc_mir: Implement RFC 2203. #61749

Conversation

davidtwco commented Jun 11, 2019

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddyb commented Jun 12, 2019

davidtwco commented Jun 12, 2019

eddyb commented Jun 12, 2019

This comment has been minimized.

davidtwco commented Jun 12, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

eddyb commented Jun 25, 2019

davidtwco commented Jun 25, 2019

This comment has been minimized.

This comment has been minimized.

eddyb commented Jul 11, 2019

eddyb commented Jul 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddyb commented Jul 18, 2019

bors commented Jul 18, 2019

bors commented Jul 18, 2019

bors commented Jul 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment