From 81f75843464ce1140040403df0a0fa0852f87386 Mon Sep 17 00:00:00 2001 From: Adam Welc Date: Thu, 14 Nov 2024 13:01:50 -0800 Subject: [PATCH] [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 --- .../crates/module-generation/src/generator.rs | 1 + .../src/source_map.rs | 11 +++++++++-- .../crates/move-compiler/src/cfgir/ast.rs | 2 ++ .../move-compiler/src/cfgir/translate.rs | 3 +++ .../move/crates/move-compiler/src/hlir/ast.rs | 2 ++ .../move-compiler/src/hlir/translate.rs | 3 ++- .../src/to_bytecode/translate.rs | 6 ++++-- .../move-ir-to-bytecode-syntax/src/syntax.rs | 19 +++++++++++-------- .../move-ir-to-bytecode/src/compiler.rs | 4 +++- .../move/crates/move-ir-types/src/ast.rs | 4 ++++ 10 files changed, 41 insertions(+), 14 deletions(-) diff --git a/external-crates/move/crates/module-generation/src/generator.rs b/external-crates/move/crates/module-generation/src/generator.rs index ebe0e131993e8..331c2277dff10 100644 --- 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 index 32ae09de384b3..06dd047191985 100644 --- 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 index 34069019ef9fd..eb28c43e6c6c4 100644 --- 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 index 9d4635dfc9e4c..d99e1b24e28e6 100644 --- 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 index 12b225e454148..b651b34f64a8d 100644 --- 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 index c4947fc4fa19a..4cdd81657c9ed 100644 --- 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 index 7c30d076a4815..2eab2bd73bf0e 100644 --- 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 index 40a8dcceb7b92..8dc4baa5bdb90 100644 --- 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 index adaa773b53614..2af8279801f01 100644 --- 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 { 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 index 0fcd68c82dee4..f759fb6acfd98 100644 --- 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,