Skip to content

Commit 1a5ce62

Browse files
committed
Optimise comparison with singleton custom types on JavaScript #4903. also fixed some clippy errors
1 parent c1746fb commit 1a5ce62

File tree

7 files changed

+181
-18
lines changed

7 files changed

+181
-18
lines changed

compiler-core/src/analyse.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,10 @@ use vec1::Vec1;
5151

5252
use self::imports::Importer;
5353

54-
#[derive(Debug, Clone, PartialEq, Eq)]
54+
#[derive(Debug, Clone, PartialEq, Eq, Default)]
5555
pub enum Inferred<T> {
5656
Known(T),
57+
#[default]
5758
Unknown,
5859
}
5960

compiler-core/src/ast.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2473,12 +2473,6 @@ impl<T> BitArraySize<T> {
24732473
}
24742474
}
24752475

2476-
impl Default for Inferred<()> {
2477-
fn default() -> Self {
2478-
Self::Unknown
2479-
}
2480-
}
2481-
24822476
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
24832477
pub enum AssignName {
24842478
Variable(EcoString),

compiler-core/src/erlang.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -708,7 +708,7 @@ fn string_inner(value: &str) -> Document<'_> {
708708
.replace_all(value, |caps: &Captures<'_>| {
709709
let slashes = caps.get(1).map_or("", |m| m.as_str());
710710

711-
if slashes.len() % 2 == 0 {
711+
if slashes.len().is_multiple_of(2) {
712712
format!("{slashes}u")
713713
} else {
714714
format!("{slashes}x")

compiler-core/src/javascript/expression.rs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1536,11 +1536,62 @@ impl<'module, 'a> Generator<'module, 'a> {
15361536
return docvec![left_doc, operator, right_doc];
15371537
}
15381538

1539+
// Optimise comparison with singleton custom types on JavaScript (https://gleam-lang/gleam/issues/4903)
1540+
if let (
1541+
_,
1542+
// RHS: singleton record constructor (arity 0)
1543+
TypedExpr::Var {
1544+
constructor:
1545+
ValueConstructor {
1546+
variant: ValueConstructorVariant::Record { arity: 0, name, .. },
1547+
..
1548+
},
1549+
..
1550+
},
1551+
) = (left, right)
1552+
{
1553+
let left_doc = self
1554+
.not_in_tail_position(Some(Ordering::Strict), |this| this.wrap_expression(left));
1555+
1556+
let constructor_name = name.to_doc();
1557+
1558+
if should_be_equal {
1559+
return docvec![left_doc, " instanceof ", constructor_name];
1560+
} else {
1561+
return docvec!["!(", left_doc, " instanceof ", constructor_name, ")"];
1562+
}
1563+
}
1564+
1565+
if let (
1566+
// LHS: singleton record constructor (arity 0)
1567+
TypedExpr::Var {
1568+
constructor:
1569+
ValueConstructor {
1570+
variant: ValueConstructorVariant::Record { arity: 0, name, .. },
1571+
..
1572+
},
1573+
..
1574+
},
1575+
_,
1576+
) = (left, right)
1577+
{
1578+
let right_doc = self
1579+
.not_in_tail_position(Some(Ordering::Strict), |this| this.wrap_expression(right));
1580+
let constructor_name = name.to_doc();
1581+
1582+
if should_be_equal {
1583+
return docvec![right_doc, " instanceof ", constructor_name];
1584+
} else {
1585+
return docvec!["!(", right_doc, " instanceof ", constructor_name, ")"];
1586+
}
1587+
}
1588+
15391589
// Other types must be compared using structural equality
15401590
let left =
15411591
self.not_in_tail_position(Some(Ordering::Strict), |this| this.wrap_expression(left));
15421592
let right =
15431593
self.not_in_tail_position(Some(Ordering::Strict), |this| this.wrap_expression(right));
1594+
15441595
self.prelude_equal_call(should_be_equal, left, right)
15451596
}
15461597

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
use crate::assert_js;
2+
3+
#[test]
4+
fn singleton_record_equality() {
5+
assert_js!(
6+
r#"
7+
pub type Wibble {
8+
Wibble
9+
Wobble
10+
}
11+
12+
pub fn is_wibble(w: Wibble) -> Bool {
13+
w == Wibble
14+
}
15+
"#,
16+
);
17+
}
18+
19+
#[test]
20+
fn singleton_record_inequality() {
21+
assert_js!(
22+
r#"
23+
pub type Wibble {
24+
Wibble
25+
Wobble
26+
}
27+
28+
pub fn is_not_wibble(w: Wibble) -> Bool {
29+
w != Wibble
30+
}
31+
"#,
32+
);
33+
}
34+
35+
#[test]
36+
fn singleton_record_reverse_order() {
37+
assert_js!(
38+
r#"
39+
pub type Wibble {
40+
Wibble
41+
Wobble
42+
}
43+
44+
pub fn is_wibble_reverse(w: Wibble) -> Bool {
45+
Wibble == w
46+
}
47+
"#,
48+
);
49+
}
50+
51+
#[test]
52+
fn non_singleton_record_equality() {
53+
assert_js!(
54+
r#"
55+
pub type Person {
56+
Person(name: String, age: Int)
57+
}
58+
59+
pub fn same_person(p1: Person, p2: Person) -> Bool {
60+
p1 == p2
61+
}
62+
"#,
63+
);
64+
}
65+
66+
#[test]
67+
fn multiple_singleton_constructors() {
68+
assert_js!(
69+
r#"
70+
pub type Status {
71+
Loading
72+
Success
73+
Error
74+
}
75+
76+
pub fn is_loading(s: Status) -> Bool {
77+
s == Loading
78+
}
79+
80+
pub fn is_success(s: Status) -> Bool {
81+
s == Success
82+
}
83+
"#,
84+
);
85+
}
86+
87+
#[test]
88+
fn mixed_singleton_and_non_singleton() {
89+
assert_js!(
90+
r#"
91+
pub type Result {
92+
Ok(value: Int)
93+
Error
94+
}
95+
96+
pub fn is_error(r: Result) -> Bool {
97+
r == Error
98+
}
99+
"#,
100+
);
101+
}
102+
103+
#[test]
104+
fn singleton_in_case_guard() {
105+
assert_js!(
106+
r#"
107+
pub type State {
108+
Active
109+
Inactive
110+
}
111+
112+
pub fn process(s: State) -> String {
113+
case s {
114+
state if state == Active -> "active"
115+
_ -> "inactive"
116+
}
117+
}
118+
"#,
119+
);
120+
}

compiler-core/src/type_.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,10 +1375,13 @@ pub struct ValueConstructor {
13751375
pub type_: Arc<Type>,
13761376
}
13771377

1378-
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
1378+
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Default)]
13791379
pub enum Deprecation {
1380+
#[default]
13801381
NotDeprecated,
1381-
Deprecated { message: EcoString },
1382+
Deprecated {
1383+
message: EcoString,
1384+
},
13821385
}
13831386

13841387
impl Deprecation {
@@ -1391,12 +1394,6 @@ impl Deprecation {
13911394
}
13921395
}
13931396

1394-
impl Default for Deprecation {
1395-
fn default() -> Self {
1396-
Self::NotDeprecated
1397-
}
1398-
}
1399-
14001397
impl ValueConstructor {
14011398
pub fn local_variable(location: SrcSpan, origin: VariableOrigin, type_: Arc<Type>) -> Self {
14021399
Self {

compiler-core/src/type_/expression.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -924,7 +924,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
924924
// !!!True // we can remove all but one negation.
925925
// ```
926926
if negations > 1 {
927-
let location = if negations % 2 == 0 {
927+
let location = if negations.is_multiple_of(2) {
928928
SrcSpan {
929929
start: starting_location.start,
930930
end: location.start + 1,
@@ -1009,7 +1009,7 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
10091009
// ---1 // we can remove all but one negation.
10101010
// ```
10111011
if negations > 1 {
1012-
let location = if negations % 2 == 0 {
1012+
let location = if negations.is_multiple_of(2) {
10131013
SrcSpan {
10141014
start: starting_location.start,
10151015
end: end + 1,

0 commit comments

Comments
 (0)