Skip to content

Commit

Permalink
Implements aliases support in DCA. (FuelLabs#4337)
Browse files Browse the repository at this point in the history 
## Description

Adds support for aliases in DCA.

Fixes FuelLabs#4328 where a DCA warning was emmited while using struct method
defined in an impl self.

Closes FuelLabs#4283
Closes FuelLabs#4328

## Checklist

- [x] 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.
- [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: Sophie Dankel <[email protected]>
  • Loading branch information
@esdrubal @sdankel
esdrubal and sdankel authored Apr 24, 2023
1 parent 1db8385 commit c4368c3
Show file tree
Hide file tree
Showing 49 changed files with 552 additions and 77 deletions.
192 changes: 162 additions & 30 deletions sway-core/src/control_flow_analysis/dead_code_analysis.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
},
transform::{self, AttributesMap},
type_system::TypeInfo,
Engines, TypeEngine, TypeId,
Engines, TypeArgument, TypeEngine, TypeId,
};
use petgraph::{prelude::NodeIndex, visit::Dfs};
use std::collections::{BTreeSet, HashMap};
Expand Down Expand Up @@ -441,7 +441,15 @@ fn connect_declaration<'eng: 'cfg, 'cfg>(
let decl_engine = engines.de();
match decl {
ty::TyDecl::VariableDecl(var_decl) => {
let ty::TyVariableDecl { body, name, .. } = &**var_decl;
let ty::TyVariableDecl {
body,
name,
type_ascription,
..
} = &**var_decl;
// Connect variable declaration node to its type ascription.
connect_type_id(engines, type_ascription.type_id, graph, entry_node)?;
// Connect variable declaration node to body expression.
let result = connect_expression(
engines,
&body.expression,
Expand Down Expand Up @@ -526,6 +534,7 @@ fn connect_declaration<'eng: 'cfg, 'cfg>(
trait_name,
items,
trait_decl_ref,
implementing_for,
..
} = decl_engine.get_impl_trait(decl_id);

Expand All @@ -537,6 +546,7 @@ fn connect_declaration<'eng: 'cfg, 'cfg>(
entry_node,
tree_type,
trait_decl_ref,
implementing_for,
options,
)?;
Ok(leaves.to_vec())
Expand All @@ -546,8 +556,9 @@ fn connect_declaration<'eng: 'cfg, 'cfg>(
connect_storage_declaration(&storage, graph, entry_node, tree_type);
Ok(leaves.to_vec())
}
ty::TyDecl::TypeAliasDecl(_) => {
// TODO - handle type aliases properly. For now, always skip DCA for them.
ty::TyDecl::TypeAliasDecl(ty::TypeAliasDecl { decl_id, .. }) => {
let type_alias = decl_engine.get_type_alias(decl_id);
connect_type_alias_declaration(engines, &type_alias, graph, entry_node)?;
Ok(leaves.to_vec())
}
ty::TyDecl::ErrorRecovery(_) | ty::TyDecl::GenericTypeForFunctionScope(_) => {
Expand Down Expand Up @@ -621,20 +632,28 @@ fn connect_impl_trait<'eng: 'cfg, 'cfg>(
entry_node: NodeIndex,
tree_type: &TreeType,
trait_decl_ref: Option<DeclRef<InterfaceDeclId>>,
implementing_for: TypeArgument,
options: NodeConnectionOptions,
) -> Result<(), CompileError> {
let decl_engine = engines.de();
let trait_decl_node = graph.namespace.find_trait(trait_name).cloned();
match trait_decl_node {
None => {
let node_ix = graph.add_node("External trait".into());
graph.add_edge(entry_node, node_ix, "".into());
}
Some(trait_decl_node) => {
graph.add_edge_from_entry(entry_node, "".into());
graph.add_edge(entry_node, trait_decl_node.trait_idx, "".into());
}
};
// If trait_decl_ref is None then the impl trait is an impl self.
// Impl self does not have any trait to point to.
if trait_decl_ref.is_some() {
let trait_decl_node = graph.namespace.find_trait(trait_name).cloned();
match trait_decl_node {
None => {
let node_ix = graph.add_node("External trait".into());
graph.add_edge(entry_node, node_ix, "".into());
}
Some(trait_decl_node) => {
graph.add_edge_from_entry(entry_node, "".into());
graph.add_edge(entry_node, trait_decl_node.trait_idx, "".into());
}
};
}

connect_type_id(engines, implementing_for.type_id, graph, entry_node)?;

let trait_entry = graph.namespace.find_trait(trait_name).cloned();
// Collect the methods that are directly implemented in the trait.
let mut trait_items_method_names = Vec::new();
Expand Down Expand Up @@ -908,7 +927,14 @@ fn connect_typed_fn_decl<'eng: 'cfg, 'cfg>(
VariableNamespaceEntry {
variable_decl_ix: fn_param_node,
},
)
);

