Skip to content

Commit

Permalink
Cleanup duplicated data from ty::TyDecl (#5751)
Browse files Browse the repository at this point in the history
## Description

This PR cleans up the `ty::TyDecl` struct by removing fields for
duplicated data that we already have access through the engines. Have
not measured the true impact of this, but should have a noticeable
performance and especially memory improvement.

## 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: IGI-111 <igi-111@protonmail.com>
Co-authored-by: Joshua Batty <joshpbatty@gmail.com>
  • Loading branch information
3 people authored May 6, 2024
1 parent 76beb04 commit 1fd9e47
Show file tree
Hide file tree
Showing 40 changed files with 362 additions and 534 deletions.
9 changes: 5 additions & 4 deletions forc-plugins/forc-doc/src/doc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use sway_core::{
language::ty::{TyAstNodeContent, TyDecl, TyImplTrait, TyModule, TyProgram, TySubmodule},
Engines,
};
use sway_types::{BaseIdent, Spanned};
use sway_types::{BaseIdent, Named, Spanned};

mod descriptor;
pub mod module;
Expand Down Expand Up @@ -77,10 +77,11 @@ impl Documentation {
let mut impl_vec: Vec<DocImplTrait> = Vec::new();

match doc.item_body.ty_decl {
TyDecl::StructDecl(ref struct_decl) => {
TyDecl::StructDecl(ref decl) => {
for (impl_trait, module_info) in impl_traits.iter_mut() {
if struct_decl.name.as_str() == impl_trait.implementing_for.span.as_str()
&& struct_decl.name.as_str()
let struct_decl = decl_engine.get_struct(&decl.decl_id);
if struct_decl.name().as_str() == impl_trait.implementing_for.span.as_str()
&& struct_decl.name().as_str()
!= impl_trait.trait_name.suffix.span().as_str()
{
let module_info_override = if let Some(decl_module_info) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ impl<'cfg> ControlFlowGraph<'cfg> {
) -> Result<Self, Vec<CompileError>> {
let mut errors = vec![];

let mut graph = ControlFlowGraph::default();
let mut graph = ControlFlowGraph::new(engines.clone());
// do a depth first traversal and cover individual inner ast nodes
let mut leaves = vec![];
for ast_entrypoint in module_nodes {
Expand Down
43 changes: 13 additions & 30 deletions sway-core/src/control_flow_analysis/dead_code_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,14 @@ fn is_entry_point(node: &TyAstNode, decl_engine: &DeclEngine, tree_type: &TreeTy
TreeType::Contract | TreeType::Library { .. } => match node {
TyAstNode {
content:
TyAstNodeContent::Declaration(TyDecl::FunctionDecl(FunctionDecl {
decl_id,
decl_span: _,
..
})),
TyAstNodeContent::Declaration(TyDecl::FunctionDecl(FunctionDecl { decl_id })),
..
} => {
let decl = decl_engine.get_function(decl_id);
decl.visibility == Visibility::Public || decl.is_test() || decl.is_fallback()
}
TyAstNode {
content:
TyAstNodeContent::Declaration(TyDecl::TraitDecl(TraitDecl {
decl_id,
decl_span: _,
..
})),
content: TyAstNodeContent::Declaration(TyDecl::TraitDecl(TraitDecl { decl_id })),
..
} => decl_engine.get_trait(decl_id).visibility.is_public(),
TyAstNode {
Expand All @@ -84,11 +75,7 @@ fn is_entry_point(node: &TyAstNode, decl_engine: &DeclEngine, tree_type: &TreeTy
} => true,
TyAstNode {
content:
TyAstNodeContent::Declaration(TyDecl::ConstantDecl(ConstantDecl {
decl_id,
decl_span: _,
..
})),
TyAstNodeContent::Declaration(TyDecl::ConstantDecl(ConstantDecl { decl_id })),
..
} => {
let decl = decl_engine.get_constant(decl_id);
Expand Down Expand Up @@ -2234,49 +2221,45 @@ fn construct_dead_code_warning_from_node(
ty::TyAstNode {
content:
ty::TyAstNodeContent::Declaration(ty::TyDecl::FunctionDecl(ty::FunctionDecl {
name,
..
decl_id,
})),
..
} => CompileWarning {
span: name.span(),
span: decl_engine.get(decl_id).name.span(),
warning_content: Warning::DeadFunctionDeclaration,
},
ty::TyAstNode {
content:
ty::TyAstNodeContent::Declaration(ty::TyDecl::StructDecl(ty::StructDecl {
name, ..
})),
ty::TyAstNodeContent::Declaration(ty::TyDecl::StructDecl(ty::StructDecl { decl_id })),
..
} => CompileWarning {
span: name.span(),
span: decl_engine.get(decl_id).name().span(),
warning_content: Warning::DeadStructDeclaration,
},
ty::TyAstNode {
content:
ty::TyAstNodeContent::Declaration(ty::TyDecl::EnumDecl(ty::EnumDecl { name, .. })),
ty::TyAstNodeContent::Declaration(ty::TyDecl::EnumDecl(ty::EnumDecl { decl_id })),
..
} => CompileWarning {
span: name.span(),
span: decl_engine.get(decl_id).name().span(),
warning_content: Warning::DeadEnumDeclaration,
},
ty::TyAstNode {
content:
ty::TyAstNodeContent::Declaration(ty::TyDecl::TraitDecl(ty::TraitDecl { name, .. })),
ty::TyAstNodeContent::Declaration(ty::TyDecl::TraitDecl(ty::TraitDecl { decl_id })),
..
} => CompileWarning {
span: name.span(),
span: decl_engine.get(decl_id).name.span(),
warning_content: Warning::DeadTrait,
},
ty::TyAstNode {
content:
ty::TyAstNodeContent::Declaration(ty::TyDecl::ConstantDecl(ty::ConstantDecl {
name,
..
decl_id,
})),
..
} => CompileWarning {
span: name.span(),
span: decl_engine.get_constant(decl_id).name().span(),
warning_content: Warning::DeadDeclaration,
},
ty::TyAstNode {
Expand Down
24 changes: 19 additions & 5 deletions sway-core/src/control_flow_analysis/flow_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub(crate) use namespace::VariableNamespaceEntry;
pub type EntryPoint = NodeIndex;
pub type ExitPoint = NodeIndex;

#[derive(Clone, Default)]
#[derive(Clone)]
/// A graph that can be used to model the control flow of a Sway program.
/// This graph is used as the basis for all of the algorithms in the control flow analysis portion
/// of the compiler.
Expand All @@ -33,10 +33,24 @@ pub struct ControlFlowGraph<'cfg> {
pub(crate) pending_entry_points_edges: Vec<(NodeIndex, ControlFlowGraphEdge)>,
pub(crate) namespace: ControlFlowNamespace,
pub(crate) decls: HashMap<IdentUnique, NodeIndex>,
pub(crate) engines: Engines,
}

pub type Graph<'cfg> = petgraph::Graph<ControlFlowGraphNode<'cfg>, ControlFlowGraphEdge>;

impl<'cfg> ControlFlowGraph<'cfg> {
pub fn new(engines: Engines) -> Self {
Self {
graph: Default::default(),
entry_points: Default::default(),
pending_entry_points_edges: Default::default(),
namespace: Default::default(),
decls: Default::default(),
engines,
}
}
}

#[derive(Clone)]
pub struct ControlFlowGraphEdge(String);

Expand Down Expand Up @@ -88,10 +102,10 @@ pub enum ControlFlowGraphNode<'cfg> {
}

impl<'cfg> GetDeclIdent for ControlFlowGraphNode<'cfg> {
fn get_decl_ident(&self) -> Option<Ident> {
fn get_decl_ident(&self, engines: &Engines) -> Option<Ident> {
match self {
ControlFlowGraphNode::OrganizationalDominator(_) => None,
ControlFlowGraphNode::ProgramNode { node, .. } => node.get_decl_ident(),
ControlFlowGraphNode::ProgramNode { node, .. } => node.get_decl_ident(engines),
ControlFlowGraphNode::EnumVariant { variant_name, .. } => Some(variant_name.clone()),
ControlFlowGraphNode::MethodDeclaration { method_name, .. } => {
Some(method_name.clone())
Expand Down Expand Up @@ -174,7 +188,7 @@ impl<'cfg> ControlFlowGraph<'cfg> {
self.pending_entry_points_edges.push((to, label));
}
pub(crate) fn add_node<'eng: 'cfg>(&mut self, node: ControlFlowGraphNode<'cfg>) -> NodeIndex {
let ident_opt = node.get_decl_ident();
let ident_opt = node.get_decl_ident(&self.engines);
let node_index = self.graph.add_node(node);
if let Some(ident) = ident_opt {
self.decls.insert(ident.into(), node_index);
Expand All @@ -200,7 +214,7 @@ impl<'cfg> ControlFlowGraph<'cfg> {
}

pub(crate) fn get_node_from_decl(&self, cfg_node: &ControlFlowGraphNode) -> Option<NodeIndex> {
if let Some(ident) = cfg_node.get_decl_ident() {
if let Some(ident) = cfg_node.get_decl_ident(&self.engines) {
if !ident.span().is_dummy() {
self.decls.get(&ident.into()).cloned()
} else {
Expand Down
2 changes: 0 additions & 2 deletions sway-core/src/decl_engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ pub(crate) mod parsed_engine;
pub mod parsed_id;
pub(crate) mod r#ref;
pub(crate) mod replace_decls;
pub(crate) mod template;

use std::collections::BTreeMap;

Expand All @@ -30,7 +29,6 @@ pub(crate) use mapping::*;
pub use r#ref::*;
pub(crate) use replace_decls::*;
use sway_types::Ident;
pub(crate) use template::*;

use crate::{
language::ty::{TyTraitInterfaceItem, TyTraitItem},
Expand Down
13 changes: 0 additions & 13 deletions sway-core/src/decl_engine/ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,6 @@ pub struct DeclRef<I> {
/// The index into the [DeclEngine].
id: I,

/// The type substitution list to apply to the `id` field for type
/// monomorphization.
subst_list: SubstList,

/// The [Span] of the entire declaration.
decl_span: Span,
}
Expand All @@ -75,7 +71,6 @@ impl<I> DeclRef<I> {
DeclRef {
name,
id,
subst_list: SubstList::new(),
decl_span,
}
}
Expand All @@ -88,10 +83,6 @@ impl<I> DeclRef<I> {
&self.id
}

pub(crate) fn subst_list(&self) -> &SubstList {
&self.subst_list
}

pub fn decl_span(&self) -> &Span {
&self.decl_span
}
Expand Down Expand Up @@ -183,7 +174,6 @@ where
// relevant/a reliable source of obj v. obj distinction
decl_span: _,
// temporarily omitted
subst_list: _,
} = self;
let DeclRef {
name: rn,
Expand All @@ -192,7 +182,6 @@ where
// relevant/a reliable source of obj v. obj distinction
decl_span: _,
// temporarily omitted
subst_list: _,
} = other;
ln == rn && decl_engine.get(lid).eq(&decl_engine.get(rid), ctx)
}
Expand All @@ -211,8 +200,6 @@ where
// these fields are not hashed because they aren't relevant/a
// reliable source of obj v. obj distinction
decl_span: _,
// temporarily omitted
subst_list: _,
} = self;
name.hash(state);
decl_engine.get(id).hash(state, engines);
Expand Down
37 changes: 0 additions & 37 deletions sway-core/src/decl_engine/template.rs

This file was deleted.

6 changes: 5 additions & 1 deletion sway-core/src/engine_threading.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::{
fmt,
hash::{BuildHasher, Hash, Hasher},
};
use sway_types::SourceEngine;
use sway_types::{SourceEngine, Span};

#[derive(Clone, Debug, Default)]
pub struct Engines {
Expand Down Expand Up @@ -377,3 +377,7 @@ where
state.finish()
}
}

pub trait SpannedWithEngines {
fn span(&self, engines: &Engines) -> Span;
}
18 changes: 12 additions & 6 deletions sway-core/src/ir_generation/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use sway_ir::{
value::Value,
InstOp, Instruction, Type, TypeContent,
};
use sway_types::{ident::Ident, integer_bits::IntegerBits, span::Spanned, Span};
use sway_types::{ident::Ident, integer_bits::IntegerBits, span::Spanned, Named, Span};
use sway_utils::mapped_stack::MappedStack;

enum ConstEvalError {
Expand Down Expand Up @@ -702,7 +702,7 @@ fn const_eval_codeblock(
Ok(None)
} else {
Err(ConstEvalError::CannotBeEvaluatedToConst {
span: decl.span().clone(),
span: decl.span(lookup.engines).clone(),
})
}
}
Expand All @@ -716,12 +716,12 @@ fn const_eval_codeblock(
})
.flatten()
{
known_consts.push(const_decl.name.clone(), constant);
bindings.push(const_decl.name.clone());
known_consts.push(ty_const_decl.name().clone(), constant);
bindings.push(ty_const_decl.name().clone());
Ok(None)
} else {
Err(ConstEvalError::CannotBeEvaluatedToConst {
span: const_decl.decl_span.clone(),
span: ty_const_decl.span.clone(),
})
}
}
Expand Down Expand Up @@ -1211,7 +1211,13 @@ mod tests {
.declarations
.iter()
.find_map(|x| match x {
ty::TyDecl::FunctionDecl(x) if x.name.as_str() == "f" => Some(x),
ty::TyDecl::FunctionDecl(x) => {
if engines.de().get_function(&x.decl_id).name.as_str() == "f" {
Some(x)
} else {
None
}
}
_ => None,
})
.expect("An function named `f` was not found.");
Expand Down
Loading

0 comments on commit 1fd9e47

Please sign in to comment.