Skip to content

Commit 2c061d4

Browse files
committed
Fix false-positive redundant comparisons warning
1 parent 69890d3 commit 2c061d4

8 files changed

+227
-5
lines changed

CHANGELOG.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,24 @@
3434
than stopping at the syntax error.
3535
([mxtthias](https://github.com/mxtthias))
3636

37+
- Fixed a false-positive warning where the compiler claimed all comparisons of
38+
constructors were redundant and equal to `True`:
39+
```gleam
40+
type Comparison {
41+
Wibble
42+
Wobble(String)
43+
}
44+
45+
pub fn main() {
46+
echo Wibble == Wibble // True
47+
echo Wobble == Wobble // False
48+
}
49+
```
50+
In reality, nullary constructors are values (`Wibble == Wibble`), while
51+
constructors with arguments are functions: `Wobble != Wobble`. The old warning
52+
incorrectly suggested both were true.
53+
([Adi Salimgereyev](https://github.com/abs0luty))
54+
3755
### Build tool
3856

3957
- The help text displayed by `gleam dev --help`, `gleam test --help`, and

compiler-core/src/type_/expression.rs

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5202,11 +5202,25 @@ fn static_compare(one: &TypedExpr, other: &TypedExpr) -> StaticComparison {
52025202
| (
52035203
ValueConstructorVariant::LocalConstant { .. },
52045204
ValueConstructorVariant::LocalConstant { .. },
5205-
)
5206-
| (ValueConstructorVariant::Record { .. }, ValueConstructorVariant::Record { .. })
5207-
if one == other =>
5208-
{
5209-
StaticComparison::CertainlyEqual
5205+
) if one == other => StaticComparison::CertainlyEqual,
5206+
5207+
// Zero-arity record constructors with the same name are certainly equal
5208+
(
5209+
ValueConstructorVariant::Record { arity: 0, .. },
5210+
ValueConstructorVariant::Record { arity: 0, .. },
5211+
) if one == other => StaticComparison::CertainlyEqual,
5212+
5213+
// Non-zero arity record constructors are functions, and comparing
5214+
// functions always returns False even if they have the same name
5215+
(
5216+
ValueConstructorVariant::Record {
5217+
arity: arity_one, ..
5218+
},
5219+
ValueConstructorVariant::Record {
5220+
arity: arity_two, ..
5221+
},
5222+
) if one == other && *arity_one > 0 && *arity_two > 0 => {
5223+
StaticComparison::CertainlyDifferent
52105224
}
52115225

52125226
(
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
source: compiler-core/src/type_/tests/warnings.rs
3+
expression: "\ntype Comparison {\n Wobble(String)\n}\n\npub fn main() {\n Wobble(\"a\") == Wobble(\"a\")\n}\n"
4+
---
5+
----- SOURCE CODE
6+
7+
type Comparison {
8+
Wobble(String)
9+
}
10+
11+
pub fn main() {
12+
Wobble("a") == Wobble("a")
13+
}
14+
15+
16+
----- WARNING
17+
warning: Redundant comparison
18+
┌─ /src/warning/wrn.gleam:7:3
19+
20+
7Wobble("a") == Wobble("a")
21+
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^ This is always `True`
22+
23+
This comparison is redundant since it always succeeds.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
source: compiler-core/src/type_/tests/warnings.rs
3+
expression: "\ntype Comparison {\n Wobble(String)\n}\n\npub fn main() {\n Wobble == Wobble\n}\n"
4+
---
5+
----- SOURCE CODE
6+
7+
type Comparison {
8+
Wobble(String)
9+
}
10+
11+
pub fn main() {
12+
Wobble == Wobble
13+
}
14+
15+
16+
----- WARNING
17+
warning: Redundant comparison
18+
┌─ /src/warning/wrn.gleam:7:3
19+
20+
7Wobble == Wobble
21+
│ ^^^^^^^^^^^^^^^^ This is always `False`
22+
23+
This comparison is redundant since it always fails.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
source: compiler-core/src/type_/tests/warnings.rs
3+
expression: "\ntype Comparison {\n Wobble(String)\n}\n\npub fn main() {\n Wobble != Wobble\n}\n"
4+
---
5+
----- SOURCE CODE
6+
7+
type Comparison {
8+
Wobble(String)
9+
}
10+
11+
pub fn main() {
12+
Wobble != Wobble
13+
}
14+
15+
16+
----- WARNING
17+
warning: Redundant comparison
18+
┌─ /src/warning/wrn.gleam:7:3
19+
20+
7Wobble != Wobble
21+
│ ^^^^^^^^^^^^^^^^ This is always `True`
22+
23+
This comparison is redundant since it always succeeds.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
source: compiler-core/src/type_/tests/warnings.rs
3+
expression: "\ntype Comparison {\n Wobble(String, Int)\n}\n\npub fn main() {\n Wobble == Wobble\n}\n"
4+
---
5+
----- SOURCE CODE
6+
7+
type Comparison {
8+
Wobble(String, Int)
9+
}
10+
11+
pub fn main() {
12+
Wobble == Wobble
13+
}
14+
15+
16+
----- WARNING
17+
warning: Redundant comparison
18+
┌─ /src/warning/wrn.gleam:7:3
19+
20+
7Wobble == Wobble
21+
│ ^^^^^^^^^^^^^^^^ This is always `False`
22+
23+
This comparison is redundant since it always fails.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
source: compiler-core/src/type_/tests/warnings.rs
3+
expression: "\ntype Comparison {\n Wibble\n}\n\npub fn main() {\n Wibble == Wibble\n}\n"
4+
---
5+
----- SOURCE CODE
6+
7+
type Comparison {
8+
Wibble
9+
}
10+
11+
pub fn main() {
12+
Wibble == Wibble
13+
}
14+
15+
16+
----- WARNING
17+
warning: Redundant comparison
18+
┌─ /src/warning/wrn.gleam:7:3
19+
20+
7Wibble == Wibble
21+
│ ^^^^^^^^^^^^^^^^ This is always `True`
22+
23+
This comparison is redundant since it always succeeds.

compiler-core/src/type_/tests/warnings.rs

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4612,3 +4612,78 @@ pub type Wobble
46124612
"#,
46134613
);
46144614
}
4615+
4616+
#[test]
4617+
fn redundant_comparison_zero_arity_constructor() {
4618+
assert_warning!(
4619+
"
4620+
type Comparison {
4621+
Wibble
4622+
}
4623+
4624+
pub fn main() {
4625+
Wibble == Wibble
4626+
}
4627+
"
4628+
);
4629+
}
4630+
4631+
#[test]
4632+
fn redundant_comparison_constructor_function() {
4633+
assert_warning!(
4634+
"
4635+
type Comparison {
4636+
Wobble(String)
4637+
}
4638+
4639+
pub fn main() {
4640+
Wobble == Wobble
4641+
}
4642+
"
4643+
);
4644+
}
4645+
4646+
#[test]
4647+
fn redundant_comparison_constructor() {
4648+
assert_warning!(
4649+
"
4650+
type Comparison {
4651+
Wobble(String)
4652+
}
4653+
4654+
pub fn main() {
4655+
Wobble(\"a\") == Wobble(\"a\")
4656+
}
4657+
"
4658+
);
4659+
}
4660+
4661+
#[test]
4662+
fn redundant_comparison_constructor_function_not_eq() {
4663+
assert_warning!(
4664+
"
4665+
type Comparison {
4666+
Wobble(String)
4667+
}
4668+
4669+
pub fn main() {
4670+
Wobble != Wobble
4671+
}
4672+
"
4673+
);
4674+
}
4675+
4676+
#[test]
4677+
fn redundant_comparison_multi_param_constructor_function() {
4678+
assert_warning!(
4679+
"
4680+
type Comparison {
4681+
Wobble(String, Int)
4682+
}
4683+
4684+
pub fn main() {
4685+
Wobble == Wobble
4686+
}
4687+
"
4688+
);
4689+
}

0 commit comments

Comments
 (0)