Skip to content

Commit

Permalink
Experimental: StorageKey feature (FuelLabs#4297)
Browse files Browse the repository at this point in the history
## Description
This PR introduces `StorageHandle` to the `core` as an **experimental**
feature. `StorageHandle` is a descriptor of a location in storage
containing two fields:
1. A key pointing to a storage slot `s`.
2. An offset pointing to a particular word in `s` or in its subsequent
slots. This offset is new and is not described in the RFC. I will
rectify that shortly.
The standard library introduces helper methods `read`, `try_read`, and
`write` to `StorageHandle` that allow reading from and writing to the
location pointed to by the handle. `try_read` returns an `Option`
wrapping the data while `read` automatically internally unwraps the data
and could panic.

The summary of this change is as follows:
* New struct `core::experimental::storage::StorageHandle`
* New storage library `std::experimental::storage::*` that should
eventually replace `std::storage::*`. This new library mirrors the old
one to the extent possible and introduces the helper methods for
`StorageHandle`. Other data structures such as `StorageVec` and
`StorageBytes` will be introduced in a separate PR.
* Add an experimental flag `--experimental-storage` to `forc` that
enables all the required code in the compiler to support `StorageHandle`
as described in the FuelLabs/sway-rfcs#23.
* Type checking phases assumes the existence of `StorageHandle` and
monomorphizes it as needed.
* Storage accesses now always return `StorageHandle` and storage
reassignment are no longer relevant.
* IR gen handles storage accesses by "filling" the key and the offset in
an aggregate representing the `StorageHandle`. The key and the offset
are statically determined based on the index of the storage variable in
the `storage` block and the field accessed, if applicable.
  * Storage initializers are now handled based on the new model
* The new approach packs struct fields by default but does not pack
across storage variables. This offers the most amount of flexibility.
This is a deviation from the RFC which I will update shortly.
* To implement `StorageMap` and other dynamic storage data structures,
we now write `impl StorageHandle<StorageMap<K, V>>` instead of `impl
StorageMap<K, V>` directly. Also, note that the `get` method now returns
a `StorageHandle` instead of the actual data. To get the actual data,
`read()` or `try_read()` should be called on the resulting handle. This
is needed for multiple reasons including proper support for nested
dynamic storage types. Rust also does the same, in a way (where `get`
actually returns ` &` which happens to coerce to the real data in
certain places).
* I added various tests but they're not comprehensive. Some tests on my
list:
  * Extensive tests for storage maps in structs (which now work!)
* Extensive tests for storage accesses with various types and struct
layouts

I still need to figure out how to do nested dynamic storage types with
this approach. The stuff I have in `map_in_map_access` in
`test_projects/experimental_storage/src/main.sw` is just an attempt but
not ergonomic at all of course.

## Checklist

- [ ] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [ ] I have requested a review from the relevant team or maintainers.
  • Loading branch information
mohammadfawaz authored Apr 13, 2023
1 parent 1e63c15 commit 9685fab
Showing 43 changed files with 3,587 additions and 81 deletions.
3 changes: 3 additions & 0 deletions forc-pkg/src/manifest.rs
Original file line number Diff line number Diff line change
@@ -214,6 +214,7 @@ pub struct BuildProfile {
pub include_tests: bool,
pub json_abi_with_callpaths: bool,
pub error_on_warnings: bool,
pub experimental_storage: bool,
}

impl Dependency {
@@ -625,6 +626,7 @@ impl BuildProfile {
include_tests: false,
json_abi_with_callpaths: false,
error_on_warnings: false,
experimental_storage: false,
}
}

@@ -641,6 +643,7 @@ impl BuildProfile {
include_tests: false,
json_abi_with_callpaths: false,
error_on_warnings: false,
experimental_storage: false,
}
}
}
7 changes: 6 additions & 1 deletion forc-pkg/src/pkg.rs
Original file line number Diff line number Diff line change
@@ -302,6 +302,8 @@ pub struct BuildOpts {
pub tests: bool,
/// The set of options to filter by member project kind.
pub member_filter: MemberFilter,
/// Enable the experimental storage implementation and UI.
pub experimental_storage: bool,
}

/// The set of options to filter type of projects to build in a workspace.
@@ -1544,7 +1546,8 @@ pub fn sway_build_config(
.print_finalized_asm(build_profile.print_finalized_asm)
.print_intermediate_asm(build_profile.print_intermediate_asm)
.print_ir(build_profile.print_ir)
.include_tests(build_profile.include_tests);
.include_tests(build_profile.include_tests)
.experimental_storage(build_profile.experimental_storage);
Ok(build_config)
}

