[Compiler-V2] Check Unused Assignments #13543

fEst1ck · 2024-06-04T08:15:24Z

Description

Implements a stackless bytecode pipeline checking for unused assignments and giving warnings to the user.

Addresses #11710.

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

trunk-io · 2024-06-04T08:15:28Z

⏱️ 6h 15m total CI duration on this PR

Job	Cumulative Duration	Recent Runs
rust-targeted-unit-tests	1h 55m	⬜ 🟥 🟥 ⬜ 🟥 (+3 more)
rust-move-tests	1h 11m	⬜ 🟩 🟩 ⬜ 🟩 (+3 more)
rust-move-unit-coverage	1h 7m	⬜ 🟩 🟩 ⬜ 🟩 (+3 more)
run-tests-main-branch	52m	🟩 🟩 🟩 🟩 🟩 (+6 more)
rust-lints	39m	⬜ 🟩 🟩 🟥 🟩 (+3 more)
check-dynamic-deps	12m	🟩 🟩 🟩 🟩 🟩 (+2 more)
general-lints	11m	🟩 🟩 🟩 🟩 🟩 (+2 more)
semgrep/ci	3m	🟩 🟩 🟩 🟩 🟩 (+2 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+2 more)
file_change_determinator	1m	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	28s	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	25s	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	18s	🟩 🟩 🟩 🟩 🟩 (+2 more)
permission-check	18s	🟩 🟩 🟩 🟩 🟩 (+2 more)

🚨 2 jobs on the last run were significantly faster/slower than expected

Job	Duration	vs 7d avg	Delta
rust-move-tests	13m	9m
rust-targeted-unit-tests	24m	18m

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

fEst1ck · 2024-06-05T06:30:04Z

third_party/move/move-compiler-v2/tests/unused-assignment/unused_assignment.exp

+warning: Unused assignment to `w`. Consider removing or prefixing with an underscore: `_w`
+  ┌─ tests/unused-assignment/unused_assignment.move:7:17
+  │
+7 │         let w = bar(false);


The location here is wrong because this is generated at see line 123

5: $t5 := test::bar($t6) # live vars:

where $t5 is assigned but not used later. However, the location of this instruction has location bar(false) which is the RHS of the original assignment let w = bar(false);.

This perhaps can be solved by my previous PR which adds location for the arguments of the stackless bytecode. When we do this check, we haven't done any transformations.

fEst1ck · 2024-06-05T06:35:33Z

third_party/move/move-compiler-v2/tests/unused-assignment/unused_assignment.exp

+warning: Unused local variable `y`. Consider removing or prefixing with an underscore: `_y`
+  ┌─ tests/unused-assignment/unused_assignment.move:5:13
+  │
+5 │         let y = 0;


Note that this warning is generated by @brianrmurphy 's flow_insensitive_checker, and NOT by our checker. Upon looking at the generated bytecode below, I believe this assignment has been optimized out on the AST level, and so we can't see it in the bytecode here.

vineethk

We need more tests. We can have unused assignments through pattern destructuring, unused assignment in a branch that is later never used, assignment in branch that is later used, unused assignment to parameters (parameters and locals are handled a bit differently, so this is always good to test) etc.

One rich source of tests is the following. Search for "unused assignment" in all *.exp files. You will find several test cases in the old compiler that should all be ported over, and the testdiff tool should updated, update_v1_diff.sh should be run to update the test diff files showing these tests being ported.

vineethk · 2024-06-05T18:48:32Z

third_party/move/move-compiler-v2/src/pipeline/unused_assignment_checker.rs

+        id: AttrId,
+        offset: CodeOffset,
+        dst: TempIndex,
+        after: bool,


Why do we need after? All uses of this method seem to pass true to this parameter.

done. (A previous commit used this when checking for unused parameters.)

vineethk · 2024-06-05T18:50:44Z

third_party/move/move-compiler-v2/src/pipeline/unused_assignment_checker.rs