connect_type_id(
engines,
fn_param.type_argument.type_id,
graph,
fn_param_node,
)?;
}

let fn_exit_node = graph.add_node(format!("\"{}\" fn exit", fn_decl.name.as_str()).into());
Expand Down Expand Up @@ -1082,6 +1108,7 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
fn_ref,
contract_call_params,
selector,
call_path_typeid,
..
} => {
let fn_decl = decl_engine.get_function(fn_ref);
Expand All @@ -1102,6 +1129,18 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
// find the function in the namespace
let fn_namespace_entry = graph.namespace.get_function(&fn_decl).cloned();

// connect function entry point to type in function application call path.
if let (Some(call_path_typeid), Some(fn_namespace_entry)) =
(call_path_typeid, fn_namespace_entry.clone())
{
connect_type_id(
engines,
*call_path_typeid,
graph,
fn_namespace_entry.entry_point,
)?;
}

let mut leaves = leaves.to_vec();

// if the parent node exists in this module, then add the monomorphized version
Expand Down Expand Up @@ -1300,6 +1339,7 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
enum_ref,
variant_name,
contents,
call_path_decl,
..
} => {
let enum_decl = decl_engine.get_enum(enum_ref);
Expand All @@ -1309,6 +1349,7 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
&enum_decl,
contents,
variant_name,
call_path_decl,
graph,
leaves,
exit_node,
Expand Down Expand Up @@ -1663,17 +1704,26 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
exp.span.clone(),
options,
),
UnsafeDowncast { exp, .. } => connect_expression(
engines,
&exp.expression,
graph,
leaves,
exit_node,
"unsafe downcast exp",
tree_type,
exp.span.clone(),
options,
),
UnsafeDowncast {
exp,
call_path_decl,
variant: _,
} => {
// Connects call path decl, useful for aliases.
connect_call_path_decl(engines, call_path_decl, graph, leaves)?;

connect_expression(
engines,
&exp.expression,
graph,
leaves,
exit_node,
"unsafe downcast exp",
tree_type,
exp.span.clone(),
options,
)
}
WhileLoop {
body, condition, ..
} => {
Expand Down Expand Up @@ -1862,6 +1912,7 @@ fn connect_enum_instantiation<'eng: 'cfg, 'cfg>(
enum_decl: &ty::TyEnumDecl,
contents: &Option<Box<ty::TyExpression>>,
variant_name: &Ident,
call_path_decl: &ty::TyDecl,
graph: &mut ControlFlowGraph<'cfg>,
leaves: &[NodeIndex],
exit_node: Option<NodeIndex>,
Expand All @@ -1884,6 +1935,9 @@ fn connect_enum_instantiation<'eng: 'cfg, 'cfg>(
(node_idx, node_idx)
});

// Connects call path decl, useful for aliases.
connect_call_path_decl(engines, call_path_decl, graph, leaves)?;

