Skip to content

Allow const parameters in array sizes to be unified #60742

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 17 commits into from
May 29, 2019

Conversation

varkor
Copy link
Member

@varkor varkor commented May 11, 2019

Fixes #60632.
Fixes #60744.
Fixes #60923.
(The last commit should probably be viewed in isolation, as it just renames things from type to kind.)

r? @eddyb

@rust-highfive

This comment has been minimized.

@varkor
Copy link
Member Author

varkor commented May 12, 2019

This changes some test output, but it's in very rare cases, and the messages are not incorrect (they're just a little extraneous).

@bors
Copy link
Collaborator

bors commented May 13, 2019

☔ The latest upstream changes (presumably #60765) made this pull request unmergeable. Please resolve the merge conflicts.

@varkor varkor force-pushed the fn-const-array-parameter branch 3 times, most recently from 7e94d17 to 6ced4f5 Compare May 13, 2019 20:32
@@ -11,7 +11,7 @@ error[E0308]: mismatched types
--> $DIR/const-array-oob-arith.rs:8:44
|
LL | const BOO: [i32; (ARR[0] - 41) as usize] = [5, 99];
| ^^^^^^^ expected an array with a fixed size of 1 elements, found one with 2 elements
| ^^^^^^^ expected `Const { ty: usize, val: Scalar(Bits { size: 8, bits: 1 }) }`, found `Const { ty: usize, val: Scalar(Bits { size: 8, bits: 2 }) }`
Copy link
Member

Choose a reason for hiding this comment

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

cc @oli-obk should we wait until printing this is fixed?

@varkor
Copy link
Member Author

varkor commented May 14, 2019

Blocks #60839.
@bors p=1

@yodaldevoid
Copy link
Contributor

I believe this is waiting on #59276, correct?

@varkor
Copy link
Member Author

varkor commented May 18, 2019

@yodaldevoid: that is correct.

@varkor
Copy link
Member Author

varkor commented May 20, 2019

I've added some tests to confirm this also fixes #60923.

@rust-highfive

This comment has been minimized.

@varkor varkor force-pushed the fn-const-array-parameter branch 2 times, most recently from 83c32f5 to eee346f Compare May 25, 2019 19:42
@varkor
Copy link
Member Author

varkor commented May 25, 2019

@eddyb: I've updated the tests following #59276.

@varkor
Copy link
Member Author

varkor commented May 27, 2019

I've added back in the special cased message for arrays of different lengths and fixed a typo in related diagnostics. This is ready for re-review @eddyb.

@@ -14,7 +14,7 @@ error[E0308]: mismatched types
--> $DIR/cannot-infer-type-for-const-param.rs:10:22
|
LL | let _ = Foo::<3>([1, 2, 3]);
| ^^^^^^^^^ expected `Const { ty: usize, val: Unevaluated(DefId(0:18 ~ cannot_infer_type_for_const_param[317d]::main[0]::{{constant}}[0]), []) }`, found `Const { ty: usize, val: Scalar(0x0000000000000003) }`
| ^^^^^^^^^ expected `3`, found `3usize`
Copy link
Member

Choose a reason for hiding this comment

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

This... is really weird? I assume the 3 is @oli-obk's printing of same-crate anon consts?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's because it can't infer the specific type, rather than an issue with printing. This is a bug, though, which should be fixed by #60839. (Which is ready to go as soon as this one is.)

Copy link
Member

Choose a reason for hiding this comment

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

I mean that the printing output is from Unevaluated for the 3 in Foo::<3> and not the value 3.

Copy link
Contributor

Choose a reason for hiding this comment

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

ugh, ok I did not envision user facing effects from that. So.... should I just do _ or {{unevaluated}} or sth?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need to jump to something else here, but we should consider it carefully.
_ is clearly the conservative choice, as for "unevaluated" - I don't like that name too much, it just happened to be useful in ty::Const (and we could/should pick another name).

@varkor varkor force-pushed the fn-const-array-parameter branch from 5211145 to 6233d1f Compare May 28, 2019 20:35
@varkor
Copy link
Member Author

varkor commented May 28, 2019

@bors r=eddyb

@bors
Copy link
Collaborator

bors commented May 28, 2019

📌 Commit 6233d1f has been approved by eddyb

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 28, 2019
Centril added a commit to Centril/rust that referenced this pull request May 28, 2019
…eddyb

Allow const parameters in array sizes to be unified

Fixes rust-lang#60632.
Fixes rust-lang#60744.
Fixes rust-lang#60923.
(The last commit should probably be viewed in isolation, as it just renames things from `type` to `kind`.)

r? @eddyb
Centril added a commit to Centril/rust that referenced this pull request May 28, 2019
…eddyb

Allow const parameters in array sizes to be unified

Fixes rust-lang#60632.
Fixes rust-lang#60744.
Fixes rust-lang#60923.
(The last commit should probably be viewed in isolation, as it just renames things from `type` to `kind`.)

r? @eddyb
bors added a commit that referenced this pull request May 28, 2019
Rollup of 9 pull requests

Successful merges:

 - #60742 (Allow const parameters in array sizes to be unified)
 - #60756 (Add better tests for hidden lifetimes in impl trait)
 - #60928 (Changes the type `mir::Mir` into `mir::Body`)
 - #61024 (tests: Centralize proc macros commonly used for testing)
 - #61157 (BufReader: In Seek impl, remove extra discard_buffer call)
 - #61195 (Special-case `.llvm` in mangler)
 - #61202 (Print PermissionExt::mode() in octal in Documentation Examples)
 - #61259 (Mailmap fixes)
 - #61273 (mention that MaybeUninit is a bit like Option)

Failed merges:

r? @ghost
@bors bors merged commit 6233d1f into rust-lang:master May 29, 2019
@varkor varkor added the F-const_generics `#![feature(const_generics)]` label Oct 19, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
F-const_generics `#![feature(const_generics)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
8 participants