Skip to content

Commit

Permalink
Fix move-compiler panic in case of inlined method use in parameter to…
Browse files Browse the repository at this point in the history
… inlined method. (#8867)

Includes test case illustrating original bug (#8516)
plus a number of tests to check corner cases of multiple inlining and recursive inlining.
(Several failed without this fix.)

Also replace panic!("ICE expected function parameter to be a lambda") with a more useful diag
output.
  • Loading branch information
brmataptos authored and gedigi committed Aug 2, 2023
1 parent b268ca4 commit d46d5a8
Show file tree
Hide file tree
Showing 25 changed files with 464 additions and 10 deletions.
21 changes: 21 additions & 0 deletions aptos-move/framework/aptos-stdlib/tests/math64_tests.move
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#[test_only]
module aptos_std::math64_tests {
use aptos_std::math64;

#[test]
fun test_nested_mul_div() {
let a = math64::mul_div(1, 1, 1);
assert!(math64::mul_div(1, a, 1) == 1, 0);
}

#[test]
fun test_nested_mul_div2() {
assert!(math64::mul_div(1, math64::mul_div(1, 1, 1),1) == 1, 0);
}

#[test]
fun test_nested_mul_div3() {
let a = math64::mul_div(1, math64::mul_div(1, 1, 1),1);
assert!(a == 1, 0);
}
}
33 changes: 23 additions & 10 deletions third_party/move/move-compiler/src/inlining/translate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,11 @@ impl<'l> Inliner<'l> {
{
for (_, mid_, mdef) in prog.modules.iter_mut() {
self.current_module = Some(*mid_);
if mode == VisitingMode::All || mdef.is_source_module {
let visit_module = match mode {
VisitingMode::All => true,
VisitingMode::SourceOnly => mdef.is_source_module,
};
if visit_module {
for (loc, fname, fdef) in mdef.functions.iter_mut() {
self.current_function = *fname;
self.current_function_loc = Some(loc);
Expand Down Expand Up @@ -345,7 +349,14 @@ impl<'l, 'r> SubstitutionVisitor<'l, 'r> {
items.push_back(sp(loc, SequenceItem_::Seq(body)));
Some(UnannotatedExp_::Block(items))
},
_ => panic!("ICE expected function parameter to be a lambda"),
_ => {
self.inliner.env.add_diag(diag!(
Inlining::Unsupported,
(repl.exp.loc,
"Inlined function-typed parameter currently must be a literal lambda expression")
));
None
},
}
},
_ => None,
Expand Down Expand Up @@ -624,14 +635,16 @@ impl<'l> Inliner<'l> {
.zip(mcall.type_arguments.iter())
.map(|(p, t)| (p.id, t.clone()))
.collect();
let (decls_for_let, bindings) = self.process_parameters(
call_loc,
fdef.signature
.parameters
.iter()
.cloned()
.zip(get_args_from_exp(&mcall.arguments)),
);
let mut inliner_visitor = OuterVisitor { inliner: self };
let mut inlined_args = mcall.arguments.clone();
Dispatcher::new(&mut inliner_visitor).exp(&mut inlined_args);
let mapped_params = fdef
.signature
.parameters
.iter()
.cloned()
.zip(get_args_from_exp(&inlined_args));
let (decls_for_let, bindings) = self.process_parameters(call_loc, mapped_params);

// Expand the body in its own independent visitor
self.inline_stack.push_front(global_name); // for cycle detection
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module 0x42::mathtest {
public inline fun fun1(a: u64, b: u64, c: u64): u64 {
(((2 * (a as u128)) + (3 * (b as u128)) + (5 * (c as u128))) as u64)
}
}

module 0x42::mathtest2 {
public inline fun fun2(a: u64, b: u64, c: u64): u64 {
(((7 * (a as u128)) + (11 * (b as u128)) + (13 * (c as u128))) as u64)
}
}

module 0x42::test {
use 0x42::mathtest;
use 0x42::mathtest2;

fun test_nested_fun1() {
let a = mathtest::fun1(2, mathtest::fun1(3, mathtest2::fun2(4, 5, 6), 7),
mathtest2::fun2(8, 9, mathtest::fun1(10, mathtest2::fun2(11, 12, 13), 14)));
assert!(a == 81911, 0);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
module 0x42::LambdaTest1 {
public inline fun inline_mul(a: u64, b: u64): u64 {
a * b
}

public inline fun inline_apply1(f: |u64|u64, b: u64) : u64 {
inline_mul(f(b) + 1, inline_mul(3, 4))
}

public inline fun inline_apply(f: |u64|u64, b: u64) : u64 {
f(b)
}
}

module 0x42::LambdaTest2 {
use 0x42::LambdaTest1;
use std::vector;

public inline fun foreach<T>(v: &vector<T>, action: |&T|) { // expected to be not implemented
let i = 0;
while (i < vector::length(v)) {
action(vector::borrow(v, i));
i = i + 1;
}
}

public fun test_inline_lambda() {
let v = vector[1, 2, 3];
let product = 1;
foreach(&v, |e| product = LambdaTest1::inline_mul(product, *e));
}

public inline fun inline_apply2(g: |u64|u64, c: u64) : u64 {
LambdaTest1::inline_apply1(|z|z, g(LambdaTest1::inline_mul(c, LambdaTest1::inline_apply(|x|x, 3)))) + 2
}

public inline fun inline_apply3(g: |u64|u64, c: u64) : u64 {
LambdaTest1::inline_apply1(g,
LambdaTest1::inline_mul(c, LambdaTest1::inline_apply(|x| {
LambdaTest1::inline_apply(|y|y, x)
},
3))) + 4
}
}

module 0x42::LambdaTest {
use 0x42::LambdaTest2;

public inline fun inline_apply(f: |u64|u64, b: u64) : u64 {
f(b)
}

public inline fun inline_apply_test() : u64 {
LambdaTest2::inline_apply2(|x| x + 1, 3) +
LambdaTest2::inline_apply2(|x| x * x, inline_apply(|y|y, 3))
}

fun test_lambda() {
let a = inline_apply_test();
assert!(a == 1, 0);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
error[E14003]: feature not supported in inlined functions
┌─ tests/move_check/inlining/lambda_param.move:7:15
7 │ inline_apply(f, b)
│ ^ Inlined function-typed parameter currently must be a literal lambda expression

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
module 0x42::LambdaParam {
public inline fun inline_apply(f: |u64|u64, b: u64) : u64 {
f(b)
}

public inline fun inline_apply2(f: |u64|u64, b: u64) : u64 {
inline_apply(f, b)
}

fun test_lambda_symbol_param() {
let a = inline_apply2(|x| x, 3);
assert!(a == 3, 0);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error[E02004]: invalid 'module' declaration
┌─ tests/move_check/inlining/multiple_nesting.move:4:9
4 │ mathtest2::mul_div2(c, a, b)
│ ^^^^^^^^^^^^^^^^^^^ '0x42::mathtest2' uses '0x42::mathtest'. This 'use' relationship creates a dependency cycle.
·
11 │ mathtest::mul_div(b, a, c)
│ ----------------- '0x42::mathtest' uses '0x42::mathtest2'

error[E14001]: recursion during function inlining not allowed
┌─ tests/move_check/inlining/multiple_nesting.move:4:20
4 │ mathtest2::mul_div2(c, a, b)
│ ^^^^^^^^ cyclic inlining: -> mul_div -> mul_div2

error[E14001]: recursion during function inlining not allowed
┌─ tests/move_check/inlining/multiple_nesting.move:11:19
11 │ mathtest::mul_div(b, a, c)
│ ^^^^^^^ cyclic inlining: -> mul_div2 -> mul_div

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
module 0x42::mathtest {
use 0x42::mathtest2;
public inline fun mul_div(a: u64, b: u64, c: u64): u64 {
mathtest2::mul_div2(c, a, b)
}
}

module 0x42::mathtest2 {
use 0x42::mathtest;
public inline fun mul_div2(a: u64, b: u64, c: u64): u64 {
mathtest::mul_div(b, a, c)
}
}

module 0x42::test {
use 0x42::mathtest;
use 0x42::mathtest2;
fun test_nested_mul_div() {
let a = mathtest::mul_div(1, mathtest::mul_div(1, 1, 1),
mathtest2::mul_div2(1, 1, 1));
assert!(a == 1, 0);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module 0x42::mathtest {
public inline fun mul_div(a: u64, b: u64, c: u64): u64 {
(((a as u128) * (b as u128) / (c as u128)) as u64)
}
}

module 0x42::test {
use 0x42::mathtest;
fun test_nested_mul_div() {
let a = mathtest::mul_div(1, mathtest::mul_div(1, 1, 1),1);
assert!(a == 1, 0);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
module 0x42::OrderSensitiveTest1 {
public inline fun inline_fun1(a: u64, b: u64): u64 {
a * b
}

public inline fun inline_fun2(a: u64, b: u64): u64 {
inline_fun1(a, b) + 2 * inline_fun3(a, b)
}

public inline fun inline_fun3(a: u64, b: u64): u64 {
a * b + 2
}
}

module 0x42::OrderSensitiveTest2 {
use 0x42::OrderSensitiveTest1;

public inline fun inline_fun1(a: u64, b: u64): u64 {
a * b + 3
}

public inline fun inline_fun2(a: u64, b: u64): u64 {
OrderSensitiveTest1::inline_fun2(inline_fun1(a, b), inline_fun3(a, b))
+ 3 * inline_fun1(a, b)
+ 5 * inline_fun3(a, b)
}

public inline fun inline_fun3(a: u64, b: u64): u64 {
a * b + 4
}
}

module 0x42::OrderSensitiveTest3 {
use 0x42::OrderSensitiveTest2;

public inline fun fun1(a: u64, b: u64): u64 {
a * b + 5
}

public fun fun2(a: u64, b: u64): u64 {
OrderSensitiveTest2::inline_fun2(7 * fun1(a, b), b)
+ 9 * fun3(a, b)
}

public inline fun fun3(a: u64, b: u64): u64 {
a * b + 6
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
module 0x42::mathtest {
public inline fun mul_div(a: u64, b: u64, c: u64): u64 {
(((a as u128) * (b as u128) / (c as u128)) as u64)
}
}

module 0x42::mathtest2 {
use 0x42::mathtest;
public inline fun mul_div2(a: u64, b: u64, c: u64): u64 {
mathtest::mul_div(b, a, c)
}
}

module 0x42::mathtest3 {
use 0x42::mathtest2;
public inline fun mul_div3(a: u64, b: u64, c: u64): u64 {
mathtest2::mul_div2(b, a, c)
}
}

module 0x42::test {
use 0x42::mathtest;
use 0x42::mathtest2;
use 0x42::mathtest3;
fun test_nested_mul_div() {
let a = mathtest::mul_div(
mathtest3::mul_div3(1, 1, 1),
mathtest::mul_div(1, 1, 1),
mathtest2::mul_div2(1, 1, 1));
assert!(a == 1, 0);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
processed 2 tasks

task 1 'run'. lines 13-13:
return values: 3
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//# publish
module 0x42::Test {

public inline fun apply(f: |u64, u64|u64, x: u64, y: u64): u64 {
f(x, y)
}

public fun test(): u64 {
apply(|x, y| x + y, 1, apply(|x, y| x * y, 2, 1))
}
}

//# run 0x42::Test::test
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
processed 3 tasks

task 2 'run'. lines 17-17:
return values: 3
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//# publish
module 0x42::Test1 {
public inline fun apply(f: |u64, u64|u64, x: u64, y: u64): u64 {
f(x, y)
}
}

//# publish
module 0x42::Test {
use 0x42::Test1;

public fun test(): u64 {
Test1::apply(|x, y| x + y, 1, Test1::apply(|x, y| x * y, 2, 1))
}
}

//# run 0x42::Test::test
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
processed 3 tasks

task 2 'run'. lines 21-21:
return values: 18
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
//# publish
module 0x42::TwoLevelTestModule {
public inline fun f1(x: u64): u64 {
x * 3
}

public inline fun f2(x: u64): u64 {
2 * x
}
}

//# publish
module 0x42::TwoLevelTestMain {
use 0x42::TwoLevelTestModule;

public fun test(): u64 {
TwoLevelTestModule::f1(TwoLevelTestModule::f2(3))
}
}

//# run 0x42::TwoLevelTestMain::test
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
processed 3 tasks

task 2 'run'. lines 25-25:
return values: 18
Loading

0 comments on commit d46d5a8

Please sign in to comment.