Skip to content

Implement float literals in valtree #98825

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

Closed

Conversation

compiler-errors
Copy link
Member

Fixes #98813
cc: @rust-lang/wg-const-eval

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 2, 2022
@rust-highfive
Copy link
Contributor

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 2, 2022
@compiler-errors
Copy link
Member Author

r? @lcnr @oli-obk

@rust-highfive rust-highfive assigned lcnr and unassigned jackh726 Jul 2, 2022
@RalfJung
Copy link
Member

RalfJung commented Jul 2, 2022

Not having floats in valtrees was somewhat deliberate since their equality is not very well-behaved.

pub fn func<const F: f64>() {}

fn main() {
let _ = func::<14.0>();
Copy link
Member

Choose a reason for hiding this comment

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

Do we really want to accept this program? Obviously it shouldn't ICE, but IMO it should be rejected.

@compiler-errors
Copy link
Member Author

Interesting. Thanks for the info @RalfJung... in that case, how should we fix this ICE? 🤔

The code seems to assume we can always turn a const lit into a val tree..?

@RalfJung
Copy link
Member

RalfJung commented Jul 2, 2022

Let's see what @lcnr @oli-obk say, my info might be outdated. :)

@compiler-errors
Copy link
Member Author

(Alternatively, we could just provide some other error kind in lit_to_const that captures the "not meaningful to evaluate into a valtree" case, and handle that try_eval_lit_or_param instead of ICEing there.)

@RalfJung
Copy link
Member

RalfJung commented Jul 3, 2022

My expectation was that types in constants are limited to "structural types" in some sense, and the type checker would bug out early when a non-structural type is used. And structural types are exactly those for which we can always create a valtree.

@compiler-errors
Copy link
Member Author

Ah.. so ideally we shouldn't be able to use floats in a const generic to begin with... hmm yeah I guess we'll see what the others say

@lcnr
Copy link
Contributor

lcnr commented Jul 4, 2022

Ah.. so ideally we shouldn't be able to use floats in a const generic to begin with...

jup, seems like this was an oversight in fn check_param_wf if adt_const_params is enabled.

We don't want to accept floats as const parameters. The only possible equality we could use for them would by bitwise equality and that seems bad for 2 reasons:

  • "equal" values can have the same bitwise repr, e.g. different nans, +0 -0.
  • fun float facts like 0.1 + 0.2 != 0.3.

@compiler-errors
Copy link
Member Author

@lcnr, soooo apparently we use search_for_structural_match_violation to determine if it's a valid const type... but ty::Float isn't a structural match violation. I'm happy to fix this, maybe adding a flag to make f32/f64 forbidden in this case. Thoughts?

@oli-obk
Copy link
Contributor

oli-obk commented Jul 4, 2022

Yea, I think adding a Float variant to https://doc.rust-lang.org/nightly/nightly-rustc/rustc_trait_selection/traits/structural_match/enum.NonStructuralMatchTyKind.html seems the right way forward, as this way the pattern matching thing can avoid emitting a structural match error and keep emitting a float future incompat error instead.

@RalfJung
Copy link
Member

RalfJung commented Jul 4, 2022

match should treat floats like non-structural-match types though, i.e., no exhaustiveness checking and no "primitive" representation in the "match tree".

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jul 11, 2022
…-obk

Deny float const params even when `adt_const_params` is enabled

Supersedes rust-lang#98825
Fixes rust-lang#98813

r? `@oli-obk`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jul 11, 2022
…-obk

Deny float const params even when `adt_const_params` is enabled

Supersedes rust-lang#98825
Fixes rust-lang#98813

r? ``@oli-obk``
@compiler-errors compiler-errors deleted the floats-in-valtrees branch August 11, 2023 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Anonymous f64 generic parameters causes compiler panic
7 participants