// insert organizational nodes for instantiation of enum
let enum_instantiation_entry_idx = graph.add_node("enum instantiation entry".into());
let enum_instantiation_exit_idx = graph.add_node("enum instantiation exit".into());
Expand Down Expand Up @@ -2060,6 +2114,85 @@ fn connect_storage_declaration<'eng: 'cfg, 'cfg>(
graph.namespace.insert_storage(field_nodes);
}

fn connect_type_alias_declaration<'eng: 'cfg, 'cfg>(
engines: Engines<'eng>,
decl: &ty::TyTypeAliasDecl,
graph: &mut ControlFlowGraph<'cfg>,
entry_node: NodeIndex,
) -> Result<(), CompileError> {
graph
.namespace
.insert_alias(decl.name().clone(), entry_node);

let ty::TyTypeAliasDecl { ty, .. } = decl;
connect_type_id(engines, ty.type_id, graph, entry_node)?;

Ok(())
}

fn connect_type_id<'eng: 'cfg, 'cfg>(
engines: Engines<'eng>,
type_id: TypeId,
graph: &mut ControlFlowGraph<'cfg>,
entry_node: NodeIndex,
) -> Result<(), CompileError> {
let decl_engine = engines.de();
let type_engine = engines.te();

match type_engine.get(type_id) {
TypeInfo::Enum(decl_ref) => {
let decl = decl_engine.get_enum(&decl_ref);
let enum_idx = graph.namespace.find_enum(decl.name());
if let Some(enum_idx) = enum_idx.cloned() {
graph.add_edge(entry_node, enum_idx, "".into());
}
for type_param in decl.type_parameters {
connect_type_id(engines, type_param.type_id, graph, entry_node)?;
}
}
TypeInfo::Struct(decl_ref) => {
let decl = decl_engine.get_struct(&decl_ref);
let struct_idx = graph.namespace.find_struct_decl(decl.name().as_str());
if let Some(struct_idx) = struct_idx.cloned() {
graph.add_edge(entry_node, struct_idx, "".into());
}
for type_param in decl.type_parameters {
connect_type_id(engines, type_param.type_id, graph, entry_node)?;
}
}
TypeInfo::Alias { name, .. } => {
let alias_idx = graph.namespace.get_alias(&name);
if let Some(alias_idx) = alias_idx.cloned() {
graph.add_edge(entry_node, alias_idx, "".into());
}
}
_ => {}
}

Ok(())
}

fn connect_call_path_decl<'eng: 'cfg, 'cfg>(
engines: Engines<'eng>,
call_path_decl: &ty::TyDecl,
graph: &mut ControlFlowGraph<'cfg>,
leaves: &[NodeIndex],
) -> Result<(), CompileError> {
let decl_engine = engines.de();
if let ty::TyDecl::TypeAliasDecl(ty::TypeAliasDecl { decl_id, .. }) = call_path_decl {
let decl = decl_engine.get_type_alias(decl_id);
let alias_idx = graph
.namespace
.get_alias(decl.name())
.cloned()
.unwrap_or_else(|| graph.add_node(format!("extern alias {}", decl.name()).into()));
for leaf in leaves {
graph.add_edge(*leaf, alias_idx, "".into());
}
}
Ok(())
}

