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

variant_count intrinsic is evaluated incorrectly in generic code #79137

Closed
tmiasko opened this issue Nov 17, 2020 · 4 comments · Fixed by #79231
Closed

variant_count intrinsic is evaluated incorrectly in generic code #79137

tmiasko opened this issue Nov 17, 2020 · 4 comments · Fixed by #79231
Assignees
Labels
C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. F-variant_count `#![feature(variant_count)]` I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way.

Comments

@tmiasko
Copy link
Contributor

tmiasko commented Nov 17, 2020

For example:

#![feature(variant_count)]

fn a<T>() -> usize {
    let v = [0u8; std::mem::variant_count::<T>()];
    std::mem::size_of_val(&v)
}

fn b() -> usize {
    let v = [0u8; std::mem::variant_count::<Result<(), ()>>()];
    std::mem::size_of_val(&v)
}

fn main() {
    println!("{}", a::<Result<(), ()>>());
    println!("{}", b());
}
$ rustc a.rs
warning: cannot use constants which depend on generic parameters in types
 --> a.rs:4:19
  |
4 |     let v = [0u8; std::mem::variant_count::<T>()];
  |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(const_evaluatable_unchecked)]` on by default
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #76200 <https://github.com/rust-lang/rust/issues/76200>

warning: 1 warning emitted

$ ./a
0
2

Similar to #73976, but with a different intrinsic.

@tmiasko tmiasko added the C-bug Category: This is a bug. label Nov 17, 2020
@jonas-schievink jonas-schievink added I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. F-variant_count `#![feature(variant_count)]` labels Nov 17, 2020
@lcnr lcnr added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Nov 17, 2020
@lcnr
Copy link
Contributor

lcnr commented Nov 17, 2020

We can probably just put ensure_monomorphic call in front of that if statement.

if let ty::Adt(ref adt, _) = tp_ty.kind() {

A more precise way to implement this would be to exhaustively match on tp_ty and forbid ty::Projection, ty::Opaque, ty::Param, ty::Bound, ty::Placeholder and ty::Infer here. This way we the intrinsic can still be evaluated for Option<T> which is probably desirable.

@wusyong
Copy link
Contributor

wusyong commented Nov 19, 2020

@rustbot claim

@wusyong
Copy link
Contributor

wusyong commented Nov 19, 2020

@lcnr Hello, I'm interested in taking the desirable approach. How is the arm for variants like ty::Projection should be handled? Which macro is more appropriate for this situation?

@lcnr
Copy link
Contributor

lcnr commented Nov 19, 2020

I quickly grepped for throw.*TooGeneric and ended up with throw_inval!(TooGeneric) here.
That only works once you know what to search for though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. F-variant_count `#![feature(variant_count)]` I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants