[move-compiler] Added function location info to the source map (#20261)

## Description This PR adds additional field to the source map describing the location of the entire function. This is needed to enable better handling of macros in the trace viewer (we need to know which instructions in a given function are outside of a function body to identify those that belong to inlined macro functions). ## Test plan All existing tests must pass
MystenLabs · Nov 14, 2024 · 81f7584 · 81f7584
1 parent 8d90acd
commit 81f7584
Show file tree

Hide file tree

Showing 10 changed files with 41 additions and 14 deletions.
diff --git a/external-crates/move/crates/module-generation/src/generator.rs b/external-crates/move/crates/module-generation/src/generator.rs
@@ -244,6 +244,7 @@ impl<'a> ModuleGenerator<'a> {
             )
         });
         let fun = Function_ {
+            loc: Spanned::unsafe_no_loc(()).loc,
             visibility: FunctionVisibility::Public,
             is_entry: false,
             signature,

diff --git a/external-crates/move/crates/move-bytecode-source-map/src/source_map.rs b/external-crates/move/crates/move-bytecode-source-map/src/source_map.rs
@@ -58,6 +58,10 @@ pub struct FunctionSourceMap {
     /// The source location for the definition of this entire function. Note that in certain
     /// instances this will have no valid source location e.g. the "main" function for modules that
     /// are treated as programs are synthesized and therefore have no valid source location.
+    pub location: Loc,
+    /// The source location for the name under which this functin is defined. Note that in certain
+    /// instances this will have no valid source location e.g. the "main" function for modules that
+    /// are treated as programs are synthesized and therefore have no valid source location.
     pub definition_location: Loc,
 
     /// The names of the type parameters to the function.
@@ -204,8 +208,9 @@ impl EnumSourceMap {
 }
 
 impl FunctionSourceMap {
-    pub fn new(definition_location: Loc, is_native: bool) -> Self {
+    pub fn new(location: Loc, definition_location: Loc, is_native: bool) -> Self {
         Self {
+            location,
             definition_location,
             type_parameters: Vec::new(),
             parameters: Vec::new(),
@@ -361,9 +366,10 @@ impl SourceMap {
         &mut self,
         fdef_idx: FunctionDefinitionIndex,
         location: Loc,
+        definition_location: Loc,
         is_native: bool,
     ) -> Result<()> {
-        self.function_map.insert(fdef_idx.0, FunctionSourceMap::new(location, is_native)).map_or(Ok(()), |_| { Err(format_err!(
+        self.function_map.insert(fdef_idx.0, FunctionSourceMap::new(location, definition_location, is_native)).map_or(Ok(()), |_| { Err(format_err!(
                     "Multiple functions at same function definition index encountered when constructing source map"
                 )) })
     }
@@ -656,6 +662,7 @@ impl SourceMap {
             empty_source_map.add_top_level_function_mapping(
                 FunctionDefinitionIndex(function_idx as TableIndex),
                 default_loc,
+                default_loc,
                 false,
             )?;
             let function_handle = module.function_handle_at(function_def.function);

diff --git a/external-crates/move/crates/move-compiler/src/cfgir/ast.rs b/external-crates/move/crates/move-compiler/src/cfgir/ast.rs
@@ -91,6 +91,7 @@ pub struct Function {
     // index in the original order as defined in the source file
     pub index: usize,
     pub attributes: Attributes,
+    pub loc: Loc,
     /// The original, declared visibility as defined in the source file
     pub visibility: Visibility,
     /// We sometimes change the visibility of functions, e.g. `entry` is marked as `public` in
@@ -317,6 +318,7 @@ impl AstDebug for (FunctionName, &Function) {
                 warning_filter,
                 index,
                 attributes,
+                loc: _,
                 visibility,
                 compiled_visibility,
                 entry,

diff --git a/external-crates/move/crates/move-compiler/src/cfgir/translate.rs b/external-crates/move/crates/move-compiler/src/cfgir/translate.rs
@@ -600,6 +600,7 @@ fn function(
         warning_filter,
         index,
         attributes,
+        loc,
         visibility,
         compiled_visibility,
         entry,
@@ -622,6 +623,7 @@ fn function(
         warning_filter,
         index,
         attributes,
+        loc,
         visibility,
         compiled_visibility,
         entry,
@@ -1064,6 +1066,7 @@ impl<'a> CFGIRVisitorContext for AbsintVisitorContext<'a> {
             warning_filter: _,
             index: _,
             attributes,
+            loc: _,
             compiled_visibility: _,
             visibility,
             entry,

diff --git a/external-crates/move/crates/move-compiler/src/hlir/ast.rs b/external-crates/move/crates/move-compiler/src/hlir/ast.rs
@@ -149,6 +149,7 @@ pub struct Function {
     // index in the original order as defined in the source file
     pub index: usize,
     pub attributes: Attributes,
+    pub loc: Loc,
     /// The original, declared visibility as defined in the source file
     pub visibility: Visibility,
     /// We sometimes change the visibility of functions, e.g. `entry` is marked as `public` in
@@ -1077,6 +1078,7 @@ impl AstDebug for (FunctionName, &Function) {
                 warning_filter,
                 index,
                 attributes,
+                loc: _,
                 visibility,
                 compiled_visibility,
                 entry,

diff --git a/external-crates/move/crates/move-compiler/src/hlir/translate.rs b/external-crates/move/crates/move-compiler/src/hlir/translate.rs
@@ -414,7 +414,7 @@ fn function(context: &mut Context, _name: FunctionName, f: T::Function) -> H::Fu
         warning_filter,
         index,
         attributes,
-        loc: _,
+        loc,
         compiled_visibility: tcompiled_visibility,
         visibility: tvisibility,
         entry,
@@ -431,6 +431,7 @@ fn function(context: &mut Context, _name: FunctionName, f: T::Function) -> H::Fu
         warning_filter,
         index,
         attributes,
+        loc,
         compiled_visibility: visibility(tcompiled_visibility),
         visibility: visibility(tvisibility),
         entry,

diff --git a/external-crates/move/crates/move-compiler/src/to_bytecode/translate.rs b/external-crates/move/crates/move-compiler/src/to_bytecode/translate.rs
@@ -566,6 +566,7 @@ fn function(
         warning_filter: _warning_filter,
         index: _index,
         attributes,
+        loc,
         compiled_visibility: v,
         // original, declared visibility is ignored. This is primarily for marking entry functions
         // as public in tests
@@ -597,15 +598,16 @@ fn function(
             IR::FunctionBody::Bytecode { locals, code }
         }
     };
-    let loc = f.loc();
+    let name_loc = f.loc();
     let name = context.function_definition_name(m, f);
     let ir_function = IR::Function_ {
+        loc,
         visibility: v,
         is_entry: entry.is_some(),
         signature,
         body,
     };
-    ((name, sp(loc, ir_function)), (parameters, attributes))
+    ((name, sp(name_loc, ir_function)), (parameters, attributes))
 }
 
 fn visibility(_context: &mut Context, v: Visibility) -> IR::FunctionVisibility {

diff --git a/external-crates/move/crates/move-ir-to-bytecode-syntax/src/syntax.rs b/external-crates/move/crates/move-ir-to-bytecode-syntax/src/syntax.rs
@@ -1644,23 +1644,26 @@ fn parse_function_decl(
         None
     };
 
+    let body = if is_native {
+        consume_token(tokens, Tok::Semicolon)?;
+        FunctionBody::Native
+    } else {
+        let (locals, body) = parse_function_block_(tokens)?;
+        FunctionBody::Move { locals, code: body }
+    };
+
+    let end_loc = tokens.previous_end_loc();
     let func_name = FunctionName(name);
     let func = Function_::new(
+        make_loc(tokens.file_hash(), start_loc, end_loc),
         visibility,
         is_entry,
         args,
         ret.unwrap_or_default(),
         type_parameters,
-        if is_native {
-            consume_token(tokens, Tok::Semicolon)?;
-            FunctionBody::Native
-        } else {
-            let (locals, body) = parse_function_block_(tokens)?;
-            FunctionBody::Move { locals, code: body }
-        },
+        body,
     );
 
-    let end_loc = tokens.previous_end_loc();
     Ok((
         func_name,
         spanned(tokens.file_hash(), start_loc, end_loc, func),

diff --git a/external-crates/move/crates/move-ir-to-bytecode/src/compiler.rs b/external-crates/move/crates/move-ir-to-bytecode/src/compiler.rs
@@ -72,11 +72,12 @@ macro_rules! record_src_loc {
             )?;
         }
     };
-    (function_decl: $context:expr, $location:expr, $function_index:expr, $is_native:expr) => {{
+    (function_decl: $context:expr, $location:expr, $definition_location: expr, $function_index:expr, $is_native:expr) => {{
         $context.set_function_index($function_index as TableIndex);
         $context.source_map.add_top_level_function_mapping(
             $context.current_function_definition_index(),
             $location,
+            $definition_location,
             $is_native,
         )?;
     }};
@@ -931,6 +932,7 @@ fn compile_function(
 ) -> Result<FunctionDefinition> {
     record_src_loc!(
         function_decl: context,
+        ast_function.value.loc,
         ast_function.loc,
         function_index,
         matches!(ast_function.value.body, FunctionBody::Native)

diff --git a/external-crates/move/crates/move-ir-types/src/ast.rs b/external-crates/move/crates/move-ir-types/src/ast.rs
@@ -369,6 +369,8 @@ pub enum FunctionBody {
 /// A Move function/procedure
 #[derive(PartialEq, Debug, Clone)]
 pub struct Function_ {
+    /// Location of the whole function
+    pub loc: Loc,
     /// The visibility
     pub visibility: FunctionVisibility,
     /// Is entry function
@@ -932,6 +934,7 @@ impl Function_ {
     /// Creates a new function declaration from the components of the function
     /// See the declaration of the struct `Function` for more details
     pub fn new(
+        loc: Loc,
         visibility: FunctionVisibility,
         is_entry: bool,
         formals: Vec<(Var, Type)>,
@@ -941,6 +944,7 @@ impl Function_ {
     ) -> Self {
         let signature = FunctionSignature::new(formals, return_type, type_parameters);
         Function_ {
+            loc,
             visibility,
             is_entry,
             signature,