Skip to content

Commit 5175a40

Browse files
GearsDatapackslpil
authored andcommitted
Remove unnecessary assignments for blocks when the value is unused
1 parent f1f45d1 commit 5175a40

7 files changed

+120
-33
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,10 @@
143143

144144
([Surya Rose](https://github.com/GearsDatapacks))
145145

146+
- The code generated for blocks on the JavaScript target has been improved and
147+
is now smaller in certain cases.
148+
([Surya Rose](https://github.com/GearsDatapacks))
149+
146150
### Build tool
147151

148152
- New projects are generated using OTP28 on GitHub Actions.

compiler-core/src/javascript/decision.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -718,7 +718,7 @@ pub fn let_<'a>(
718718
];
719719

720720
match scope_position {
721-
expression::Position::NotTail(_ordering) => doc,
721+
expression::Position::Expression(_) | expression::Position::Statement => doc,
722722
expression::Position::Tail => docvec![doc, line(), "return ", assignment_name, ";"],
723723
expression::Position::Assign(variable) => {
724724
docvec![doc, line(), variable, " = ", assignment_name, ";"]

compiler-core/src/javascript/expression.rs

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,15 @@ use std::sync::Arc;
1515

1616
#[derive(Debug, Clone)]
1717
pub enum Position {
18+
/// We are compiling the last expression in a function, meaning that it should
19+
/// use `return` to return the value it produces from the function.
1820
Tail,
19-
NotTail(Ordering),
21+
/// We are inside a function, but the value of this expression isn't being
22+
/// used, so we don't need to do anything with the returned value.
23+
Statement,
24+
/// The value of this expression needs to be used inside another expression,
25+
/// so we need to use the value that is returned by this expression.
26+
Expression(Ordering),
2027
/// We are compiling an expression inside a block, meaning we must assign
2128
/// to the `_block` variable at the end of the scope, because blocks are not
2229
/// expressions in JS.
@@ -38,8 +45,8 @@ impl Position {
3845
#[must_use]
3946
pub fn ordering(&self) -> Ordering {
4047
match self {
41-
Self::NotTail(ordering) => *ordering,
42-
Self::Tail | Self::Assign(_) => Ordering::Loose,
48+
Self::Expression(ordering) => *ordering,
49+
Self::Tail | Self::Assign(_) | Self::Statement => Ordering::Loose,
4350
}
4451
}
4552
}
@@ -518,7 +525,7 @@ impl<'module, 'a> Generator<'module, 'a> {
518525
pub fn wrap_return(&mut self, document: Document<'a>) -> Document<'a> {
519526
match &self.scope_position {
520527
Position::Tail => docvec!["return ", document, ";"],
521-
Position::NotTail(_) => document,
528+
Position::Expression(_) | Position::Statement => document,
522529
Position::Assign(name) => docvec![name.clone(), " = ", document, ";"],
523530
}
524531
}
@@ -536,10 +543,12 @@ impl<'module, 'a> Generator<'module, 'a> {
536543
{
537544
let new_ordering = ordering.unwrap_or(self.scope_position.ordering());
538545

539-
let function_position =
540-
std::mem::replace(&mut self.function_position, Position::NotTail(new_ordering));
546+
let function_position = std::mem::replace(
547+
&mut self.function_position,
548+
Position::Expression(new_ordering),
549+
);
541550
let scope_position =
542-
std::mem::replace(&mut self.scope_position, Position::NotTail(new_ordering));
551+
std::mem::replace(&mut self.scope_position, Position::Expression(new_ordering));
543552

544553
let result = compile(self);
545554

@@ -562,7 +571,7 @@ impl<'module, 'a> Generator<'module, 'a> {
562571
record_assignment: Some(_),
563572
..
564573
},
565-
Position::NotTail(Ordering::Loose),
574+
Position::Expression(Ordering::Loose),
566575
) => self.wrap_block(|this| this.expression(expression)),
567576
(
568577
TypedExpr::Panic { .. }
@@ -574,7 +583,7 @@ impl<'module, 'a> Generator<'module, 'a> {
574583
record_assignment: Some(_),
575584
..
576585
},
577-
Position::NotTail(Ordering::Strict),
586+
Position::Expression(Ordering::Strict),
578587
) => self.immediately_invoked_function_expression(expression, |this, expr| {
579588
this.expression(expr)
580589
}),
@@ -597,8 +606,8 @@ impl<'module, 'a> Generator<'module, 'a> {
597606
match &self.scope_position {
598607
// Here the document is a return statement: `return <expr>;`
599608
// or an assignment: `_block = <expr>;`
600-
Position::Tail | Position::Assign(_) => document,
601-
Position::NotTail(_) => docvec!["(", document, ")"],
609+
Position::Tail | Position::Assign(_) | Position::Statement => document,
610+
Position::Expression(_) => docvec!["(", document, ")"],
602611
}
603612
}
604613

@@ -644,7 +653,7 @@ impl<'module, 'a> Generator<'module, 'a> {
644653
);
645654
let function_position = std::mem::replace(
646655
&mut self.function_position,
647-
Position::NotTail(Ordering::Strict),
656+
Position::Expression(Ordering::Strict),
648657
);
649658

650659
// Generate the expression
@@ -774,12 +783,14 @@ impl<'module, 'a> Generator<'module, 'a> {
774783
}
775784
}
776785
match &self.scope_position {
777-
Position::Tail | Position::Assign(_) => self.block_document(statements),
778-
Position::NotTail(Ordering::Strict) => self
786+
Position::Tail | Position::Assign(_) | Position::Statement => {
787+
self.block_document(statements)
788+
}
789+
Position::Expression(Ordering::Strict) => self
779790
.immediately_invoked_function_expression(statements, |this, statements| {
780791
this.statements(statements)
781792
}),
782-
Position::NotTail(Ordering::Loose) => self.wrap_block(|this| {
793+
Position::Expression(Ordering::Loose) => self.wrap_block(|this| {
783794
// Save previous scope
784795
let current_scope_vars = this.current_scope_vars.clone();
785796

@@ -806,11 +817,16 @@ impl<'module, 'a> Generator<'module, 'a> {
806817
let mut documents = Vec::with_capacity(count * 3);
807818
for (i, statement) in statements.iter().enumerate() {
808819
if i + 1 < count {
809-
documents.push(
810-
self.not_in_tail_position(Some(Ordering::Loose), |this| {
811-
this.statement(statement)
812-
}),
813-
);
820+
let function_position =
821+
std::mem::replace(&mut self.function_position, Position::Statement);
822+
let scope_position =
823+
std::mem::replace(&mut self.scope_position, Position::Statement);
824+
825+
documents.push(self.statement(statement));
826+
827+
self.function_position = function_position;
828+
self.scope_position = scope_position;
829+
814830
if requires_semicolon(statement) {
815831
documents.push(";".to_doc());
816832
}
@@ -845,7 +861,7 @@ impl<'module, 'a> Generator<'module, 'a> {
845861
let js_name = self.next_local_var(name);
846862
let assignment = docvec!["let ", js_name.clone(), " = ", subject, ";"];
847863
let assignment = match &self.scope_position {
848-
Position::NotTail(_) => assignment,
864+
Position::Expression(_) | Position::Statement => assignment,
849865
Position::Tail => docvec![assignment, line(), "return ", js_name, ";"],
850866
Position::Assign(block_variable) => docvec![
851867
assignment,
@@ -900,7 +916,7 @@ impl<'module, 'a> Generator<'module, 'a> {
900916
});
901917

902918
match &self.scope_position {
903-
Position::NotTail(_) => check,
919+
Position::Expression(_) | Position::Statement => check,
904920
Position::Tail | Position::Assign(_) => {
905921
docvec![check, line(), self.wrap_return("undefined".to_doc())]
906922
}

compiler-core/src/javascript/tests/blocks.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,3 +344,27 @@ pub fn main() {
344344
"#
345345
)
346346
}
347+
348+
#[test]
349+
fn blocks_whose_values_are_unused_do_not_generate_assignments() {
350+
// There's no point generating `_block` assignments here, as the values
351+
// would be unused.
352+
assert_js!(
353+
"
354+
pub fn main() {
355+
{
356+
let x = 10
357+
echo x
358+
}
359+
360+
{
361+
let a = 1
362+
let b = 2
363+
a + b
364+
}
365+
366+
Nil
367+
}
368+
"
369+
);
370+
}

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__blocks__blocks_returning_functions.snap

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
---
22
source: compiler-core/src/javascript/tests/blocks.rs
3-
assertion_line: 172
43
expression: "\npub fn b() {\n {\n fn(cb) { cb(1) }\n }\n {\n fn(cb) { cb(2) }\n }\n 3\n}\n"
5-
snapshot_kind: text
64
---
75
----- SOURCE CODE
86

@@ -19,7 +17,7 @@ pub fn b() {
1917

2018
----- COMPILED JAVASCRIPT
2119
export function b() {
22-
((cb) => { return cb(1); });
23-
((cb) => { return cb(2); });
20+
(cb) => { return cb(1); };
21+
(cb) => { return cb(2); };
2422
return 3;
2523
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
source: compiler-core/src/javascript/tests/blocks.rs
3+
expression: "\npub fn main() {\n {\n let x = 10\n echo x\n }\n\n {\n let a = 1\n let b = 2\n a + b\n }\n\n Nil\n}\n"
4+
---
5+
----- SOURCE CODE
6+
7+
pub fn main() {
8+
{
9+
let x = 10
10+
echo x
11+
}
12+
13+
{
14+
let a = 1
15+
let b = 2
16+
a + b
17+
}
18+
19+
Nil
20+
}
21+
22+
23+
----- COMPILED JAVASCRIPT
24+
import * as $stdlib$dict from "../../gleam_stdlib/dict.mjs";
25+
import {
26+
Empty as $Empty,
27+
NonEmpty as $NonEmpty,
28+
CustomType as $CustomType,
29+
bitArraySlice,
30+
bitArraySliceToInt,
31+
BitArray as $BitArray,
32+
List as $List,
33+
UtfCodepoint as $UtfCodepoint,
34+
} from "../gleam.mjs";
35+
36+
export function main() {
37+
{
38+
let x = 10;
39+
echo(x, undefined, "src/module.gleam", 5)
40+
};
41+
{
42+
let a = 1;
43+
let b = 2;
44+
a + b
45+
};
46+
return undefined;
47+
}
48+
49+
// ...omitted code from `templates/echo.mjs`...

compiler-core/src/javascript/tests/snapshots/gleam_core__javascript__tests__blocks__nested_multiexpr_non_ending_blocks.snap

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
---
22
source: compiler-core/src/javascript/tests/blocks.rs
3-
assertion_line: 75
43
expression: "\npub fn go() {\n let x = {\n 1\n {\n 2\n 3\n }\n 4\n }\n x\n}\n"
5-
snapshot_kind: text
64
---
75
----- SOURCE CODE
86

@@ -24,12 +22,10 @@ export function go() {
2422
let _block;
2523
{
2624
1;
27-
let _block$1;
2825
{
2926
2;
30-
_block$1 = 3;
31-
}
32-
_block$1;
27+
3
28+
};
3329
_block = 4;
3430
}
3531
let x = _block;

0 commit comments

Comments
 (0)