Hover on floating consts shows zero if their value is positive

[RReverser]

rust-analyzer version: rust-analyzer version: 81805d4 2022-05-24 nightly

rustc version: rustc 1.60.0 (7737e0b5c 2022-04-04)

relevant settings: (eg. client settings, or environment variables like CARGO, RUSTUP_HOME or CARGO_HOME)

If I have consts like those:

const INT_POS: i32 = 10;
const INT_NEG: i32 = -10;
const F32_POS: f32 = 10.0;
const F32_NEG: f32 = -10.0;
const F64_POS: f64 = 10.0;
const F64_NEG: f64 = -10.0;

Then hovering on INT_POS, INT_NEG, F32_NEG and F64_NEG shows their values as expected. E.g.:

However, hovering F32_POS and F64_POS, for some reason, shows zeros:

[jhgg]

We never actually seem to lower the ast::LiteralKind for a FloatNumber to its proper value in hir:

rust-analyzer/crates/hir-def/src/body/lower.rs

Line 981 in 60a2ab4

Literal::Float(Default::default(), ty)

[RReverser]

Then I'm even more puzzled why negative numbers work as expected.

[jhgg]

So, the reason for this strange behavior is that const evaluation doesn't support negated unary op for floats:

rust-analyzer/crates/hir-ty/src/consteval.rs

Line 205 in 1f011fa

_ => return Err(ConstEvalError::NotSupported("this kind of operator")),

Thus, we fall-back to using the syntax tree to render the value:

rust-analyzer/crates/ide/src/hover/render.rs

Line 353 in 75689f2

Err(_) => it.value(db).map(|x| format!("{}", x)),

However, when handling a raw literal e.g: 10.0 we look at the literal value directly, which is set to 0 by the aforementioned lowering code.

rust-analyzer/crates/hir-ty/src/consteval.rs

Line 159 in 1f011fa

Expr::Literal(l) => Ok(ComputedExpr::Literal(l.clone())),

So the fix here would be to fix the lowering code to actually specify a real value for the float.

[feniljain]

On looking into it, the reason it is not implemented over here:

rust-analyzer/crates/hir-def/src/body/lower.rs

Line 981 in d7c1474

Literal::Float(Default::default(), ty)

is because of this FIXME reason note:

rust-analyzer/crates/hir-def/src/expr.rs

Line 49 in d7c1474

Float(u64, Option<BuiltinFloat>), // FIXME: f64 is not Eq

[RReverser]

is because of this FIXME reason note

If that's not something quick to implement, could we at least fall back to raw literal handling as in

Thus, we fall-back to using the syntax tree to render the value:

Instead of rendering zero?

[feniljain]

@jhgg would it better to return ConstEvalError::NotSupported for Float variant of Literal over here:

rust-analyzer/crates/hir-ty/src/consteval.rs

Line 159 in d7c1474

Expr::Literal(l) => Ok(ComputedExpr::Literal(l.clone())),

And I will add a note around here

rust-analyzer/crates/hir-def/src/expr.rs

Line 49 in d7c1474

Float(u64, Option<BuiltinFloat>), // FIXME: f64 is not Eq

for fixing this error, in case we implement some semi-precisoned/hacky way of Eq for f64 in future

[RReverser]

in case we implement some semi-precisoned/hacky way of Eq for f64 in future

You could probably use OrderedFloat<f64> from https://docs.rs/ordered-float/latest/ordered_float/struct.OrderedFloat.html or a similar helper instead of the current u64.

[flodiebold]

We should be able to just encode the float into the u64 as bits, using to_bits and from_bits. We don't need ordering here, and bit-wise equality should be fine.

[RReverser]

That works too. Personally I'd still use some newtype wrapper that would use {to,from}_bits only in implementation of PartialEq for readability, otherwise the next contributor will be equally confused by float being represented as u64.

[rben01]

I'm seeing this issue on rust-analyzer version: 0.3.1178-standalone (a670ff888 2022-08-21). Not sure if it's due to a regression or if bd06902 just hasn't made it into master yet, but just thought I'd bring it up.