/// Checks [AttributesMap] for `#[allow(dead_code)]` usage, if so returns true
/// otherwise returns false.
fn allow_dead_code(attributes: AttributesMap) -> bool {
Expand Down Expand Up @@ -2110,9 +2243,8 @@ fn allow_dead_code_ast_node(decl_engine: &DeclEngine, node: &ty::TyAstNode) -> b
.find(|v| v.name == *variant_name)
.map(|enum_variant| allow_dead_code(enum_variant.attributes))
.unwrap_or(false),
ty::TyDecl::TypeAliasDecl { .. } => {
// TODO - handle type aliases properly. For now, always skip DCA for them.
true
ty::TyDecl::TypeAliasDecl(ty::TypeAliasDecl { decl_id, .. }) => {
allow_dead_code(decl_engine.get_type_alias(decl_id).attributes)
}
ty::TyDecl::ImplTrait { .. } => false,
ty::TyDecl::AbiDecl { .. } => false,
Expand Down
8 changes: 8 additions & 0 deletions sway-core/src/control_flow_analysis/flow_graph/namespace.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ pub struct ControlFlowNamespace {
pub(crate) const_namespace: HashMap<Ident, NodeIndex>,
pub(crate) storage: HashMap<Ident, NodeIndex>,
pub(crate) code_blocks: Vec<ControlFlowCodeBlock>,
pub(crate) alias: HashMap<IdentUnique, NodeIndex>,
}

#[derive(Default, Clone)]
Expand Down Expand Up @@ -240,4 +241,11 @@ impl ControlFlowNamespace {
}
None
}

pub(crate) fn insert_alias(&mut self, name: Ident, entry: NodeIndex) {
self.alias.insert(name.into(), entry);
}
pub(crate) fn get_alias(&self, name: &Ident) -> Option<&NodeIndex> {
self.alias.get(&name.into())
}
}
9 changes: 6 additions & 3 deletions sway-core/src/ir_generation/function.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ impl<'eng> FnCompiler<'eng> {
fn_ref,
selector,
type_binding: _,
call_path_typeid: _,
} => {
if let Some(metadata) = selector {
self.compile_contract_call(
Expand Down Expand Up @@ -402,9 +403,11 @@ impl<'eng> FnCompiler<'eng> {
ty::TyExpressionVariant::AbiName(_) => {
Ok(Value::new_constant(context, Constant::new_unit(context)))
}
ty::TyExpressionVariant::UnsafeDowncast { exp, variant } => {
self.compile_unsafe_downcast(context, md_mgr, exp, variant)
}
ty::TyExpressionVariant::UnsafeDowncast {
exp,
variant,
call_path_decl: _,
} => self.compile_unsafe_downcast(context, md_mgr, exp, variant),
ty::TyExpressionVariant::EnumTag { exp } => {
self.compile_enum_tag(context, md_mgr, exp.to_owned())
}
Expand Down
11 changes: 11 additions & 0 deletions sway-core/src/language/ty/ast_node.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,17 @@ impl TyAstNode {
let decl = decl_engine.get_constant(decl_id);
decl.visibility.is_public()
}
TyAstNode {
content:
TyAstNodeContent::Declaration(TyDecl::TypeAliasDecl(TypeAliasDecl {
decl_id,
..
})),
..
} => {
let decl = decl_engine.get_type_alias(decl_id);
decl.visibility.is_public()
}
_ => false,
},
}
Expand Down
6 changes: 4 additions & 2 deletions sway-core/src/language/ty/declaration/declaration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,8 @@ impl DisplayWithEngines for TyDecl {
TyDecl::FunctionDecl(FunctionDecl { name, .. })
| TyDecl::TraitDecl(TraitDecl { name, .. })
| TyDecl::StructDecl(StructDecl { name, .. })
| TyDecl::TypeAliasDecl(TypeAliasDecl { name, .. })
| TyDecl::ImplTrait(ImplTrait { name, .. })
| TyDecl::EnumDecl(EnumDecl { name, .. }) => name.as_str().into(),
_ => String::new(),
}
Expand Down Expand Up @@ -778,8 +780,8 @@ impl TyDecl {
AbiDecl(_) => "abi",
GenericTypeForFunctionScope(_) => "generic type parameter",
ErrorRecovery(_) => "error",
StorageDecl(_) => "contract storage declaration",
TypeAliasDecl(_) => "type alias declaration",
StorageDecl(_) => "contract storage",
TypeAliasDecl(_) => "type alias",
}
}

Expand Down
Loading

0 comments on commit c4368c3

Please sign in to comment.