-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[compiler v2] Fix bugs around compilation of vector code #9636
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -512,7 +512,16 @@ impl<'a> FunctionGenerator<'a> { | |
) { | ||
let fun_ctx = ctx.fun_ctx; | ||
self.abstract_push_args(ctx, source); | ||
if inst.is_empty() { | ||
if let Some(opcode) = ctx.fun_ctx.module.get_well_known_function_code( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I note that V1 code checks (in fake_natives.rs) for this check for the existence of an attribute (AttributeName_::Known(KnownAttribute::Native(NativeAttribute::BytecodeInstruction))) before trying to match a "fake native"/"well-known function". I might agree with their comment about developer sanity here:
If someday our descendants decide to add a new well-known function they might have an easier time if they get a warning from the compiler that it sees the #[bytecode_instruction] annotation on the function definition but doesn't know how to translate it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure whether there is any value to this attribute, and I remember the author wasn't sure either. The attribute would have to at least also denote the bytecode which is going to be used. In fact, I think those functions should not appear in the first place in vector, a refactoring we may want to do at a later point. (Specification builtin functions do not have source code.) They have already introduced quite some confusion including security incidents. |
||
&ctx.fun_ctx.loc, | ||
id, | ||
Some( | ||
self.gen | ||
.signature(&ctx.fun_ctx.module, &ctx.fun_ctx.loc, inst.to_vec()), | ||
), | ||
) { | ||
self.emit(opcode) | ||
} else if inst.is_empty() { | ||
let idx = self.gen.function_index( | ||
&fun_ctx.module, | ||
&fun_ctx.loc, | ||
|
@@ -523,7 +532,7 @@ impl<'a> FunctionGenerator<'a> { | |
let idx = self.gen.function_instantiation_index( | ||
&fun_ctx.module, | ||
&fun_ctx.loc, | ||
fun_ctx.fun.func_env, | ||
&fun_ctx.module.env.get_function(id), | ||
inst.to_vec(), | ||
); | ||
self.emit(FF::Bytecode::CallGeneric(idx)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
// ---- Model Dump | ||
module 0x42::reference_conversion { | ||
private fun deref(r: &u64) { | ||
Deref(r) | ||
} | ||
private fun use_it() { | ||
{ | ||
let x: u64 = 42; | ||
{ | ||
let r: &mut u64 = Borrow(Mutable)(x); | ||
r = 43; | ||
reference_conversion::deref(r) | ||
} | ||
} | ||
} | ||
} // end 0x42::reference_conversion | ||
|
||
============ initial bytecode ================ | ||
|
||
[variant baseline] | ||
fun reference_conversion::deref($t0: &u64): u64 { | ||
var $t1: u64 | ||
0: $t1 := read_ref($t0) | ||
1: return $t1 | ||
} | ||
|
||
|
||
[variant baseline] | ||
fun reference_conversion::use_it(): u64 { | ||
var $t0: u64 | ||
var $t1: u64 | ||
var $t2: u64 | ||
var $t3: &mut u64 | ||
var $t4: &mut u64 | ||
var $t5: u64 | ||
var $t6: &u64 | ||
0: $t2 := 42 | ||
1: $t1 := move($t2) | ||
2: $t4 := borrow_local($t1) | ||
3: $t3 := move($t4) | ||
4: $t5 := 43 | ||
5: write_ref($t3, $t5) | ||
6: $t6 := freeze_ref($t3) | ||
7: $t0 := reference_conversion::deref($t6) | ||
8: return $t0 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
module 0x42::reference_conversion { | ||
|
||
fun deref(r: &u64): u64 { | ||
*r | ||
} | ||
|
||
fun use_it(): u64 { | ||
let x = 42; | ||
let r = &mut x; | ||
*r = 43; | ||
deref(r) | ||
} | ||
|
||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
============ initial bytecode ================ | ||
|
||
[variant baseline] | ||
fun Test::foo($t0: u64): u64 { | ||
var $t1: u64 | ||
0: $t1 := Test::identity<u64>($t0) | ||
1: return $t1 | ||
} | ||
|
||
|
||
[variant baseline] | ||
fun Test::identity<#0>($t0: #0): #0 { | ||
var $t1: #0 | ||
0: $t1 := move($t0) | ||
1: return $t1 | ||
} | ||
|
||
============ after LiveVarAnalysisProcessor: ================ | ||
|
||
[variant baseline] | ||
fun Test::foo($t0: u64): u64 { | ||
var $t1: u64 | ||
# live vars: $t0 | ||
0: $t1 := Test::identity<u64>($t0) | ||
# live vars: $t1 | ||
1: return $t1 | ||
} | ||
|
||
|
||
[variant baseline] | ||
fun Test::identity<#0>($t0: #0): #0 { | ||
var $t1: #0 | ||
# live vars: $t0 | ||
0: $t1 := move($t0) | ||
# live vars: $t1 | ||
1: return $t1 | ||
} | ||
|
||
|
||
============ disassembled file-format ================== | ||
// Move bytecode v6 | ||
module 42.Test { | ||
|
||
|
||
foo(Arg0: u64): u64 { | ||
B0: | ||
0: MoveLoc[0](Arg0: u64) | ||
1: Call identity<u64>(u64): u64 | ||
2: Ret | ||
} | ||
identity<Ty0>(Arg0: Ty0): Ty0 { | ||
B0: | ||
0: MoveLoc[0](Arg0: Ty0) | ||
1: StLoc[1](loc0: Ty0) | ||
2: MoveLoc[1](loc0: Ty0) | ||
3: Ret | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
module 0x42::Test { | ||
fun identity<T>(x: T): T { | ||
x | ||
} | ||
|
||
fun foo(x: u64): u64 { | ||
identity(x) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the advantage of this C style errors versus Rust errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This actually neither or, its an error with source location information added to the compilers global environment, which can contain many of them. Its marked as 'internal' because it is unexpected. Nevertheless will be reported with possible errors at the source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+possible other errors at the source