@@ -1986,6 +1989,7 @@ fn build_profile_from_opts(
time_phases,
tests,
error_on_warnings,
experimental_storage,
..
} = build_options;
let mut selected_build_profile = BuildProfile::DEBUG;
@@ -2036,6 +2040,7 @@ fn build_profile_from_opts(
profile.include_tests |= tests;
profile.json_abi_with_callpaths |= pkg.json_abi_with_callpaths;
profile.error_on_warnings |= error_on_warnings;
profile.experimental_storage |= experimental_storage;

Ok((selected_build_profile.to_string(), profile))
}
1 change: 1 addition & 0 deletions forc-plugins/forc-client/src/op/deploy.rs
Original file line number Diff line number Diff line change
@@ -255,6 +255,7 @@ fn build_opts_from_cmd(cmd: &cmd::Deploy) -> pkg::BuildOpts {
build_target: BuildTarget::default(),
tests: false,
member_filter: pkg::MemberFilter::only_contracts(),
experimental_storage: cmd.build_profile.experimental_storage,
}
}

1 change: 1 addition & 0 deletions forc-plugins/forc-client/src/op/run.rs
Original file line number Diff line number Diff line change
@@ -175,5 +175,6 @@ fn build_opts_from_cmd(cmd: &cmd::Run) -> pkg::BuildOpts {
debug_outfile: cmd.build_output.debug_file.clone(),
tests: false,
member_filter: pkg::MemberFilter::only_scripts(),
experimental_storage: cmd.build_profile.experimental_storage,
}
}
3 changes: 3 additions & 0 deletions forc-test/src/lib.rs
Original file line number Diff line number Diff line change
@@ -107,6 +107,8 @@ pub struct Opts {
pub error_on_warnings: bool,
/// Output the time elapsed over each part of the compilation process.
pub time_phases: bool,
/// Enable the experimental storage implementation and UI.
pub experimental_storage: bool,
}

/// The set of options provided for controlling logs printed for each test.
@@ -372,6 +374,7 @@ impl Opts {
time_phases: self.time_phases,
tests: true,
member_filter: Default::default(),
experimental_storage: self.experimental_storage,
}
}
}
1 change: 1 addition & 0 deletions forc/src/cli/commands/test.rs
Original file line number Diff line number Diff line change
@@ -198,5 +198,6 @@ fn opts_from_cmd(cmd: Command) -> forc_test::Opts {
binary_outfile: cmd.build.output.bin_file,
debug_outfile: cmd.build.output.debug_file,
build_target: cmd.build.build_target,
experimental_storage: cmd.build.profile.experimental_storage,
}
}
3 changes: 3 additions & 0 deletions forc/src/cli/shared.rs
Original file line number Diff line number Diff line change
@@ -49,6 +49,9 @@ pub struct BuildProfile {
/// Treat warnings as errors.
#[clap(long)]
pub error_on_warnings: bool,
/// Enable the experimental storage implementation and UI.
#[clap(long)]
pub experimental_storage: bool,
}

/// Options related to printing stages of compiler output.
1 change: 1 addition & 0 deletions forc/src/ops/forc_build.rs
Original file line number Diff line number Diff line change
@@ -39,5 +39,6 @@ fn opts_from_cmd(cmd: BuildCommand) -> pkg::BuildOpts {
build_target: cmd.build.build_target,
tests: cmd.tests,
member_filter: Default::default(),
experimental_storage: cmd.build.profile.experimental_storage,
}
}
1 change: 1 addition & 0 deletions forc/src/ops/forc_contract_id.rs
Original file line number Diff line number Diff line change
@@ -72,5 +72,6 @@ fn build_opts_from_cmd(cmd: &ContractIdCommand) -> pkg::BuildOpts {
build_target: BuildTarget::default(),
tests: false,
member_filter: pkg::MemberFilter::only_contracts(),
experimental_storage: cmd.build_profile.experimental_storage,
}
}
1 change: 1 addition & 0 deletions forc/src/ops/forc_predicate_root.rs
Original file line number Diff line number Diff line change
@@ -43,5 +43,6 @@ fn build_opts_from_cmd(cmd: PredicateRootCommand) -> pkg::BuildOpts {
build_target: BuildTarget::default(),
tests: false,
member_filter: pkg::MemberFilter::only_predicates(),
experimental_storage: cmd.build_profile.experimental_storage,
}
}
9 changes: 9 additions & 0 deletions sway-core/src/build_config.rs
Original file line number Diff line number Diff line change
@@ -46,6 +46,7 @@ pub struct BuildConfig {
pub(crate) print_finalized_asm: bool,
pub(crate) print_ir: bool,
pub(crate) include_tests: bool,
pub(crate) experimental_storage: bool,
}

