nandi.uk
Fork of daniellemaywood.uk/gleam — Wasm codegen work
Address review comment
author
Giacomo Cavalieri
committer
Louis Pilfold
date
(Mar 30, 2024, 12:07 PM UTC)
commit
e0321757
e03217579e32a2564a9922e235035d8eb637578e
parent
d8192004
d8192004fa690bd2de78ff78b30f74e013716d11
+207
-54
+50
-29
compiler-core/src/ast/typed.rs
+50
-29
compiler-core/src/ast/typed.rs
···
376
376
}
377
377
378
378
pub fn is_literal(&self) -> bool {
379
-
matches!(
380
-
self,
379
+
match self {
381
380
Self::Int { .. }
382
-
| Self::List { .. }
383
-
| Self::Float { .. }
384
-
| Self::Tuple { .. }
385
-
| Self::String { .. }
386
-
| Self::BitArray { .. }
387
-
)
381
+
| Self::List { .. }
382
+
| Self::Float { .. }
383
+
| Self::Tuple { .. }
384
+
| Self::String { .. }
385
+
| Self::BitArray { .. } => true,
386
+
_ => false,
387
+
}
388
388
}
389
389
390
390
/// Returns `true` if the typed expr is [`Var`].
···
392
392
/// [`Var`]: TypedExpr::Var
393
393
#[must_use]
394
394
pub fn is_var(&self) -> bool {
395
-
matches!(self, Self::Var { .. })
395
+
match self {
396
+
Self::Var { .. } => true,
397
+
_ => false,
398
+
}
396
399
}
397
400
398
401
pub(crate) fn get_documentation(&self) -> Option<&str> {
···
444
447
}
445
448
}
446
449
447
-
pub fn is_binop(&self) -> bool {
450
+
pub fn is_pure_value_constructor(&self) -> bool {
448
451
match self {
449
-
Self::BinOp { .. } => true,
450
-
_ => false,
451
-
}
452
-
}
452
+
TypedExpr::Int { .. }
453
+
| TypedExpr::Float { .. }
454
+
| TypedExpr::String { .. }
455
+
| TypedExpr::List { .. }
456
+
| TypedExpr::Tuple { .. }
457
+
| TypedExpr::BitArray { .. }
458
+
| TypedExpr::Var { .. }
459
+
| TypedExpr::BinOp { .. }
460
+
| TypedExpr::RecordAccess { .. }
461
+
| TypedExpr::TupleIndex { .. }
462
+
| TypedExpr::RecordUpdate { .. }
463
+
| TypedExpr::ModuleSelect { .. }
464
+
| TypedExpr::Fn { .. } => true,
465
+
466
+
TypedExpr::NegateBool { value, .. } | TypedExpr::NegateInt { value, .. } => {
467
+
value.is_pure_value_constructor()
468
+
}
453
469
454
-
pub fn is_record_constructor(&self) -> bool {
455
-
match self {
456
-
Self::Call { fun, .. } => match fun.as_ref() {
457
-
TypedExpr::Var { constructor, .. } if constructor.variant.is_record() => true,
458
-
_ => false,
459
-
},
460
-
_ => false,
461
-
}
462
-
}
470
+
// A pipeline is a pure value constructor if its last step is a record builder.
471
+
// For example `wibble() |> wobble() |> Ok`
472
+
TypedExpr::Pipeline { finally, .. } => finally.is_record_builder(),
473
+
474
+
// A function call is a pure record constructor if it is a record builder.
475
+
// For example `Ok(wobble(wibble()))`
476
+
TypedExpr::Call { fun, .. } => fun.as_ref().is_record_builder(),
477
+
478
+
// Blocks and Cases are not considered pure value constructors for now,
479
+
// in the future we might want to do something a bit smarter and inspect
480
+
// their content to see if they end up returning something that is a
481
+
// pure value constructor and raise a warning for those as well.
482
+
TypedExpr::Block { .. } | TypedExpr::Case { .. } => false,
463
483
464
-
pub fn is_record_access(&self) -> bool {
465
-
match self {
466
-
Self::RecordAccess { .. } => true,
467
-
_ => false,
484
+
// `panic` and `todo` are never considered pure value constructors,
485
+
// we don't want to raise a warning for an unused value if it's one
486
+
// of those two.
487
+
TypedExpr::Todo { .. } | TypedExpr::Panic { .. } => false,
468
488
}
469
489
}
470
490
471
-
pub fn is_record_update(&self) -> bool {
491
+
pub fn is_record_builder(&self) -> bool {
472
492
match self {
473
-
Self::RecordUpdate { .. } => true,
493
+
TypedExpr::Call { fun, .. } => fun.is_record_builder(),
494
+
TypedExpr::Var { constructor, .. } => constructor.variant.is_record(),
474
495
_ => false,
475
496
}
476
497
}
+1
-5
compiler-core/src/type_/expression.rs
+1
-5
compiler-core/src/type_/expression.rs
···
397
397
.emit(Warning::ImplicitlyDiscardedResult {
398
398
location: discarded.location(),
399
399
});
400
-
} else if discarded.is_binop()
401
-
|| discarded.is_record_constructor()
402
-
|| discarded.is_record_access()
403
-
|| discarded.is_record_update()
404
-
{
400
+
} else if discarded.is_pure_value_constructor() {
405
401
self.environment.warnings.emit(Warning::UnusedValue {
406
402
location: discarded.location(),
407
403
})
-2
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_binary_operation_raises_a_warning.snap
-2
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_binary_operation_raises_a_warning.snap
+9
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_bool_negation_raises_a_warning.snap
+9
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_bool_negation_raises_a_warning.snap
+9
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_function_literal_raises_a_warning.snap
+9
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_function_literal_raises_a_warning.snap
+11
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_int_negation_raises_a_warning.snap
+11
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_int_negation_raises_a_warning.snap
+9
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_pipeline_ending_with_variant_raises_a_warning.snap
+9
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_pipeline_ending_with_variant_raises_a_warning.snap
···
1
+
---
2
+
source: compiler-core/src/type_/tests/warnings.rs
3
+
expression: "\npub type Wibble(a) { Wibble(a) }\npub fn wibble(a) { a }\n\npub fn main() {\n 1 |> wibble |> Wibble\n 1\n}\n"
4
+
---
5
+
warning: Unused value
6
+
┌─ /src/warning/wrn.gleam:6:3
7
+
│
8
+
6 │ 1 |> wibble |> Wibble
9
+
│ ^^^^^^^^^^^^^^^^^^^^^ This value is never used
-2
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_record_access_raises_a_warning.snap
-2
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_record_access_raises_a_warning.snap
-2
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_record_constructor_raises_a_warning.snap
-2
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_record_constructor_raises_a_warning.snap
-2
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_record_update_raises_a_warning.snap
-2
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_record_update_raises_a_warning.snap
+9
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_tuple_index_raises_a_warning.snap
+9
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_tuple_index_raises_a_warning.snap
+9
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_variable_raises_a_warning.snap
+9
compiler-core/src/type_/tests/snapshots/gleam_core__type___tests__warnings__unused_variable_raises_a_warning.snap
+91
compiler-core/src/type_/tests/warnings.rs
+91
compiler-core/src/type_/tests/warnings.rs
···
1269
1269
"#
1270
1270
);
1271
1271
}
1272
+
1273
+
#[test]
1274
+
fn unused_variable_raises_a_warning() {
1275
+
assert_warning!(
1276
+
r#"
1277
+
pub fn main() {
1278
+
let number = 1
1279
+
number
1280
+
1
1281
+
}
1282
+
"#
1283
+
);
1284
+
}
1285
+
1286
+
#[test]
1287
+
fn unused_function_literal_raises_a_warning() {
1288
+
assert_warning!(
1289
+
r#"
1290
+
pub fn main() {
1291
+
fn(n) { n + 1 }
1292
+
1
1293
+
}
1294
+
"#
1295
+
);
1296
+
}
1297
+
1298
+
#[test]
1299
+
fn unused_tuple_index_raises_a_warning() {
1300
+
assert_warning!(
1301
+
r#"
1302
+
pub fn main() {
1303
+
#(1, 2).0
1304
+
1
1305
+
}
1306
+
"#
1307
+
);
1308
+
}
1309
+
1310
+
#[test]
1311
+
fn unused_bool_negation_raises_a_warning() {
1312
+
assert_warning!(
1313
+
r#"
1314
+
pub fn main() {
1315
+
!True
1316
+
1
1317
+
}
1318
+
"#
1319
+
);
1320
+
}
1321
+
1322
+
#[test]
1323
+
fn unused_int_negation_raises_a_warning() {
1324
+
assert_warning!(
1325
+
r#"
1326
+
pub fn main() {
1327
+
-1
1328
+
1
1329
+
}
1330
+
"#
1331
+
);
1332
+
}
1333
+
1334
+
#[test]
1335
+
fn unused_pipeline_ending_with_variant_raises_a_warning() {
1336
+
assert_warning!(
1337
+
r#"
1338
+
pub type Wibble(a) { Wibble(a) }
1339
+
pub fn wibble(a) { a }
1340
+
1341
+
pub fn main() {
1342
+
1 |> wibble |> Wibble
1343
+
1
1344
+
}
1345
+
"#
1346
+
);
1347
+
}
1348
+
1349
+
#[test]
1350
+
fn unused_pipeline_not_ending_with_variant_raises_no_warnings() {
1351
+
assert_no_warnings!(
1352
+
r#"
1353
+
pub type Wibble(a) { Wibble(a) }
1354
+
pub fn wibble(a) { a }
1355
+
1356
+
pub fn main() {
1357
+
1 |> wibble |> wibble
1358
+
1
1359
+
}
1360
+
"#
1361
+
);
1362
+
}
+1
-1
compiler-core/src/warning.rs
+1
-1
compiler-core/src/warning.rs
···
638
638
type_::Warning::UnusedValue { location } => Diagnostic {
639
639
title: "Unused value".into(),
640
640
text: "".into(),
641
-
hint: Some("You can safely remove it.".into()),
641
+
hint: None,
642
642
level: diagnostic::Level::Warning,
643
643
location: Some(Location {
644
644
path: path.to_path_buf(),
+4
-4
test-package-compiler/cases/imported_external_fns/src/two.gleam
+4
-4
test-package-compiler/cases/imported_external_fns/src/two.gleam
···
12
12
const const_unqualified_aliased = xthing
13
13
14
14
pub fn the_consts() {
15
-
const_qualified
16
-
const_qualified_aliased
17
-
const_unqualified
18
-
const_unqualified_aliased
15
+
let _ = const_qualified
16
+
let _ = const_qualified_aliased
17
+
let _ = const_unqualified
18
+
let _ = const_unqualified_aliased
19
19
const_qualified()
20
20
const_qualified_aliased()
21
21
const_unqualified()
+4
-7
test-package-compiler/src/snapshots/test_package_compiler__generated_tests__imported_external_fns.snap
+4
-7
test-package-compiler/src/snapshots/test_package_compiler__generated_tests__imported_external_fns.snap
···
65
65
66
66
-spec the_consts() -> nil.
67
67
the_consts() ->
68
-
fun thing:new/0,
69
-
fun thing:new/0,
70
-
fun thing:new/0,
71
-
fun thing:new/0,
68
+
_ = fun thing:new/0,
69
+
_ = fun thing:new/0,
70
+
_ = fun thing:new/0,
71
+
_ = fun thing:new/0,
72
72
thing:new(),
73
73
thing:new(),
74
74
thing:new(),
···
84
84
two]},
85
85
{registered, []}
86
86
]}.
87
-
88
-
89
-