Skip to content

Commit

Permalink
Introduce a way for entries generated by tests to be associated with …
Browse files Browse the repository at this point in the history
…their related function declaration (FuelLabs#3509)

closes FuelLabs#3508.

To continue with improving in-language testing (like the linked issues),
we need to have a way of getting the original function declaration for
the test that we are running from `forc-test`. Currently `forc-test`
only works on `FinalizedEntry`s generated by the tests and we cannot get
the function declaration from the entry point. Here I am introducing an
optional field to the `FinalizedEntry` for tests. If the
`FinalizedEntry` is generated from a test function, corresponding
`FinalizedEntry`'s declaration id is populated and can be retrieved. To
pass this declaration id information I needed to preserve it in the IR
generation step to do that I added a metadata for it and populate it for
the test function declarations. So for test function declarations we are
inserting their declaration index to the metadata. I did not insert the
`DeclarationId` since it does not implement `Hash`, and we are already
inserting the span for the function declaration. Since `DeclarationId`
is basically the index + span, storing only the index was enough. After
the IR generation before creating an entry point I construct the
`DeclarationId` from index and span (which comes from the metadata) and
pass that to `FinalizedEntry`. This way in the IR generation step we are
not losing the declaration index + span (and thus, the declaration id)
and can retrieve the test function's declaration later on.

unblocks FuelLabs#3490.
unblocks FuelLabs#3266.
kayagokalp authored Dec 7, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent d605418 commit aa95047
Showing 17 changed files with 161 additions and 25 deletions.
14 changes: 11 additions & 3 deletions sway-core/src/asm_generation/asm_builder.rs
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ use super::{

use crate::{
asm_lang::{virtual_register::*, Label, Op, VirtualImmediate12, VirtualImmediate18, VirtualOp},
declaration_engine::DeclarationId,
error::*,
fuel_prelude::fuel_crypto::Hasher,
metadata::MetadataManager,
@@ -54,7 +55,7 @@ pub(super) struct AsmBuilder<'ir> {

// Final resulting VM bytecode ops; entry functions with their function and label, and regular
// non-entry functions.
entries: Vec<(Function, Label, Vec<Op>)>,
entries: Vec<(Function, Label, Vec<Op>, Option<DeclarationId>)>,
non_entries: Vec<Vec<Op>>,

// In progress VM bytecode ops.
@@ -64,7 +65,12 @@ pub(super) struct AsmBuilder<'ir> {
type AsmBuilderResult = (
DataSection,
RegisterSequencer,
Vec<(Function, Label, AbstractInstructionSet)>,
Vec<(
Function,
Label,
AbstractInstructionSet,
Option<DeclarationId>,
)>,
Vec<AbstractInstructionSet>,
);

@@ -114,7 +120,9 @@ impl<'ir> AsmBuilder<'ir> {
self.reg_seqr,
self.entries
.into_iter()
.map(|(f, l, ops)| (f, l, AbstractInstructionSet { ops }))
.map(|(f, l, ops, test_decl_id)| {
(f, l, AbstractInstructionSet { ops }, test_decl_id)
})
.collect(),
self.non_entries
.into_iter()
9 changes: 8 additions & 1 deletion sway-core/src/asm_generation/asm_builder/functions.rs
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ use crate::{
virtual_register::*, Op, OrganizationalOp, VirtualImmediate12, VirtualImmediate24,
VirtualOp,
},
declaration_engine::DeclarationId,
error::*,
fuel_prelude::fuel_asm::GTFArgs,
};
@@ -147,6 +148,11 @@ impl<'ir> AsmBuilder<'ir> {
let (start_label, end_label) = self.func_to_labels(&function);
let md = function.get_metadata(self.context);
let span = self.md_mgr.md_to_span(self.context, md);
let test_decl_index = self.md_mgr.md_to_test_decl_index(self.context, md);
let test_decl_id = match (&span, &test_decl_index) {
(Some(span), Some(decl_index)) => Some(DeclarationId::new(*decl_index, span.clone())),
_ => None,
};
let comment = format!(
"--- start of function: {} ---",
function.get_name(self.context)
@@ -251,7 +257,8 @@ impl<'ir> AsmBuilder<'ir> {
let mut ops = Vec::new();
ops.append(&mut self.cur_bytecode);
if func_is_entry {
self.entries.push((function, start_label, ops));
self.entries
.push((function, start_label, ops, test_decl_id));
} else {
self.non_entries.push(ops);
}
4 changes: 4 additions & 0 deletions sway-core/src/asm_generation/finalized_asm.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use super::{DataSection, InstructionSet, ProgramKind};
use crate::asm_lang::allocated_ops::AllocatedOpcode;
use crate::declaration_engine::DeclarationId;
use crate::error::*;
use crate::source_map::SourceMap;

@@ -28,6 +29,9 @@ pub struct FinalizedEntry {
pub imm: u64,
/// The function selector (only `Some` for contract ABI methods).
pub selector: Option<[u8; 4]>,
/// If this entry is constructed from a test function contains the declaration id for that
/// function, otherwise contains `None`.
pub test_decl_id: Option<DeclarationId>,
}

impl FinalizedAsm {
3 changes: 2 additions & 1 deletion sway-core/src/asm_generation/from_ir.rs
Original file line number Diff line number Diff line change
@@ -112,10 +112,11 @@ fn compile_module_to_asm(
let (data_section, reg_seqr, entries, non_entries) = builder.finalize();
let entries = entries
.into_iter()
.map(|(func, label, ops)| {
.map(|(func, label, ops, test_decl_id)| {
let selector = func.get_selector(context);
let name = func.get_name(context).to_string();
AbstractEntry {
test_decl_id,
selector,
label,
ops,
7 changes: 4 additions & 3 deletions sway-core/src/asm_generation/programs.rs
Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@ use super::{
DataSection, InstructionSet,
};

use crate::asm_lang::Label;
use crate::{asm_lang::Label, declaration_engine::DeclarationId};

type SelectorOpt = Option<[u8; 4]>;
type FnName = String;
@@ -40,6 +40,7 @@ pub(super) struct AbstractEntry {
pub(super) label: Label,
pub(super) ops: AbstractInstructionSet,
pub(super) name: FnName,
pub(super) test_decl_id: Option<DeclarationId>,
}

/// An AllocatedProgram represents code which has allocated registers but still has abstract
@@ -49,13 +50,13 @@ pub(super) struct AllocatedProgram {
data_section: DataSection,
prologue: AllocatedAbstractInstructionSet,
functions: Vec<AllocatedAbstractInstructionSet>,
entries: Vec<(SelectorOpt, Label, FnName)>,
entries: Vec<(SelectorOpt, Label, FnName, Option<DeclarationId>)>,
}

/// A FinalProgram represents code which may be serialized to VM bytecode.
pub(super) struct FinalProgram {
kind: ProgramKind,
data_section: DataSection,
ops: InstructionSet,
entries: Vec<(SelectorOpt, ImmOffset, FnName)>,
entries: Vec<(SelectorOpt, ImmOffset, FnName, Option<DeclarationId>)>,
}
9 changes: 8 additions & 1 deletion sway-core/src/asm_generation/programs/abstract.rs
Original file line number Diff line number Diff line change
@@ -46,7 +46,14 @@ impl AbstractProgram {
let entries = self
.entries
.iter()
.map(|entry| (entry.selector, entry.label, entry.name.clone()))
.map(|entry| {
(
entry.selector,
entry.label,
entry.name.clone(),
entry.test_decl_id.clone(),
)
})
.collect();

// Gather all the functions together, optimise and then verify the instructions.
4 changes: 2 additions & 2 deletions sway-core/src/asm_generation/programs/allocated.rs
Original file line number Diff line number Diff line change
@@ -23,12 +23,12 @@ impl AllocatedProgram {
let entries = self
.entries
.into_iter()
.map(|(selector, label, name)| {
.map(|(selector, label, name, test_decl_id)| {
let offset = label_offsets
.remove(&label)
.expect("no offset for entry")
.offs;
(selector, offset, name)
(selector, offset, name, test_decl_id)
})
.collect();

3 changes: 2 additions & 1 deletion sway-core/src/asm_generation/programs/final.rs
Original file line number Diff line number Diff line change
@@ -11,10 +11,11 @@ impl FinalProgram {
entries: self
.entries
.into_iter()
.map(|(selector, imm, fn_name)| FinalizedEntry {
.map(|(selector, imm, fn_name, test_decl_id)| FinalizedEntry {
imm,
fn_name,
selector,
test_decl_id,
})
.collect(),
}
2 changes: 1 addition & 1 deletion sway-core/src/declaration_engine/declaration_id.rs
Original file line number Diff line number Diff line change
@@ -92,7 +92,7 @@ impl ReplaceDecls for DeclarationId {
}

impl DeclarationId {
pub(super) fn new(index: usize, span: Span) -> DeclarationId {
pub(crate) fn new(index: usize, span: Span) -> DeclarationId {
DeclarationId(index, span)
}

29 changes: 22 additions & 7 deletions sway-core/src/ir_generation/compile.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
declaration_engine::declaration_engine::de_get_constant,
declaration_engine::{declaration_engine::de_get_constant, DeclarationId},
language::{ty, Visibility},
metadata::MetadataManager,
semantic_analysis::namespace,
@@ -26,7 +26,7 @@ pub(super) fn compile_script(
namespace: &namespace::Module,
declarations: &[ty::TyDeclaration],
logged_types_map: &HashMap<TypeId, LogId>,
test_fns: &[ty::TyFunctionDeclaration],
test_fns: &[(ty::TyFunctionDeclaration, DeclarationId)],
) -> Result<Module, CompileError> {
let module = Module::new(context, Kind::Script);
let mut md_mgr = MetadataManager::default();
@@ -47,6 +47,7 @@ pub(super) fn compile_script(
module,
main_function,
logged_types_map,
None,
)?;
compile_tests(
type_engine,
@@ -67,7 +68,7 @@ pub(super) fn compile_predicate(
namespace: &namespace::Module,
declarations: &[ty::TyDeclaration],
logged_types: &HashMap<TypeId, LogId>,
test_fns: &[ty::TyFunctionDeclaration],
test_fns: &[(ty::TyFunctionDeclaration, DeclarationId)],
) -> Result<Module, CompileError> {
let module = Module::new(context, Kind::Predicate);
let mut md_mgr = MetadataManager::default();
@@ -88,6 +89,7 @@ pub(super) fn compile_predicate(
module,
main_function,
&HashMap::new(),
None,
)?;
compile_tests(
type_engine,
@@ -107,7 +109,7 @@ pub(super) fn compile_contract(
namespace: &namespace::Module,
declarations: &[ty::TyDeclaration],
logged_types_map: &HashMap<TypeId, LogId>,
test_fns: &[ty::TyFunctionDeclaration],
test_fns: &[(ty::TyFunctionDeclaration, DeclarationId)],
type_engine: &TypeEngine,
) -> Result<Module, CompileError> {
let module = Module::new(context, Kind::Contract);
@@ -150,7 +152,7 @@ pub(super) fn compile_library(
namespace: &namespace::Module,
declarations: &[ty::TyDeclaration],
logged_types_map: &HashMap<TypeId, LogId>,
test_fns: &[ty::TyFunctionDeclaration],
test_fns: &[(ty::TyFunctionDeclaration, DeclarationId)],
) -> Result<Module, CompileError> {
let module = Module::new(context, Kind::Library);
let mut md_mgr = MetadataManager::default();
@@ -271,6 +273,7 @@ fn compile_declarations(
Ok(())
}

#[allow(clippy::too_many_arguments)]
pub(super) fn compile_function(
type_engine: &TypeEngine,
context: &mut Context,
@@ -279,6 +282,7 @@ pub(super) fn compile_function(
ast_fn_decl: &ty::TyFunctionDeclaration,
logged_types_map: &HashMap<TypeId, LogId>,
is_entry: bool,
test_decl_id: Option<DeclarationId>,
) -> Result<Option<Function>, CompileError> {
// Currently monomorphisation of generics is inlined into main() and the functions with generic
// args are still present in the AST declarations, but they can be ignored.
@@ -301,6 +305,7 @@ pub(super) fn compile_function(
args,
None,
logged_types_map,
test_decl_id,
)
.map(Some)
}
@@ -313,6 +318,7 @@ pub(super) fn compile_entry_function(
module: Module,
ast_fn_decl: &ty::TyFunctionDeclaration,
logged_types_map: &HashMap<TypeId, LogId>,
test_decl_id: Option<DeclarationId>,
) -> Result<Function, CompileError> {
let is_entry = true;
compile_function(
@@ -323,6 +329,7 @@ pub(super) fn compile_entry_function(
ast_fn_decl,
logged_types_map,
is_entry,
test_decl_id,
)
.map(|f| f.expect("entry point should never contain generics"))
}
@@ -333,18 +340,19 @@ pub(super) fn compile_tests(
md_mgr: &mut MetadataManager,
module: Module,
logged_types_map: &HashMap<TypeId, LogId>,
test_fns: &[ty::TyFunctionDeclaration],
test_fns: &[(ty::TyFunctionDeclaration, DeclarationId)],
) -> Result<Vec<Function>, CompileError> {
test_fns
.iter()
.map(|ast_fn_decl| {
.map(|(ast_fn_decl, decl_id)| {
compile_entry_function(
type_engine,
context,
md_mgr,
module,
ast_fn_decl,
logged_types_map,
Some(decl_id.clone()),
)
})
.collect()
@@ -379,6 +387,7 @@ fn compile_fn_with_args(
args: Vec<(String, Type, Span)>,
selector: Option<[u8; 4]>,
logged_types_map: &HashMap<TypeId, LogId>,
test_decl_id: Option<DeclarationId>,
) -> Result<Function, CompileError> {
let inline_opt = ast_fn_decl.inline();
let ty::TyFunctionDeclaration {
@@ -413,6 +422,11 @@ fn compile_fn_with_args(
let storage_md_idx = md_mgr.purity_to_md(context, *purity);
let mut metadata = md_combine(context, &span_md_idx, &storage_md_idx);

let decl_index = test_decl_id.map(|decl_id| *decl_id as usize);
if let Some(decl_index) = decl_index {
let test_decl_index_md_idx = md_mgr.test_decl_index_to_md(context, decl_index);
metadata = md_combine(context, &metadata, &test_decl_index_md_idx);
}
if let Some(inline) = inline_opt {
let inline_md_idx = md_mgr.inline_to_md(context, inline);
metadata = md_combine(context, &metadata, &inline_md_idx);
@@ -560,5 +574,6 @@ fn compile_abi_method(
args,
Some(selector),
logged_types_map,
None,
)
}
1 change: 1 addition & 0 deletions sway-core/src/ir_generation/function.rs
Original file line number Diff line number Diff line change
@@ -1128,6 +1128,7 @@ impl<'te> FnCompiler<'te> {
&callee_fn_decl,
&self.logged_types_map,
is_entry,
None,
)?
.unwrap();
self.recreated_fns.insert(fn_key, new_func);
8 changes: 5 additions & 3 deletions sway-core/src/language/ty/module.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use sway_types::Ident;

use crate::{
declaration_engine::de_get_function, language::ty::*, language::DepName,
declaration_engine::{de_get_function, DeclarationId},
language::ty::*,
language::DepName,
semantic_analysis::namespace,
};

@@ -39,15 +41,15 @@ impl TyModule {
}

/// All test functions within this module.
pub fn test_fns(&self) -> impl '_ + Iterator<Item = TyFunctionDeclaration> {
pub fn test_fns(&self) -> impl '_ + Iterator<Item = (TyFunctionDeclaration, DeclarationId)> {
self.all_nodes.iter().filter_map(|node| {
if let TyAstNodeContent::Declaration(TyDeclaration::FunctionDeclaration(ref decl_id)) =
node.content
{
let fn_decl = de_get_function(decl_id.clone(), &node.span)
.expect("no function declaration for ID");
if fn_decl.is_test() {
return Some(fn_decl);
return Some((fn_decl, decl_id.clone()));
}
}
None
2 changes: 1 addition & 1 deletion sway-core/src/language/ty/program.rs
Original file line number Diff line number Diff line change
@@ -429,7 +429,7 @@ impl TyProgram {
}

/// All test function declarations within the program.
pub fn test_fns(&self) -> impl '_ + Iterator<Item = TyFunctionDeclaration> {
pub fn test_fns(&self) -> impl '_ + Iterator<Item = (TyFunctionDeclaration, DeclarationId)> {
self.root
.submodules_recursive()
.flat_map(|(_, submod)| submod.module.test_fns())
45 changes: 45 additions & 0 deletions sway-core/src/metadata.rs
Original file line number Diff line number Diff line change
@@ -20,12 +20,14 @@ pub(crate) struct MetadataManager {
md_storage_op_cache: HashMap<MetadataIndex, StorageOperation>,
md_storage_key_cache: HashMap<MetadataIndex, u64>,
md_inline_cache: HashMap<MetadataIndex, Inline>,
md_test_decl_index_cache: HashMap<MetadataIndex, usize>,

span_md_cache: HashMap<Span, MetadataIndex>,
file_loc_md_cache: HashMap<*const PathBuf, MetadataIndex>,
storage_op_md_cache: HashMap<Purity, MetadataIndex>,
storage_key_md_cache: HashMap<u64, MetadataIndex>,
inline_md_cache: HashMap<Inline, MetadataIndex>,
test_decl_index_md_cache: HashMap<usize, MetadataIndex>,
}

#[derive(Clone, Copy)]
@@ -61,6 +63,29 @@ impl MetadataManager {
})
}

pub(crate) fn md_to_test_decl_index(
&mut self,
context: &Context,
md_idx: Option<MetadataIndex>,
) -> Option<usize> {
Self::for_each_md_idx(context, md_idx, |md_idx| {
self.md_test_decl_index_cache
.get(&md_idx)
.cloned()
.or_else(|| {
// Create a new decl index and save it in the cache
md_idx
.get_content(context)
.unwrap_struct("decl_index", 1)
.and_then(|fields| {
let index = fields[0].unwrap_integer().map(|index| index as usize)?;
self.md_test_decl_index_cache.insert(md_idx, index);
Some(index)
})
})
})
}

pub(crate) fn md_to_storage_op(
&mut self,
context: &Context,
@@ -198,6 +223,26 @@ impl MetadataManager {
})
}

pub(crate) fn test_decl_index_to_md(
&mut self,
context: &mut Context,
decl_index: usize,
) -> Option<MetadataIndex> {
self.test_decl_index_md_cache
.get(&decl_index)
.copied()
.or_else(|| {
let md_idx = MetadataIndex::new_struct(
context,
"decl_index",
vec![Metadatum::Integer(decl_index as u64)],
);
self.test_decl_index_md_cache.insert(decl_index, md_idx);

Some(md_idx)
})
}

pub(crate) fn storage_key_to_md(
&mut self,
context: &mut Context,
29 changes: 29 additions & 0 deletions sway-ir/tests/serialize/test.ir
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
script {
// check: fn main() -> (), !1 {
fn main() -> (), !1 {
entry():
// check: v0 = const unit ()
v0 = const unit ()
ret () v0
}

// check: fn my_test_func() -> (), !4 {
fn my_test_func() -> (), !4 {
entry():
// check: v0 = const unit ()
v0 = const unit ()
ret () v0
}
}

// check: !0 = "a string\\n"
// check: !1 = span !0
// check: !2 = span !0
// check: !3 = decl_index 4
// check: !4 = (!2 !3)

!0 = "a string\\n"
!1 = span !0 9 21
!2 = span !0 307 341
!3 = decl_index 4
!4 = (!2 !3)
2 changes: 1 addition & 1 deletion test/src/ir_generation/mod.rs
Original file line number Diff line number Diff line change
@@ -141,7 +141,7 @@ pub(super) async fn run(filter_regex: Option<&regex::Regex>) -> Result<()> {
let tree_type = typed_program.kind.tree_type();

// Compile to IR.
let include_tests = false;
let include_tests = true;
let mut ir = compile_program(&typed_program, include_tests, &type_engine)
.unwrap_or_else(|e| {
panic!("Failed to compile test {}:\n{e}", path.display());
15 changes: 15 additions & 0 deletions test/src/ir_generation/tests/test_metadata.sw
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
script;

fn main() {}
fn my_func() {}

#[test()]
fn my_test_func() {
my_func();
}

// check: fn main() -> (), $(main_md=$MD) {
// check: fn my_test_func() -> (), $(test_md=$MD) {

// check: $(decl_index_md=$MD) = decl_index
// check: $test_md = ($MD $decl_index_md)

0 comments on commit aa95047

Please sign in to comment.