+    ) {
+        let data = target.data;
+        // only check for user defined variables
+        if let Some(dst_name) = data.local_names.get(&dst) {


Does this include parameters to a function? Can you please include a test that has unused assignment to a parameter?

Which test is it? I seem to not be able to find such a test.

Any answer?

Sorry, I missed this comment. See for example third_party/move/move-compiler-v2/tests/unused-assignment/unused_call_assign_shadow.exp. The flow-insensitive checker warns unused parameters; the unused assignment checker does NOT warn them, because the parameters are always inferred to be alive before the first instruction by the current analysis, even if they are not used anywhere at all.

@fEst1ck I still don't see it, and maybe I was not clear about what I was asking, I have tried to make it concrete below:

We need to have a function foo with a parameter x. We then assign a value to x in the function foo. After this assignment, there should be no read of x (thus, there is an unused assignment to a parameter). I do not think the test case you point to (or others, at the time I inspected them) has a similar case.

@vineethk Thanks for the clarification. I added a new test third_party/move/move-compiler-v2/tests/unused-assignment/unused_assign_to_param.move for this.

vineethk · 2024-06-05T18:53:45Z

third_party/move/move-compiler-v2/src/pipeline/unused_assignment_checker.rs

+            let dst_name = dst_name.display(target.func_env.symbol_pool()).to_string();
+            if !dst_name.starts_with('_') && live_var_info.get(&dst).is_none() {
+                let loc = target.get_bytecode_loc(id);
+                target.global_env().diag(Severity::Warning, &loc, &format!("Unused assignment to `{}`. Consider removing or prefixing with an underscore: `_{}`", dst_name, dst_name));


Seems like an unusually long line, maybe rustfmt did not break it up conservatively because of the string involved?

Manually edited.

vineethk · 2024-06-05T19:00:30Z

third_party/move/move-compiler-v2/tests/testsuite.rs

+                .set_experiment(Experiment::UNUSED_ASSIGNMENT_CHECK, true),
+            stop_after: StopAfter::BytecodePipeline(Some("UnusedAssignmentChecker")),
+            dump_ast: DumpLevel::None,
+            dump_bytecode: DumpLevel::EndStage,


Do we need the bytecode to be dumped for these tests? It seems like a distraction from reviewing the warnings.

Done. Was using it for debugging.

fEst1ck · 2024-06-06T22:36:37Z

@vineethk Thanks for the comments! All addressed now.

fEst1ck · 2024-06-06T22:40:40Z

@wrwg We cannot remove @brianrmurphy 's check implemented in flow_insensitive_checker.rs because 1) some unused assignments are optimized out by the AST simplified (but are warned by Brian's check), 2) for unused parameters, the AST level check provides better location.

vineethk · 2024-06-10T15:52:08Z

.../move/move-compiler-v2/tests/unused-assignment/v1-locals/unused_resource_explicit_return.exp

+
+
+Diagnostics:
+warning: Unused assignment to `r`. Consider removing or prefixing with an underscore: `_r`


Hmm. Looks like we are duplicating warnings (although they are of different "types", to a user, they are very similar). What does v1 do in these cases? IMO, we should not have a unused assignment warning if for the same location we have already provided unused variable warning.

This should be avoided, but the solution is probably not trivial.

Submitted issue #13744 tracking this.

vineethk · 2024-06-10T16:02:16Z

We need more tests. We can have unused assignments through pattern destructuring, unused assignment in a branch that is later never used, assignment in branch that is later used, unused assignment to parameters (parameters and locals are handled a bit differently, so this is always good to test) etc.

One rich source of tests is the following. Search for "unused assignment" in all *.exp files. You will find several test cases in the old compiler that should all be ported over, and the testdiff tool should updated, update_v1_diff.sh should be run to update the test diff files showing these tests being ported.

@fEst1ck I can't seem to find tests that have unused assignments through pattern destructuring (let S { a, b } = foo();).

fEst1ck · 2024-06-18T08:04:31Z

We need more tests. We can have unused assignments through pattern destructuring, unused assignment in a branch that is later never used, assignment in branch that is later used, unused assignment to parameters (parameters and locals are handled a bit differently, so this is always good to test) etc.
One rich source of tests is the following. Search for "unused assignment" in all *.exp files. You will find several test cases in the old compiler that should all be ported over, and the testdiff tool should updated, update_v1_diff.sh should be run to update the test diff files showing these tests being ported.

@fEst1ck I can't seem to find tests that have unused assignments through pattern destructuring (let S { a, b } = foo();).

done

brmataptos · 2024-06-18T21:05:48Z

We need more tests. We can have unused assignments through pattern destructuring, unused assignment in a branch that is later never used, assignment in branch that is later used, unused assignment to parameters (parameters and locals are handled a bit differently, so this is always good to test) etc.
One rich source of tests is the following. Search for "unused assignment" in all *.exp files. You will find several test cases in the old compiler that should all be ported over, and the testdiff tool should updated, update_v1_diff.sh should be run to update the test diff files showing these tests being ported.

@fEst1ck I can't seem to find tests that have unused assignments through pattern destructuring (let S { a, b } = foo();).

done

See the following tests from move-compiler:

move-compiler/tests/move_check/translated_ir_tests/move/commands/mixed_lvalue.move
move-compiler/tests/move_check/locals/assign_partial_resource.move
move-compiler/tests/move_check/locals/assign_resource.move
move-compiler/tests/move_check/locals/struct_use_before_assign.move
move-compiler/tests/move_check/locals/unused_resource.move
move-compiler/tests/move_check/liveness/unused_assignment.move
move-compiler/tests/move_check/locals/struct_use_before_assign.move
move-compiler/tests/move_check/locals/unused_resource_explicit_return.move

fEst1ck · 2024-06-19T18:59:52Z

We need more tests. We can have unused assignments through pattern destructuring, unused assignment in a branch that is later never used, assignment in branch that is later used, unused assignment to parameters (parameters and locals are handled a bit differently, so this is always good to test) etc.
One rich source of tests is the following. Search for "unused assignment" in all *.exp files. You will find several test cases in the old compiler that should all be ported over, and the testdiff tool should updated, update_v1_diff.sh should be run to update the test diff files showing these tests being ported.

@fEst1ck I can't seem to find tests that have unused assignments through pattern destructuring (let S { a, b } = foo();).

done

See the following tests from move-compiler:
move-compiler/tests/move_check/translated_ir_tests/move/commands/mixed_lvalue.move
move-compiler/tests/move_check/locals/assign_partial_resource.move
move-compiler/tests/move_check/locals/assign_resource.move
move-compiler/tests/move_check/locals/struct_use_before_assign.move
move-compiler/tests/move_check/locals/unused_resource.move
move-compiler/tests/move_check/liveness/unused_assignment.move
move-compiler/tests/move_check/locals/struct_use_before_assign.move
move-compiler/tests/move_check/locals/unused_resource_explicit_return.move

Imported in V2.

fEst1ck · 2024-06-19T19:00:11Z

All comments addressed. PTAL @vineethk @brmataptos

brmataptos · 2024-06-21T22:56:12Z

third_party/move/move-compiler-v2/tests/unused-assignment/v1-commands/mixed_lvalue.move

+
+    public fun mixed() {
+        let r = 0;
+        let r_ref = &mut r;


Should have unused assignment

There is an "invalid assignment" warning in the front end, so we never reach the phase of unused assignment checking here.

brmataptos · 2024-06-21T22:56:21Z

third_party/move/move-compiler-v2/tests/unused-assignment/v1-commands/mixed_lvalue.move

+    public fun mixed() {
+        let r = 0;
+        let r_ref = &mut r;
+        let s = S { f: 0 };


unused assignment here, too.

brmataptos · 2024-06-21T23:14:11Z

third_party/move/move-compiler-v2/src/pipeline/unused_assignment_checker.rs

+    ) {
+        let data = target.data;
+        // only check for user defined variables
+        if let Some(dst_name) = data.local_names.get(&dst) {


Any answer?

brmataptos · 2024-06-21T23:18:18Z

third_party/move/move-compiler-v2/src/pipeline/unused_assignment_checker.rs

+            if !dst_name.starts_with('_') && live_after.get(&dst).is_none() {
+                let loc = target.get_bytecode_loc(id);
+                target
+					.global_env()


no tab characters, please.

brmataptos · 2024-06-21T23:20:14Z

third_party/move/move-compiler-v2/tests/ability-transform/borrowed_from_one_path.exp

@@ -71,6 +71,50 @@ fun m::f($t0: u8, $t1: &vector<u64>): u64 {
 14: return $t2
 }

+============ after LiveVarAnalysisProcessor: ================


Why are we printing debugging info in test outputs?

It's the original test configuration for the ability transformer. We have this new debugging info since we run one more live var analysis.

brmataptos · 2024-06-21T23:25:35Z

third_party/move/move-compiler-v2/tests/bytecode-generator/wildcard7.exp

@@ -42,6 +42,14 @@ public fun m::test(): u8 {
 }


+Diagnostics:
+warning: Unused assignment to `q`. Consider removing or prefixing with an underscore: `_q`


This warning is redundant. We could suppress in the case of no uses, in favor of "unused variable". But it's simpler to remove the declaration when the "unused variable" warning is generated. Filed #13791 to track that.

I explored removing the unused declaration back in the AST and it's pretty messy. Would be simpler to suppress the redundant warning here in the case of a variable declaration. Perhaps that will require a change to the bytecode to track whether an assignment/loc corresponds to a variable declaration.

fEst1ck · 2024-06-24T20:11:23Z

All comments addressed. PTAL @brmataptos

I missed your comments on the test for unused function parameters. Please see the reply above @vineethk

github-actions · 2024-06-28T03:55:37Z

✅ Forge suite `realistic_env_max_load` success on `45054712bb53685761d4b8f5677fb5ca6b4467ce`

two traffics test: inner traffic : committed: 8686.389644776496 txn/s, latency: 4509.443668494171 ms, (p50: 4400 ms, p90: 5400 ms, p99: 9600 ms), latency samples: 3757400
two traffics test : committed: 99.94734286841467 txn/s, latency: 2041.1488636363636 ms, (p50: 2000 ms, p90: 2200 ms, p99: 5400 ms), latency samples: 1760
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.217, avg: 0.209", "QsPosToProposal: max: 0.291, avg: 0.239", "ConsensusProposalToOrdered: max: 0.310, avg: 0.286", "ConsensusOrderedToCommit: max: 0.360, avg: 0.346", "ConsensusProposalToCommit: max: 0.642, avg: 0.632"]
Max round gap was 1 [limit 4] at version 1767859. Max no progress secs was 5.003106 [limit 15] at version 1767859.
Test Ok

github-actions · 2024-06-28T03:56:15Z

✅ Forge suite `compat` success on `f648076a280621dbfd4e73b1ca83e3a3f52878ed` ==> `45054712bb53685761d4b8f5677fb5ca6b4467ce`

Compatibility test results for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 45054712bb53685761d4b8f5677fb5ca6b4467ce (PR)
1. Check liveness of validators at old version: f648076a280621dbfd4e73b1ca83e3a3f52878ed
compatibility::simple-validator-upgrade::liveness-check : committed: 4193.801738972211 txn/s, latency: 7309.977463134543 ms, (p50: 6800 ms, p90: 12800 ms, p99: 15400 ms), latency samples: 153260
2. Upgrading first Validator to new version: 45054712bb53685761d4b8f5677fb5ca6b4467ce
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 3401.3443716143843 txn/s, latency: 7127.351930803571 ms, (p50: 8400 ms, p90: 9700 ms, p99: 11200 ms), latency samples: 89600
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 3300.003525350022 txn/s, latency: 9453.440878328674 ms, (p50: 9400 ms, p90: 14800 ms, p99: 15100 ms), latency samples: 135940
3. Upgrading rest of first batch to new version: 45054712bb53685761d4b8f5677fb5ca6b4467ce
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 3604.1467036441627 txn/s, latency: 7259.904322244852 ms, (p50: 8800 ms, p90: 9700 ms, p99: 10000 ms), latency samples: 88380
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 3248.614830302983 txn/s, latency: 9586.05591021717 ms, (p50: 9600 ms, p90: 14800 ms, p99: 15100 ms), latency samples: 137220
4. upgrading second batch to new version: 45054712bb53685761d4b8f5677fb5ca6b4467ce
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 3966.5252119297497 txn/s, latency: 6006.68093196112 ms, (p50: 4400 ms, p90: 13700 ms, p99: 16500 ms), latency samples: 104940
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 6104.419525962199 txn/s, latency: 5298.48018744551 ms, (p50: 4900 ms, p90: 9500 ms, p99: 10400 ms), latency samples: 229400
5. check swarm health
Compatibility test for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 45054712bb53685761d4b8f5677fb5ca6b4467ce passed
Test Ok

github-actions · 2024-06-28T03:57:51Z

✅ Forge suite `framework_upgrade` success on `f648076a280621dbfd4e73b1ca83e3a3f52878ed` ==> `45054712bb53685761d4b8f5677fb5ca6b4467ce`

Compatibility test results for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 45054712bb53685761d4b8f5677fb5ca6b4467ce (PR)
Upgrade the nodes to version: 45054712bb53685761d4b8f5677fb5ca6b4467ce
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1212.4517523424004 txn/s, submitted: 1213.7846051298784 txn/s, failed submission: 1.3328527874779046 txn/s, expired: 1.3328527874779046 txn/s, latency: 2474.9159673873214 ms, (p50: 1800 ms, p90: 4500 ms, p99: 6900 ms), latency samples: 109160
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1149.2443547064197 txn/s, submitted: 1151.4347823202183 txn/s, failed submission: 2.190427613798767 txn/s, expired: 2.190427613798767 txn/s, latency: 2823.004320203304 ms, (p50: 1800 ms, p90: 5000 ms, p99: 11900 ms), latency samples: 94440
5. check swarm health
Compatibility test for f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 45054712bb53685761d4b8f5677fb5ca6b4467ce passed
Upgrade the remaining nodes to version: 45054712bb53685761d4b8f5677fb5ca6b4467ce
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1168.6552803680677 txn/s, submitted: 1171.5310173820342 txn/s, failed submission: 2.8757370139664737 txn/s, expired: 2.8757370139664737 txn/s, latency: 2751.767206132879 ms, (p50: 2100 ms, p90: 4800 ms, p99: 8700 ms), latency samples: 105660
Test Ok

fEst1ck marked this pull request as ready for review June 4, 2024 17:31

fEst1ck force-pushed the unused-assign branch from 17b4c59 to 7fa87eb Compare June 4, 2024 17:31

fEst1ck commented Jun 5, 2024

View reviewed changes

fEst1ck requested review from wrwg, vineethk and rahxephon89 June 5, 2024 06:36

vineethk reviewed Jun 5, 2024

View reviewed changes

fEst1ck force-pushed the unused-assign branch 2 times, most recently from 89f28ab to a01899e Compare June 6, 2024 22:26

fEst1ck requested a review from vineethk June 6, 2024 22:36

vineethk reviewed Jun 10, 2024

View reviewed changes

wrwg requested a review from brmataptos June 11, 2024 18:49

fEst1ck force-pushed the unused-assign branch from 18c82fe to d76d5bf Compare June 18, 2024 08:02

fEst1ck requested a review from vineethk June 18, 2024 08:10

fEst1ck force-pushed the unused-assign branch 2 times, most recently from d097ebc to 5d5d645 Compare June 19, 2024 18:59

brmataptos reviewed Jun 21, 2024

View reviewed changes

fEst1ck force-pushed the unused-assign branch from 5d5d645 to dda8e07 Compare June 24, 2024 20:09

fEst1ck requested a review from brmataptos June 24, 2024 20:11

fEst1ck force-pushed the unused-assign branch from dda8e07 to 914b295 Compare June 25, 2024 17:03

Zekun Wang added 15 commits June 27, 2024 22:19

update tests

827224a

update tests

b9968df

format

347c08f

add test

dd2dda4

import more tests

9940030

tab to space

f81412a

add test

4d60540

remove wrong doc

15c95ca

add test

7ce7e3b

update tests

e999b70

update tests

c2f52d2

update tests

29312ec

update tests

868ea64

update tests

24f4e84

add exp

acaeda4

fEst1ck force-pushed the unused-assign branch from e41f27e to acaeda4 Compare June 28, 2024 02:19

This comment has been minimized.

Sign in to view

update tests

4505471

This comment has been minimized.

Sign in to view

fEst1ck merged commit 3d4d746 into aptos-labs:main Jun 28, 2024
47 checks passed



		Diagnostics:
		warning: Unused assignment to `r`. Consider removing or prefixing with an underscore: `_r`

[Compiler-V2] Check Unused Assignments #13543

[Compiler-V2] Check Unused Assignments #13543

Conversation

fEst1ck commented Jun 4, 2024 • edited by brmataptos Loading

Description

Type of Change

Which Components or Systems Does This Change Impact?

How Has This Been Tested?

trunk-io bot commented Jun 4, 2024 • edited Loading

fEst1ck Jun 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vineethk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fEst1ck commented Jun 6, 2024

fEst1ck commented Jun 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vineethk commented Jun 10, 2024

fEst1ck commented Jun 18, 2024

brmataptos commented Jun 18, 2024

fEst1ck commented Jun 19, 2024

fEst1ck commented Jun 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fEst1ck commented Jun 24, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jun 28, 2024

✅ Forge suite realistic_env_max_load success on 45054712bb53685761d4b8f5677fb5ca6b4467ce

github-actions bot commented Jun 28, 2024

✅ Forge suite compat success on f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 45054712bb53685761d4b8f5677fb5ca6b4467ce

github-actions bot commented Jun 28, 2024

✅ Forge suite framework_upgrade success on f648076a280621dbfd4e73b1ca83e3a3f52878ed ==> 45054712bb53685761d4b8f5677fb5ca6b4467ce

fEst1ck commented Jun 4, 2024 •

edited by brmataptos

Loading

trunk-io bot commented Jun 4, 2024 •

edited

Loading

fEst1ck Jun 5, 2024 •

edited

Loading

✅ Forge suite `realistic_env_max_load` success on `45054712bb53685761d4b8f5677fb5ca6b4467ce`

✅ Forge suite `compat` success on `f648076a280621dbfd4e73b1ca83e3a3f52878ed` ==> `45054712bb53685761d4b8f5677fb5ca6b4467ce`

✅ Forge suite `framework_upgrade` success on `f648076a280621dbfd4e73b1ca83e3a3f52878ed` ==> `45054712bb53685761d4b8f5677fb5ca6b4467ce`