From 74b2422bafc1f438985868b6a9dc00be838d3b77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Matos?= Date: Fri, 3 Nov 2023 12:33:25 +0000 Subject: [PATCH] Fix DCA handling for `__revert` intrinsic and `std::revert`. (#5235) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This fixes dead code analysis for `__revert` and `std::revert`. ~~We add a new `NodeReturnOptions` return struct to `connect_expression` which can be used to signal to the parent code block handler that the expression sibling node should not have its leaves connected.~~ Fixes https://github.com/FuelLabs/sway/issues/5214. ## 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] 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: Igor Rončević --- forc-pkg/src/lib.rs | 4 -- forc-pkg/src/manifest.rs | 2 +- forc-pkg/src/pkg.rs | 2 +- .../dead_code_analysis.rs | 24 ++++++++++- sway-types/src/constants.rs | 4 ++ .../should_pass/dca/revert/Forc.lock | 16 +++++++ .../should_pass/dca/revert/Forc.toml | 10 +++++ .../should_pass/dca/revert/src/main.sw | 43 +++++++++++++++++++ .../should_pass/dca/revert/test.toml | 17 ++++++++ 9 files changed, 115 insertions(+), 7 deletions(-) create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/Forc.lock create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/Forc.toml create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/src/main.sw create mode 100644 test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/test.toml diff --git a/forc-pkg/src/lib.rs b/forc-pkg/src/lib.rs index 8581a8423ee..7143194d5d5 100644 --- a/forc-pkg/src/lib.rs +++ b/forc-pkg/src/lib.rs @@ -15,7 +15,3 @@ pub use manifest::{ }; #[doc(inline)] pub use pkg::*; - -const CORE: &str = "core"; -const STD: &str = "std"; -const PRELUDE: &str = "prelude"; diff --git a/forc-pkg/src/manifest.rs b/forc-pkg/src/manifest.rs index eee211ba26f..50188dbd5c2 100644 --- a/forc-pkg/src/manifest.rs +++ b/forc-pkg/src/manifest.rs @@ -615,7 +615,7 @@ impl PackageManifest { /// Note: If only `core` is specified, we are unable to implicitly add `std` as we cannot /// guarantee that the user's `core` is compatible with the implicit `std`. fn implicitly_include_std_if_missing(&mut self) { - use crate::{CORE, STD}; + use sway_types::constants::{CORE, STD}; // Don't include `std` if: // - this *is* `core` or `std`. // - either `core` or `std` packages are already specified. diff --git a/forc-pkg/src/pkg.rs b/forc-pkg/src/pkg.rs index a0f9a21270f..8d0bf1ebc04 100644 --- a/forc-pkg/src/pkg.rs +++ b/forc-pkg/src/pkg.rs @@ -2,7 +2,6 @@ use crate::{ lock::Lock, manifest::{BuildProfile, Dependency, ManifestFile, MemberManifestFiles, PackageManifestFile}, source::{self, IPFSNode, Source}, - CORE, PRELUDE, STD, }; use anyhow::{anyhow, bail, Context, Error, Result}; use forc_util::{ @@ -46,6 +45,7 @@ use sway_core::{ BuildTarget, Engines, FinalizedEntry, }; use sway_error::{error::CompileError, handler::Handler, warning::CompileWarning}; +use sway_types::constants::{CORE, PRELUDE, STD}; use sway_types::{Ident, Span, Spanned}; use sway_utils::{constants, time_expr, PerformanceData, PerformanceMetric}; use tracing::{info, warn}; diff --git a/sway-core/src/control_flow_analysis/dead_code_analysis.rs b/sway-core/src/control_flow_analysis/dead_code_analysis.rs index e907eceab3e..95efb30bff0 100644 --- a/sway-core/src/control_flow_analysis/dead_code_analysis.rs +++ b/sway-core/src/control_flow_analysis/dead_code_analysis.rs @@ -12,12 +12,17 @@ use crate::{ }; use petgraph::{prelude::NodeIndex, visit::Dfs}; use std::collections::{BTreeSet, HashMap}; +use sway_ast::Intrinsic; use sway_error::{error::CompileError, type_error::TypeError}; use sway_error::{ handler::Handler, warning::{CompileWarning, Warning}, }; -use sway_types::{constants::ALLOW_DEAD_CODE_NAME, span::Span, Ident, Named, Spanned}; +use sway_types::{ + constants::{ALLOW_DEAD_CODE_NAME, STD}, + span::Span, + Ident, Named, Spanned, +}; impl<'cfg> ControlFlowGraph<'cfg> { pub(crate) fn find_dead_code(&self, decl_engine: &DeclEngine) -> Vec { @@ -1263,6 +1268,17 @@ fn connect_expression<'eng: 'cfg, 'cfg>( for leaf in current_leaf { graph.add_edge(leaf, fn_exit_point, "".into()); } + + // check for std::revert and connect to the exit node if that's the case. + // we are guaranteed a full call path here since the type checker calls to_fullpath. + if let Some(prefix) = fn_decl.call_path.prefixes.first() { + if prefix.as_str() == STD && fn_decl.call_path.suffix.as_str() == "revert" { + if let Some(exit_node) = exit_node { + graph.add_edge(fn_exit_point, exit_node, "revert".into()); + return Ok(vec![]); + } + } + } // the exit points get connected to an exit node for the application if !is_external { if let Some(exit_node) = exit_node { @@ -1876,6 +1892,12 @@ fn connect_intrinsic_function<'eng: 'cfg, 'cfg>( accum.append(&mut res); Ok::<_, CompileError>(accum) })?; + if let Some(exit_node) = exit_node { + if kind == &Intrinsic::Revert { + graph.add_edge(node, exit_node, "revert".into()); + result = vec![]; + } + } Ok(result) } diff --git a/sway-types/src/constants.rs b/sway-types/src/constants.rs index b10a918ffdc..190da5a6b34 100644 --- a/sway-types/src/constants.rs +++ b/sway-types/src/constants.rs @@ -64,3 +64,7 @@ pub const VALID_ATTRIBUTE_NAMES: &[&str] = &[ CFG_ATTRIBUTE_NAME, DEPRECATED_ATTRIBUTE_NAME, ]; + +pub const CORE: &str = "core"; +pub const STD: &str = "std"; +pub const PRELUDE: &str = "prelude"; diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/Forc.lock b/test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/Forc.lock new file mode 100644 index 00000000000..792bc80d288 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/Forc.lock @@ -0,0 +1,16 @@ +[[package]] +name = "core" +source = "path+from-root-32A1D8C54897C96A" + +[[package]] +name = "revert" +source = "member" +dependencies = [ + "core", + "std", +] + +[[package]] +name = "std" +source = "path+from-root-32A1D8C54897C96A" +dependencies = ["core"] diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/Forc.toml b/test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/Forc.toml new file mode 100644 index 00000000000..b45913cba99 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/Forc.toml @@ -0,0 +1,10 @@ +[project] +authors = ["Fuel Labs "] +entry = "main.sw" +license = "Apache-2.0" +name = "revert" +implicit-std = false + +[dependencies] +core = { path = "../../../../../../../sway-lib-core" } +std = { path = "../../../../../../../sway-lib-std" } diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/src/main.sw b/test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/src/main.sw new file mode 100644 index 00000000000..38295bc8496 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/src/main.sw @@ -0,0 +1,43 @@ +script; + +fn revert_01() -> u32 { + __revert(0); + + 422 +} + +fn revert_02() -> u32 { + __revert(0); + + return 2; +} + +fn revert_03(a: u64) -> u32 { + if a > 0 { + __revert(0); + return 1; + } + else { + return 0; + } + + return 3; +} + +fn std_revert_04() -> u32 { + revert(0); + + return 4; +} + + +fn main() -> u32 { + let _ = revert_01(); + let _ = revert_02(); + let _ = revert_03(0); + let _ = std_revert_04(); + 0 +} + + + diff --git a/test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/test.toml b/test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/test.toml new file mode 100644 index 00000000000..02cf4853be2 --- /dev/null +++ b/test/src/e2e_vm_tests/test_programs/should_pass/dca/revert/test.toml @@ -0,0 +1,17 @@ +category = "compile" +expected_warnings = 5 + +# check: $()422 +# nextln: $()This code is unreachable. + +# check: $()return 2; +# nextln: $()This code is unreachable. + +# check: $()return 1; +# nextln: $()This code is unreachable. + +# check: $()return 3; +# nextln: $()This code is unreachable. + +# check: $()return 4; +# nextln: $()This code is unreachable.