Skip to content

Commit

Permalink
Add a new name field on DeclId and remove some unnecessary lookup…
Browse files Browse the repository at this point in the history
…s in the `DeclEngine` (#4028)

## Description

This PR is a subset of #3744. It adds a `name` field to `DeclId` which
contains the name of the declaration. This is helpful in that it allows
us to bypass lookups in the `DeclEngine` for trivial name info and is
also a subset of #3744.

This PR also removes/refactors some of these unnecessary lookups in the
`DeclEngine` in light of the new `name` field.

## Checklist

- [x] I have linked to any relevant issues.
- [ ] 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).
- [ ] 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: emilyaherbert <emily.herbert@fuel.sh>
  • Loading branch information
emilyaherbert and emilyaherbert authored Feb 8, 2023
1 parent 2d2a8e5 commit 6b3ae49
Show file tree
Hide file tree
Showing 20 changed files with 259 additions and 431 deletions.
7 changes: 6 additions & 1 deletion sway-core/src/asm_generation/fuel/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::{
use sway_ir::*;

use either::Either;
use sway_types::Ident;

/// A summary of the adopted calling convention:
///
Expand Down Expand Up @@ -150,7 +151,11 @@ impl<'ir> FuelAsmBuilder<'ir> {
let span = self.md_mgr.md_to_span(self.context, md);
let test_decl_index = self.md_mgr.md_to_test_decl_index(self.context, md);
let test_decl_id = match (&span, &test_decl_index) {
(Some(span), Some(decl_index)) => Some(DeclId::new(*decl_index, span.clone())),
(Some(span), Some(decl_index)) => Some(DeclId::new(
Ident::new(span.clone()),
*decl_index,
span.clone(),
)),
_ => None,
};
let comment = format!(
Expand Down
30 changes: 12 additions & 18 deletions sway-core/src/control_flow_analysis/analyze_return_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ fn connect_node<'eng: 'cfg, 'cfg>(
..
})
| ty::TyAstNodeContent::ImplicitReturnExpression(_) => {
let this_index = graph.add_node(engines, node.into());
let this_index = graph.add_node(node.into());
for leaf_ix in leaves {
graph.add_edge(*leaf_ix, this_index, "".into());
}
Expand All @@ -149,14 +149,14 @@ fn connect_node<'eng: 'cfg, 'cfg>(
// An abridged version of the dead code analysis for a while loop
// since we don't really care about what the loop body contains when detecting
// divergent paths
let node = graph.add_node(engines, node.into());
let node = graph.add_node(node.into());
for leaf in leaves {
graph.add_edge(*leaf, node, "while loop entry".into());
}
Ok(NodeConnection::NextStep(vec![node]))
}
ty::TyAstNodeContent::Expression(ty::TyExpression { .. }) => {
let entry = graph.add_node(engines, node.into());
let entry = graph.add_node(node.into());
// insert organizational dominator node
// connected to all current leaves
for leaf in leaves {
Expand Down Expand Up @@ -189,15 +189,15 @@ fn connect_declaration<'eng: 'cfg, 'cfg>(
| StorageDeclaration(_)
| GenericTypeForFunctionScope { .. } => Ok(leaves.to_vec()),
VariableDeclaration(_) | ConstantDeclaration(_) => {
let entry_node = graph.add_node(engines, node.into());
let entry_node = graph.add_node(node.into());
for leaf in leaves {
graph.add_edge(*leaf, entry_node, "".into());
}
Ok(vec![entry_node])
}
FunctionDeclaration(decl_id) => {
let fn_decl = decl_engine.get_function(decl_id.clone(), &decl.span())?;
let entry_node = graph.add_node(engines, node.into());
let entry_node = graph.add_node(node.into());
for leaf in leaves {
graph.add_edge(*leaf, entry_node, "".into());
}
Expand All @@ -210,7 +210,7 @@ fn connect_declaration<'eng: 'cfg, 'cfg>(
methods,
..
} = decl_engine.get_impl_trait(decl_id.clone(), &span)?;
let entry_node = graph.add_node(engines, node.into());
let entry_node = graph.add_node(node.into());
for leaf in leaves {
graph.add_edge(*leaf, entry_node, "".into());
}
Expand Down Expand Up @@ -239,15 +239,12 @@ fn connect_impl_trait<'eng: 'cfg, 'cfg>(
// insert method declarations into the graph
for method_decl_id in methods {
let fn_decl = decl_engine.get_function(method_decl_id.clone(), &trait_name.span())?;
let fn_decl_entry_node = graph.add_node(
let fn_decl_entry_node = graph.add_node(ControlFlowGraphNode::MethodDeclaration {
span: fn_decl.span.clone(),
method_name: fn_decl.name.clone(),
method_decl_id: method_decl_id.clone(),
engines,
ControlFlowGraphNode::MethodDeclaration {
span: fn_decl.span.clone(),
method_name: fn_decl.name.clone(),
method_decl_id: method_decl_id.clone(),
engines,
},
);
});
graph.add_edge(entry_node, fn_decl_entry_node, "".into());
// connect the impl declaration node to the functions themselves, as all trait functions are
// public if the trait is in scope
Expand Down Expand Up @@ -288,10 +285,7 @@ fn connect_typed_fn_decl<'eng: 'cfg, 'cfg>(
_span: Span,
) -> Result<(), CompileError> {
let type_engine = engines.te();
let fn_exit_node = graph.add_node(
engines,
format!("\"{}\" fn exit", fn_decl.name.as_str()).into(),
);
let fn_exit_node = graph.add_node(format!("\"{}\" fn exit", fn_decl.name.as_str()).into());
let return_nodes =
depth_first_insertion_code_block(engines, &fn_decl.body, graph, &[entry_node])?;
for node in return_nodes {
Expand Down
Loading

0 comments on commit 6b3ae49

Please sign in to comment.