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

array repeat exprs do not check the array type is well formed #111091

Closed
BoxyUwU opened this issue May 2, 2023 · 3 comments · Fixed by #111100
Closed

array repeat exprs do not check the array type is well formed #111091

BoxyUwU opened this issue May 2, 2023 · 3 comments · Fixed by #111100
Assignees
@BoxyUwU
Labels
C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@BoxyUwU
Copy link
Member

BoxyUwU commented May 2, 2023

I tried this code:

trait Foo {
    const ASSOC: [u8];
}

fn bar<T: Foo>() {
    let a = [T::ASSOC; 2];
}

fn main() {}

I expected to see this happen: It should not compile as the local variable a has type [[u8]; 2] which is not well formed

Instead, this happened: it compiled 😬

Meta

playground version:

Nightly channel

Build using the Nightly version: 1.71.0-nightly

(2023-04-30 9ecda8de85ce893cc3fc)
Backtrace 

<backtrace>

@BoxyUwU BoxyUwU added the C-bug Category: This is a bug. label May 2, 2023
@BoxyUwU BoxyUwU self-assigned this May 2, 2023
@jyn514 jyn514 added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-types Relevant to the types team, which will review and decide on the PR/issue. labels May 2, 2023
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 2, 2023
@asquared31415
Copy link
Contributor

I am pretty sure bar can't be called, since Foo can't be implemented by any type, since the const has a type that's not Sized (maybe some nightly feature enables this?). Is it an issue that a trait can have a const with a type that's not Sized?

@BoxyUwU BoxyUwU mentioned this issue May 2, 2023
Merged
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 3, 2023
@Dylan-DPC
Rollup merge of rust-lang#111100 - BoxyUwU:array_repeat_expr_wf, r=co…
1831409 
…mpiler-errors

check array type of repeat exprs is wf

Fixes rust-lang#111091

Also makes sure that we actually renumber regions in the length of repeat exprs which we previously weren't doing and would cause ICEs in `adt_const_params` + `generic_const_exprs` from attempting to prove the wf goals when the length was an unevaluated constant with `'erased` in the `ty` field of `Const`

The duplicate errors are caused by the fact that `const_arg_to_const`/`array_len_to_const` in `FnCtxt` adds a `WellFormed` goal for the created `Const` which is also checked by the added `WellFormed(array_ty)`. I don't want to change this to just emit a `T: Sized` goal for the element type since that would ignore `ConstArgHasType` wf requirements and generally uncomfortable with the idea of trying to sync up `wf::obligations` for arrays and the code in hir typeck for repeat exprs.

r? `@compiler-errors`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 3, 2023
@Dylan-DPC
Rollup merge of rust-lang#111100 - BoxyUwU:array_repeat_expr_wf, r=co…
8215170 
…mpiler-errors

check array type of repeat exprs is wf

Fixes rust-lang#111091

Also makes sure that we actually renumber regions in the length of repeat exprs which we previously weren't doing and would cause ICEs in `adt_const_params` + `generic_const_exprs` from attempting to prove the wf goals when the length was an unevaluated constant with `'erased` in the `ty` field of `Const`

The duplicate errors are caused by the fact that `const_arg_to_const`/`array_len_to_const` in `FnCtxt` adds a `WellFormed` goal for the created `Const` which is also checked by the added `WellFormed(array_ty)`. I don't want to change this to just emit a `T: Sized` goal for the element type since that would ignore `ConstArgHasType` wf requirements and generally uncomfortable with the idea of trying to sync up `wf::obligations` for arrays and the code in hir typeck for repeat exprs.

r? ``@compiler-errors``
@apiraino
Copy link
Contributor

apiraino commented May 4, 2023 •
edited
Loading

@BoxyUwU I've tried bisecting this but I can't find a previous version where compiling with rustc returns an error. Is this unsoundness always been there or can we bisect to a previous version that it didn't?

@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label May 4, 2023
@bors bors closed this as completed in c0ca84b May 4, 2023
@BoxyUwU
Copy link
Member Author

BoxyUwU commented May 4, 2023

I imagine its likely that this always was broken lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@BoxyUwU BoxyUwU

Labels
C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

check array type of repeat exprs is wf BoxyUwU/rust
5 participants
@apiraino @BoxyUwU @jyn514 @asquared31415 @rustbot