Skip to content

✨Use parsed float value for useless comparison warnings #5030

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

✨Use parsed float value for useless comparison warnings #5030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1b6d00b
✨ Include parsed value for float literals
fruno-bulax Sep 29, 2025
573bd5d
✨ Use f64 for float comparison check
fruno-bulax Oct 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions compiler-core/src/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::analyse::Inferred;
use crate::bit_array;
use crate::build::{ExpressionPosition, Located, Target, module_erlang_name};
use crate::exhaustiveness::CompiledCase;
use crate::parse::SpannedString;
use crate::parse::{NotNan, SpannedString};
use crate::type_::error::VariableOrigin;
use crate::type_::expression::{Implementations, Purity};
use crate::type_::printer::Names;
Expand Down Expand Up @@ -1702,11 +1702,11 @@ fn pattern_and_expression_are_the_same(pattern: &TypedPattern, expression: &Type
// `"wibble" -> "wibble"`
(
TypedPattern::Float {
value: pattern_value,
float_value: pattern_value,
..
},
TypedExpr::Float { value, .. },
) => pattern_value == value,
TypedExpr::Float { float_value, .. },
) => pattern_value == float_value,
(TypedPattern::Float { .. }, _) => false,

(
Expand Down Expand Up @@ -2319,6 +2319,7 @@ pub enum Pattern<Type> {
Float {
location: SrcSpan,
value: EcoString,
float_value: NotNan,
},

String {
Expand Down
1 change: 1 addition & 0 deletions compiler-core/src/ast/constant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub enum Constant<T, RecordTag> {
Float {
location: SrcSpan,
value: EcoString,
float_value: NotNan,
},

String {
Expand Down
2 changes: 2 additions & 0 deletions compiler-core/src/ast/typed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use super::*;
use crate::{
build::ExpressionPosition,
exhaustiveness::CompiledCase,
parse::NotNan,
type_::{HasType, Type, ValueConstructorVariant, bool},
};

Expand All @@ -22,6 +23,7 @@ pub enum TypedExpr {
location: SrcSpan,
type_: Arc<Type>,
value: EcoString,
float_value: NotNan,
},

String {
Expand Down
3 changes: 3 additions & 0 deletions compiler-core/src/ast/untyped.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use vec1::Vec1;

use crate::parse::NotNan;

use super::*;

#[derive(Debug, Clone, PartialEq, Eq)]
Expand All @@ -13,6 +15,7 @@ pub enum UntypedExpr {
Float {
location: SrcSpan,
value: EcoString,
float_value: NotNan,
},

String {
Expand Down
7 changes: 6 additions & 1 deletion compiler-core/src/ast/visit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -774,6 +774,7 @@ where
location,
type_,
value,
float_value: _,
} => v.visit_typed_expr_float(location, type_, value),
TypedExpr::String {
location,
Expand Down Expand Up @@ -1568,7 +1569,11 @@ where
value,
int_value: _,
} => v.visit_typed_pattern_int(location, value),
Pattern::Float { location, value } => v.visit_typed_pattern_float(location, value),
Pattern::Float {
location,
value,
float_value: _,
} => v.visit_typed_pattern_float(location, value),
Pattern::String { location, value } => v.visit_typed_pattern_string(location, value),
Pattern::Variable {
location,
Expand Down
58 changes: 49 additions & 9 deletions compiler-core/src/ast_folder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::{
UntypedUseAssignment, Use, UseAssignment,
},
build::Target,
parse::NotNan,
type_::error::VariableOrigin,
};

Expand Down Expand Up @@ -258,7 +259,11 @@ pub trait UntypedExprFolder: TypeAstFolder + UntypedConstantFolder + PatternFold
value,
int_value,
} => self.fold_int(location, value, int_value),
UntypedExpr::Float { location, value } => self.fold_float(location, value),
UntypedExpr::Float {
location,
value,
float_value,
} => self.fold_float(location, value, float_value),
UntypedExpr::String { location, value } => self.fold_string(location, value),

UntypedExpr::Block {
Expand Down Expand Up @@ -714,8 +719,17 @@ pub trait UntypedExprFolder: TypeAstFolder + UntypedConstantFolder + PatternFold
}
}

fn fold_float(&mut self, location: SrcSpan, value: EcoString) -> UntypedExpr {
UntypedExpr::Float { location, value }
fn fold_float(
&mut self,
location: SrcSpan,
value: EcoString,
float_value: NotNan,
) -> UntypedExpr {
UntypedExpr::Float {
location,
value,
float_value,
}
}

fn fold_string(&mut self, location: SrcSpan, value: EcoString) -> UntypedExpr {
Expand Down Expand Up @@ -937,7 +951,11 @@ pub trait UntypedConstantFolder {
int_value,
} => self.fold_constant_int(location, value, int_value),

Constant::Float { location, value } => self.fold_constant_float(location, value),
Constant::Float {
location,
value,
float_value,
} => self.fold_constant_float(location, value, float_value),

Constant::String { location, value } => self.fold_constant_string(location, value),

Expand Down Expand Up @@ -998,8 +1016,17 @@ pub trait UntypedConstantFolder {
}
}

fn fold_constant_float(&mut self, location: SrcSpan, value: EcoString) -> UntypedConstant {
Constant::Float { location, value }
fn fold_constant_float(
&mut self,
location: SrcSpan,
value: EcoString,
float_value: NotNan,
) -> UntypedConstant {
Constant::Float {
location,
value,
float_value,
}
}

fn fold_constant_string(&mut self, location: SrcSpan, value: EcoString) -> UntypedConstant {
Expand Down Expand Up @@ -1188,7 +1215,11 @@ pub trait PatternFolder {
int_value,
} => self.fold_pattern_int(location, value, int_value),

Pattern::Float { location, value } => self.fold_pattern_float(location, value),
Pattern::Float {
location,
value,
float_value,
} => self.fold_pattern_float(location, value, float_value),

Pattern::String { location, value } => self.fold_pattern_string(location, value),

Expand Down Expand Up @@ -1277,8 +1308,17 @@ pub trait PatternFolder {
}
}

fn fold_pattern_float(&mut self, location: SrcSpan, value: EcoString) -> UntypedPattern {
Pattern::Float { location, value }
fn fold_pattern_float(
&mut self,
location: SrcSpan,
value: EcoString,
float_value: NotNan,
) -> UntypedPattern {
Pattern::Float {
location,
value,
float_value,
}
}

fn fold_pattern_string(&mut self, location: SrcSpan, value: EcoString) -> UntypedPattern {
Expand Down
2 changes: 2 additions & 0 deletions compiler-core/src/metadata/module_decoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::{
},
build::Origin,
line_numbers::{Character, LineNumbers},
parse::NotNan,
reference::{Reference, ReferenceKind, ReferenceMap},
schema_capnp::{self as schema, *},
type_::{
Expand Down Expand Up @@ -396,6 +397,7 @@ impl ModuleDecoder {
Constant::Float {
location: Default::default(),
value: value.into(),
float_value: NotNan::parse(value).expect("float value to parse as non-NaN f64"),
}
}

Expand Down
5 changes: 5 additions & 0 deletions compiler-core/src/metadata/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use crate::{
},
build::Origin,
line_numbers::LineNumbers,
parse::NotNan,
reference::{Reference, ReferenceKind},
type_::{
self, Deprecation, ModuleInterface, Opaque, References, Type, TypeAliasConstructor,
Expand Down Expand Up @@ -1056,6 +1057,7 @@ fn constant_float() {
let module = constant_module(Constant::Float {
location: Default::default(),
value: "1.0".into(),
float_value: NotNan::ONE,
});

assert_eq!(roundtrip(&module), module);
Expand Down Expand Up @@ -1084,6 +1086,7 @@ fn constant_tuple() {
Constant::Float {
location: Default::default(),
value: "1.0".into(),
float_value: NotNan::ONE,
},
Constant::Tuple {
location: Default::default(),
Expand All @@ -1096,6 +1099,7 @@ fn constant_tuple() {
Constant::Float {
location: Default::default(),
value: "1.0".into(),
float_value: NotNan::ONE,
},
],
},
Expand Down Expand Up @@ -1146,6 +1150,7 @@ fn constant_record() {
value: Constant::Float {
location: Default::default(),
value: "0.0".into(),
float_value: NotNan::ZERO,
},
},
CallArg {
Expand Down
52 changes: 49 additions & 3 deletions compiler-core/src/parse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,11 +515,12 @@ where
}
}

Some((start, Token::Float { value }, end)) => {
Some((start, Token::Float { value, float_value }, end)) => {
self.advance();
UntypedExpr::Float {
location: SrcSpan { start, end },
value,
float_value,
}
}

Expand Down Expand Up @@ -1399,11 +1400,12 @@ where
int_value,
}
}
Some((start, Token::Float { value }, end)) => {
Some((start, Token::Float { value, float_value }, end)) => {
self.advance();
Pattern::Float {
location: SrcSpan { start, end },
value,
float_value,
}
}
Some((start, Token::Hash, _)) => {
Expand Down Expand Up @@ -3096,11 +3098,12 @@ where
}))
}

Some((start, Token::Float { value }, end)) => {
Some((start, Token::Float { value, float_value }, end)) => {
self.advance();
Ok(Some(Constant::Float {
value,
location: SrcSpan { start, end },
float_value,
}))
}

Expand Down Expand Up @@ -4696,3 +4699,46 @@ impl PatternPosition {
}
}
}

/// A thin f64 wrapper that does not permit NaN.
/// This allows us to implement `Eq`, which require reflexivity.
///
/// Used for gleam float literals, which cannot be NaN.
/// While there is no syntax for "infinity", float literals
/// may overflow into (possibly negative) infinity on the JS target.
#[derive(Clone, Copy, Debug, PartialEq)]
pub struct NotNan(f64);

impl NotNan {
pub const ONE: Self = NotNan(1.0);
pub const ZERO: Self = NotNan(0.0);

/// Parse from a string, returning `None` if the string
/// is not a valid f64 or the float is `NaN``
pub fn parse(value: &str) -> Option<Self> {
value
.replace("_", "")
.parse::<f64>()
.ok()
.filter(|f| !f.is_nan())
.map(NotNan)
}

pub fn value(&self) -> f64 {
self.0
}
}

impl Eq for NotNan {}
impl Ord for NotNan {
fn cmp(&self, other: &Self) -> Ordering {
self.0
.partial_cmp(&other.0)
.expect("Only NaN comparisons should fail")
}
}
impl PartialOrd for NotNan {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
}
3 changes: 3 additions & 0 deletions compiler-core/src/parse/lexer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use ecow::EcoString;

use crate::ast::SrcSpan;
use crate::parse::NotNan;
use crate::parse::error::{LexicalError, LexicalErrorType};
use crate::parse::token::Token;
use std::char;
Expand Down Expand Up @@ -621,10 +622,12 @@ where
value.push_str(&exponent_run);
}
let end_pos = self.get_pos();
let float_value = NotNan::parse(&value).expect("float value to parse as non-NaN f64");
Ok((
start_pos,
Token::Float {
value: value.into(),
float_value,
},
end_pos,
))
Expand Down
6 changes: 6 additions & 0 deletions compiler-core/src/parse/snapshots/gleam_core__parse__tests__nested_block.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ expression: "{ 1 { 1.0 2.0 } 3 }"
end: 9,
},
value: "1.0",
float_value: NotNan(
1.0,
),
Comment on lines +37 to +39
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this the ONE constant you defined earlier?

Suggested change
float_value: NotNan(
1.0,
),
float_value: NotNan::ONE,

Copy link
Contributor Author

@fruno-bulax fruno-bulax Oct 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is the snapshot of the parser test, not rust code! 🕵️

Copy link
Member

Choose a reason for hiding this comment

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

Oh my, sorry for that, I totally missed the context 😆

},
),
Expression(
Expand All @@ -43,6 +46,9 @@ expression: "{ 1 { 1.0 2.0 } 3 }"
end: 13,
},
value: "2.0",
float_value: NotNan(
2.0,
),
},
),
],
Expand Down
Loading
Loading