Skip to content

Commit

Permalink
Fix DCA handling for __revert intrinsic and std::revert. (#5235)
Browse files Browse the repository at this point in the history
## 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 #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ć <[email protected]>
  • Loading branch information
tritao and ironcev authored Nov 3, 2023
1 parent 0baf365 commit 74b2422
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 7 deletions.
4 changes: 0 additions & 4 deletions forc-pkg/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,3 @@ pub use manifest::{
};
#[doc(inline)]
pub use pkg::*;

const CORE: &str = "core";
const STD: &str = "std";
const PRELUDE: &str = "prelude";
2 changes: 1 addition & 1 deletion forc-pkg/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion forc-pkg/src/pkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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};
Expand Down
24 changes: 23 additions & 1 deletion sway-core/src/control_flow_analysis/dead_code_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<CompileWarning> {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}

Expand Down
4 changes: 4 additions & 0 deletions sway-types/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Original file line number Diff line number Diff line change
@@ -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"]
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
name = "revert"
implicit-std = false

[dependencies]
core = { path = "../../../../../../../sway-lib-core" }
std = { path = "../../../../../../../sway-lib-std" }
Original file line number Diff line number Diff line change
@@ -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
}



Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit 74b2422

Please sign in to comment.