impl BuildConfig {
@@ -88,6 +89,7 @@ impl BuildConfig {
print_finalized_asm: false,
print_ir: false,
include_tests: false,
experimental_storage: false,
}
}

@@ -126,6 +128,13 @@ impl BuildConfig {
}
}

pub fn experimental_storage(self, a: bool) -> Self {
Self {
experimental_storage: a,
..self
}
}

/// Whether or not to include test functions in parsing, type-checking and codegen.
///
/// This should be set to `true` by invocations like `forc test` or `forc check --tests`.
5 changes: 5 additions & 0 deletions sway-core/src/ir_generation.rs
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@ pub fn compile_program(
program: &ty::TyProgram,
include_tests: bool,
engines: Engines<'_>,
experimental_storage: bool,
) -> Result<Context, CompileError> {
let declaration_engine = engines.de();

@@ -59,6 +60,7 @@ pub fn compile_program(
&logged_types,
&messages_types,
&test_fns,
experimental_storage,
),
ty::TyProgramKind::Predicate { main_function } => compile::compile_predicate(
engines,
@@ -69,6 +71,7 @@ pub fn compile_program(
&logged_types,
&messages_types,
&test_fns,
experimental_storage,
),
ty::TyProgramKind::Contract { abi_entries } => compile::compile_contract(
&mut ctx,
@@ -79,6 +82,7 @@ pub fn compile_program(
&messages_types,
&test_fns,
engines,
experimental_storage,
),
ty::TyProgramKind::Library { .. } => compile::compile_library(
engines,
@@ -88,6 +92,7 @@ pub fn compile_program(
&logged_types,
&messages_types,
&test_fns,
experimental_storage,
),
}?;

24 changes: 24 additions & 0 deletions sway-core/src/ir_generation/compile.rs
Original file line number Diff line number Diff line change
@@ -30,6 +30,7 @@ pub(super) fn compile_script(
logged_types_map: &HashMap<TypeId, LogId>,
messages_types_map: &HashMap<TypeId, MessageId>,
test_fns: &[(ty::TyFunctionDecl, DeclRefFunction)],
experimental_storage: bool,
) -> Result<Module, CompileError> {
let module = Module::new(context, Kind::Script);
let mut md_mgr = MetadataManager::default();
@@ -52,6 +53,7 @@ pub(super) fn compile_script(
logged_types_map,
messages_types_map,
None,
experimental_storage,
)?;
compile_tests(
engines,
@@ -61,6 +63,7 @@ pub(super) fn compile_script(
logged_types_map,
messages_types_map,
test_fns,
experimental_storage,
)?;

Ok(module)
@@ -76,6 +79,7 @@ pub(super) fn compile_predicate(
logged_types: &HashMap<TypeId, LogId>,
messages_types: &HashMap<TypeId, MessageId>,
test_fns: &[(ty::TyFunctionDecl, DeclRefFunction)],
experimental_storage: bool,
) -> Result<Module, CompileError> {
let module = Module::new(context, Kind::Predicate);
let mut md_mgr = MetadataManager::default();
@@ -98,6 +102,7 @@ pub(super) fn compile_predicate(
&HashMap::new(),
&HashMap::new(),
None,
experimental_storage,
)?;
compile_tests(
engines,
@@ -107,6 +112,7 @@ pub(super) fn compile_predicate(
logged_types,
messages_types,
test_fns,
experimental_storage,
)?;

Ok(module)
@@ -122,6 +128,7 @@ pub(super) fn compile_contract(
messages_types_map: &HashMap<TypeId, MessageId>,
test_fns: &[(ty::TyFunctionDecl, DeclRefFunction)],
engines: Engines<'_>,
experimental_storage: bool,
) -> Result<Module, CompileError> {
let module = Module::new(context, Kind::Contract);
let mut md_mgr = MetadataManager::default();
@@ -144,6 +151,7 @@ pub(super) fn compile_contract(
logged_types_map,
messages_types_map,
engines,
experimental_storage,
)?;
}
compile_tests(
@@ -154,11 +162,13 @@ pub(super) fn compile_contract(
logged_types_map,
messages_types_map,
test_fns,
experimental_storage,
)?;

Ok(module)
}

#[allow(clippy::too_many_arguments)]
pub(super) fn compile_library(
engines: Engines<'_>,
context: &mut Context,
@@ -167,6 +177,7 @@ pub(super) fn compile_library(
logged_types_map: &HashMap<TypeId, LogId>,
messages_types_map: &HashMap<TypeId, MessageId>,
test_fns: &[(ty::TyFunctionDecl, DeclRefFunction)],
experimental_storage: bool,
) -> Result<Module, CompileError> {
let module = Module::new(context, Kind::Library);
let mut md_mgr = MetadataManager::default();
@@ -188,6 +199,7 @@ pub(super) fn compile_library(
logged_types_map,
messages_types_map,
test_fns,
experimental_storage,
)?;

Ok(module)
@@ -308,6 +320,7 @@ pub(super) fn compile_function(
messages_types_map: &HashMap<TypeId, MessageId>,
is_entry: bool,
test_decl_ref: Option<DeclRefFunction>,
experimental_storage: bool,
) -> Result<Option<Function>, CompileError> {
// Currently monomorphization of generics is inlined into main() and the functions with generic
// args are still present in the AST declarations, but they can be ignored.
@@ -325,6 +338,7 @@ pub(super) fn compile_function(
logged_types_map,
messages_types_map,
test_decl_ref,
experimental_storage,
)
.map(Some)
}
@@ -340,6 +354,7 @@ pub(super) fn compile_entry_function(
logged_types_map: &HashMap<TypeId, LogId>,
messages_types_map: &HashMap<TypeId, MessageId>,
test_decl_ref: Option<DeclRefFunction>,
experimental_storage: bool,
) -> Result<Function, CompileError> {
let is_entry = true;
compile_function(
@@ -352,10 +367,12 @@ pub(super) fn compile_entry_function(
messages_types_map,
is_entry,
test_decl_ref,
experimental_storage,
)
.map(|f| f.expect("entry point should never contain generics"))
}

#[allow(clippy::too_many_arguments)]
pub(super) fn compile_tests(
engines: Engines<'_>,
context: &mut Context,
@@ -364,6 +381,7 @@ pub(super) fn compile_tests(
logged_types_map: &HashMap<TypeId, LogId>,
messages_types_map: &HashMap<TypeId, MessageId>,
test_fns: &[(ty::TyFunctionDecl, DeclRefFunction)],
experimental_storage: bool,
) -> Result<Vec<Function>, CompileError> {
test_fns
.iter()
@@ -377,6 +395,7 @@ pub(super) fn compile_tests(
logged_types_map,
messages_types_map,
Some(decl_ref.clone()),
experimental_storage,
)
})
.collect()
@@ -394,6 +413,7 @@ fn compile_fn(
logged_types_map: &HashMap<TypeId, LogId>,
messages_types_map: &HashMap<TypeId, MessageId>,
test_decl_ref: Option<DeclRefFunction>,
experimental_storage: bool,
) -> Result<Function, CompileError> {
let type_engine = engines.te();
let decl_engine = engines.de();
@@ -478,6 +498,7 @@ fn compile_fn(
func,
logged_types_map,
messages_types_map,
experimental_storage,
);
let mut ret_val = compiler.compile_code_block(context, md_mgr, body)?;

@@ -516,6 +537,7 @@ fn compile_fn(
Ok(func)
}

#[allow(clippy::too_many_arguments)]
fn compile_abi_method(
context: &mut Context,
md_mgr: &mut MetadataManager,
@@ -524,6 +546,7 @@ fn compile_abi_method(
logged_types_map: &HashMap<TypeId, LogId>,
messages_types_map: &HashMap<TypeId, MessageId>,
engines: Engines<'_>,
experimental_storage: bool,
) -> Result<Function, CompileError> {
let type_engine = engines.te();
let decl_engine = engines.de();
@@ -563,5 +586,6 @@ fn compile_abi_method(
logged_types_map,
messages_types_map,
None,
experimental_storage,
)
}
Loading

0 comments on commit 9685fab

Please sign in to comment.