Skip to content

Commit

Permalink
fix dca for encoding v1 autogenerated code (#6006)
Browse files Browse the repository at this point in the history
## Description

This PR is part of #5727 and
fixes a problem with AbiEncode/AbiDecode code generation. The problem
lies in the fact that to encode/decode structs we need to "touch" all
fields. Which makes all fields to pass DCA.

Now, when a `StructFieldAccess` lives inside an autogenerated source,
the "code flow graph" will be built as usual, but no edge will be
created to the struct field, nor to the field owner struct declaration.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [x] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] 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).
- [x] I have requested a review from the relevant team or maintainers.

---------

Co-authored-by: Joshua Batty <[email protected]>
  • Loading branch information
xunilrj and JoshuaBatty authored May 20, 2024
1 parent f9a72da commit 573a9ff
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 21 deletions.
18 changes: 13 additions & 5 deletions sway-core/src/control_flow_analysis/dead_code_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1643,13 +1643,21 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
for leaf in leaves {
graph.add_edge(*leaf, this_ix, "".into());
}
graph.add_edge(this_ix, field_ix, "".into());

if let Some(struct_node_ix) = graph
.namespace
.find_struct_decl(resolved_type_of_parent.suffix.as_str())
// autogenerated code should not increase usage of a struct field
if !engines
.se()
.is_span_in_autogenerated(&expression_span)
.unwrap_or(false)
{
graph.add_edge(this_ix, *struct_node_ix, "".into());
graph.add_edge(this_ix, field_ix, "".into());

if let Some(struct_node_ix) = graph
.namespace
.find_struct_decl(resolved_type_of_parent.suffix.as_str())
{
graph.add_edge(this_ix, *struct_node_ix, "".into());
}
}

Ok(vec![this_ix])
Expand Down
1 change: 0 additions & 1 deletion sway-core/src/type_system/unify/unifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,6 @@ impl<'a> Unifier<'a> {
self.unify(handler, rtp.type_id, etp.type_id, span);
});
} else {
dbg!(rn == en, rvs.len() == evs.len(), rtps.len() == etps.len());
let internal = format!("[{received:?}] versus [{expected:?}]");
let (received, expected) = self.assign_args(received, expected);
handler.emit_err(
Expand Down
12 changes: 2 additions & 10 deletions sway-ir/src/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -816,16 +816,8 @@ impl<'a, 'eng> InstructionVerifier<'a, 'eng> {

fn verify_load(&self, src_val: &Value) -> Result<(), IrError> {
// Just confirm `src_val` is a pointer.
let r = self
.get_ptr_type(src_val, IrError::VerifyLoadFromNonPointer)
.map(|_| ());

if r.is_err() {
let meta = src_val.get_metadata(self.context).unwrap();
dbg!(&self.context.metadata[meta.0], &r);
}

r
self.get_ptr_type(src_val, IrError::VerifyLoadFromNonPointer)
.map(|_| ())
}

fn verify_log(&self, log_val: &Value, log_ty: &Type, log_id: &Value) -> Result<(), IrError> {
Expand Down
8 changes: 8 additions & 0 deletions sway-types/src/source_engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ impl Clone for SourceEngine {
impl SourceEngine {
const AUTOGENERATED_PATH: &'static str = "<autogenerated>";

pub fn is_span_in_autogenerated(&self, span: &crate::Span) -> Option<bool> {
span.source_id().map(|s| self.is_source_id_autogenerated(s))
}

pub fn is_source_id_autogenerated(&self, source_id: &SourceId) -> bool {
self.get_path(source_id).starts_with("<autogenerated>")
}

/// This function retrieves an integer-based source ID for a provided path buffer.
/// If an ID already exists for the given path, the function will return that
/// existing ID. If not, a new ID will be created.
Expand Down
2 changes: 1 addition & 1 deletion test/src/e2e_vm_tests/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub(crate) async fn deploy_contract(file_name: &str, run_config: &RunConfig) ->
true => BuildProfile::RELEASE.to_string(),
false => BuildProfile::DEBUG.to_string(),
},
no_encoding_v1: !dbg!(run_config.experimental.new_encoding),
no_encoding_v1: !run_config.experimental.new_encoding,
..Default::default()
})
.await
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ script;
mod utils;
use utils::Foo;


struct Bar {
value: u64
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
category = "compile"
expected_warnings = 1

expected_warnings = 0
# check: $()struct Bar {
# nextln: $()value: u64
# nextln: $()This struct field is never accessed.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ expected_result_new_encoding = { action = "return_data", value = "00000000000020
validate_abi = true
expected_warnings = 26

#check: $()pub struct S
#nextln: $()This struct field is never accessed.
#nextln: $()pub enum Enum

#check: $()pub struct S
#nextln: $()This struct field is never accessed.
#nextln: $()pub enum Enum

#check: $()pub enum Enum
#nextln: $()This enum is never used.

Expand Down
2 changes: 0 additions & 2 deletions test/src/sdk-harness/Forc.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ members = [
"test_projects/asset_ops",
"test_projects/block",
"test_projects/call_frames",
# TODO Uncomment these when SDK supports encoding V1 for configurables
# https://github.com/FuelLabs/sway/issues/5727
"test_projects/configurables_in_contract",
"test_projects/configurables_in_script",
"test_projects/context",
Expand Down

0 comments on commit 573a9ff

Please sign in to comment.