Skip to content

Commit

Permalink
Remove DeRef and DeRefMut implementations in the namespace modu…
Browse files Browse the repository at this point in the history
…le (#5577)

## Description

This PR is the first step of a large refactoring of the `namespace`
module.

The PR removes the `DeRef` and `DeRefMut` implementations in the
`namespace` module, as they make refactoring more difficult to keep
track of. This change means that calls to `Root` and `Module` must now
explicitly go through `Namespace`, since that is the type that is passed
along during typechecking.

I'm submitting the PR now because there have been changes to `master`
while I've been working on this, and the changes are really difficult to
merge, so I want `master` to be relatively stable.

Subsequent PRs will:
- Eliminate the `dst` parameter from all import functions (the
destination is always the current module).
- Move all import functions to `Namespace` (they currently reside in
`Module` where they aren't needed, and where they require a clone the
module path to work).
- Move `Namespace::init` to `Root`, and change how it is used (it
currently holds a constant declaration of `CONTRACT_ID` for contract
modules, and that declaration is cloned into every submodule. The nice
way to do this is to declare it in the root module and implicitly import
it into each submodule).
- Eliminate the `Option` part of `Module::name` (modules are not allowed
to be nameless - the option only exists because they root module can in
principle be nameless, but it ought to be named based on the package the
module structure is in).
- Fix the import and name resolution functions so that they don't treat
absolute paths as relative (this should fix some of the performance
issues we're seeing).
- Make `Namespace::root` a reference so that the entire module structure
isn't cloned for every submodule (this requires a change in ownership of
the root module).
- Introduce reexports in the form of `pub use`.

This PR is the first step in fixing #5498. 

## 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.
  • Loading branch information
jjcnn authored Feb 8, 2024
1 parent 52767cc commit 51e8c9d
Show file tree
Hide file tree
Showing 29 changed files with 353 additions and 255 deletions.
24 changes: 12 additions & 12 deletions forc-pkg/src/pkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ pub struct CompiledPackage {
pub program_abi: ProgramABI,
pub storage_slots: Vec<StorageSlot>,
pub bytecode: BuiltPackageBytecode,
pub namespace: namespace::Root,
pub root_module: namespace::Module,
pub warnings: Vec<CompileWarning>,
pub metrics: PerformanceData,
}
Expand Down Expand Up @@ -1656,18 +1656,18 @@ pub fn dependency_namespace(

let _ = namespace.star_import_with_reexports(
&Handler::default(),
engines,
&[CORE, PRELUDE].map(|s| Ident::new_no_span(s.into())),
&[],
engines,
true,
);

if has_std_dep(graph, node) {
let _ = namespace.star_import_with_reexports(
&Handler::default(),
engines,
&[STD, PRELUDE].map(|s| Ident::new_no_span(s.into())),
&[],
engines,
true,
);
}
Expand Down Expand Up @@ -1807,7 +1807,7 @@ pub fn compile(
let storage_slots = typed_program.storage_slots.clone();
let tree_type = typed_program.kind.tree_type();

let namespace = typed_program.root.namespace.clone().into();
let namespace = typed_program.root.namespace.clone();

if handler.has_errors() {
return fail(handler);
Expand Down Expand Up @@ -1932,7 +1932,7 @@ pub fn compile(
storage_slots,
tree_type,
bytecode,
namespace,
root_module: namespace.root_module().clone(),
warnings,
metrics,
};
Expand Down Expand Up @@ -2430,9 +2430,9 @@ pub fn build(
}

if let TreeType::Library = compiled.tree_type {
let mut namespace = namespace::Module::from(compiled.namespace);
namespace.name = Some(Ident::new_no_span(pkg.name.clone()));
lib_namespace_map.insert(node, namespace);
let mut root_module = compiled.root_module;
root_module.name = Some(Ident::new_no_span(pkg.name.clone()));
lib_namespace_map.insert(node, root_module);
}
source_map.insert_dependency(descriptor.manifest_file.dir());

Expand Down Expand Up @@ -2679,9 +2679,9 @@ pub fn check(
match programs.typed.as_ref() {
Ok(typed_program) => {
if let TreeType::Library = typed_program.kind.tree_type() {
let mut namespace = typed_program.root.namespace.clone();
namespace.name = Some(Ident::new_no_span(pkg.name.clone()));
namespace.span = Some(
let mut module = typed_program.root.namespace.module().clone();
module.name = Some(Ident::new_no_span(pkg.name.clone()));
module.span = Some(
Span::new(
manifest.entry_string()?,
0,
Expand All @@ -2690,7 +2690,7 @@ pub fn check(
)
.unwrap(),
);
lib_namespace_map.insert(node, namespace.module().clone());
lib_namespace_map.insert(node, module);
}

source_map.insert_dependency(manifest.dir());
Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/abi_generation/fuel_abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -885,7 +885,7 @@ fn call_path_display(ctx: &mut AbiContext, call_path: &CallPath) -> String {
for (index, prefix) in call_path.prefixes.iter().enumerate() {
let mut skip_prefix = false;
if index == 0 {
if let Some(root_name) = ctx.program.root.namespace.name.clone() {
if let Some(root_name) = &ctx.program.root.namespace.module().name {
if prefix.as_str() == root_name.as_str() {
skip_prefix = true;
}
Expand Down
8 changes: 4 additions & 4 deletions sway-core/src/ir_generation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub fn compile_program<'eng>(
engines,
&mut ctx,
main_function,
&root.namespace,
root.namespace.module(),
declarations,
&logged_types,
&messages_types,
Expand All @@ -77,7 +77,7 @@ pub fn compile_program<'eng>(
engines,
&mut ctx,
main_function,
&root.namespace,
root.namespace.module(),
declarations,
&logged_types,
&messages_types,
Expand All @@ -86,7 +86,7 @@ pub fn compile_program<'eng>(
ty::TyProgramKind::Contract { abi_entries } => compile::compile_contract(
&mut ctx,
abi_entries,
&root.namespace,
root.namespace.module(),
declarations,
&logged_types,
&messages_types,
Expand All @@ -96,7 +96,7 @@ pub fn compile_program<'eng>(
ty::TyProgramKind::Library { .. } => compile::compile_library(
engines,
&mut ctx,
&root.namespace,
root.namespace.module(),
declarations,
&logged_types,
&messages_types,
Expand Down
4 changes: 2 additions & 2 deletions sway-core/src/ir_generation/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,9 @@ pub(crate) fn compile_constants(
module: Module,
module_ns: &namespace::Module,
) -> Result<(), CompileError> {
for decl_name in module_ns.get_all_declared_symbols() {
for decl_name in module_ns.items().get_all_declared_symbols() {
if let Some(ty::TyDecl::ConstantDecl(ty::ConstantDecl { decl_id, .. })) =
module_ns.symbols.get(decl_name)
module_ns.items().symbols.get(decl_name)
{
let const_decl = engines.de().get_constant(decl_id);
let call_path = const_decl.call_path.clone();
Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/ir_generation/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ pub(crate) fn compile_const_decl(
(_, Some(config_val), _) => Ok(Some(config_val)),
(None, None, Some(module_ns)) => {
// See if we it's a global const and whether we can compile it *now*.
let decl = module_ns.check_symbol(&call_path.suffix);
let decl = module_ns.items().check_symbol(&call_path.suffix);
let const_decl = match const_decl {
Some(decl) => Some(decl),
None => None,
Expand Down
12 changes: 6 additions & 6 deletions sway-core/src/language/call_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,10 +313,10 @@ impl CallPath {
let mut is_external = false;
let mut is_absolute = false;

if let Some(use_synonym) = namespace.use_synonyms.get(&self.suffix) {
if let Some(use_synonym) = namespace.module().items().use_synonyms.get(&self.suffix) {
synonym_prefixes = use_synonym.0.clone();
is_absolute = use_synonym.3;
let submodule = namespace.submodule(&[use_synonym.0[0].clone()]);
let submodule = namespace.module().submodule(&[use_synonym.0[0].clone()]);
if let Some(submodule) = submodule {
is_external = submodule.is_external;
}
Expand All @@ -325,7 +325,7 @@ impl CallPath {
let mut prefixes: Vec<Ident> = vec![];

if !is_external {
if let Some(pkg_name) = &namespace.root().module.name {
if let Some(pkg_name) = &namespace.root_module().name {
prefixes.push(pkg_name.clone());
}

Expand All @@ -343,7 +343,7 @@ impl CallPath {
suffix: self.suffix.clone(),
is_absolute: true,
}
} else if let Some(m) = namespace.submodule(&[self.prefixes[0].clone()]) {
} else if let Some(m) = namespace.module().submodule(&[self.prefixes[0].clone()]) {
// If some prefixes are already present, attempt to complete the path by adding the
// package name and the path to the current submodule.
//
Expand All @@ -357,7 +357,7 @@ impl CallPath {
}
} else {
let mut prefixes: Vec<Ident> = vec![];
if let Some(pkg_name) = &namespace.root().module.name {
if let Some(pkg_name) = &namespace.root_module().name {
prefixes.push(pkg_name.clone());
}
for mod_path in namespace.mod_path() {
Expand Down Expand Up @@ -391,7 +391,7 @@ impl CallPath {
let converted = self.to_fullpath(namespace);

if let Some(first) = converted.prefixes.first() {
if namespace.root().name == Some(first.clone()) {
if namespace.root_module().name == Some(first.clone()) {
return converted.lshift();
}
}
Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,7 +587,7 @@ pub fn parsed_to_ast(
&mut ctx,
&mut md_mgr,
module,
&typed_program.root.namespace,
typed_program.root.namespace.module(),
) {
handler.emit_err(e);
}
Expand Down
3 changes: 1 addition & 2 deletions sway-core/src/semantic_analysis/ast_node/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,7 @@ impl ty::TyCodeBlock {

let never_decl_opt = ctx
.namespace
.root()
.resolve_symbol(
.resolve_root_symbol(
&Handler::default(),
engines,
&never_mod_path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ impl ty::TyAbiDecl {
all_items.push(TyImplItem::Constant(decl_ref.clone()));
let const_shadowing_mode = ctx.const_shadowing_mode();
let generic_shadowing_mode = ctx.generic_shadowing_mode();
let _ = ctx.namespace.insert_symbol(
let _ = ctx.namespace.module_mut().items_mut().insert_symbol(
handler,
const_name.clone(),
ty::TyDecl::ConstantDecl(ty::ConstantDecl {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ impl<'a, 'b> AutoImplAbiEncodeContext<'a, 'b> {
// skip module "core"
// Because of ordering, we cannot guarantee auto impl
// for structs inside "core"
if matches!(self.ctx.namespace.root().name.as_ref(), Some(x) if x.as_str() == "core") {
if matches!(self.ctx.namespace.root_module_name(), Some(x) if x.as_str() == "core") {
return false;
}

Expand Down Expand Up @@ -531,6 +531,8 @@ impl<'a, 'b> AutoImplAbiEncodeContext<'a, 'b> {
let handler = Handler::default();
self.ctx
.namespace
.module_mut()
.items_mut()
.implemented_traits
.check_if_trait_constraints_are_satisfied_for_type(
&handler,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,8 @@ impl TyDecl {

// declarations are not allowed
ctx.namespace
.module_mut()
.items_mut()
.set_storage_declaration(handler, decl_ref.clone())?;
decl_ref.into()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl ty::TyFunctionParameter {
pub fn insert_into_namespace(&self, handler: &Handler, ctx: TypeCheckContext) {
let const_shadowing_mode = ctx.const_shadowing_mode();
let generic_shadowing_mode = ctx.generic_shadowing_mode();
let _ = ctx.namespace.insert_symbol(
let _ = ctx.namespace.module_mut().items_mut().insert_symbol(
handler,
self.name.clone(),
ty::TyDecl::VariableDecl(Box::new(ty::TyVariableDecl {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,8 @@ fn type_check_trait_implementation(
// Check to see if the type that we are implementing for implements the
// supertraits of this trait.
ctx.namespace
.module_mut()
.items_mut()
.implemented_traits
.check_if_trait_constraints_are_satisfied_for_type(
handler,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ impl TyTraitDecl {
all_items.push(TyImplItem::Constant(decl_ref.clone()));
let const_shadowing_mode = ctx.const_shadowing_mode();
let generic_shadowing_mode = ctx.generic_shadowing_mode();
let _ = ctx.namespace.insert_symbol(
let _ = ctx.namespace.module_mut().items_mut().insert_symbol(
handler,
const_name.clone(),
ty::TyDecl::ConstantDecl(ty::ConstantDecl {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1023,16 +1023,18 @@ impl ty::TyExpression {
let decl_engine = ctx.engines.de();
let engines = ctx.engines();

if !ctx.namespace.has_storage_declared() {
if !ctx.namespace.module().items().has_storage_declared() {
return Err(handler.emit_err(CompileError::NoDeclaredStorage { span: span.clone() }));
}

let storage_fields = ctx
.namespace
.module()
.items()
.get_storage_field_descriptors(handler, decl_engine)?;

// Do all namespace checking here!
let (storage_access, mut access_type) = ctx.namespace.apply_storage_load(
let (storage_access, mut access_type) = ctx.namespace.module().items().apply_storage_load(
handler,
ctx.engines,
ctx.namespace,
Expand All @@ -1050,7 +1052,7 @@ impl ty::TyExpression {
let storage_key_ident = Ident::new_with_override("StorageKey".into(), span.clone());

// Search for the struct declaration with the call path above.
let storage_key_decl_opt = ctx.namespace.root().resolve_symbol(
let storage_key_decl_opt = ctx.namespace.resolve_root_symbol(
handler,
engines,
&storage_key_mod_path,
Expand Down Expand Up @@ -1254,7 +1256,7 @@ impl ty::TyExpression {
path.push(before.inner.clone());
let not_module = {
let h = Handler::default();
ctx.namespace.check_submodule(&h, &path).is_err()
ctx.namespace.module().check_submodule(&h, &path).is_err()
};

// Not a module? Not a `Enum::Variant` either?
Expand Down Expand Up @@ -1373,6 +1375,7 @@ impl ty::TyExpression {
is_module = {
let call_path_binding = unknown_call_path_binding.clone();
ctx.namespace
.module()
.check_submodule(
&module_probe_handler,
&[
Expand Down Expand Up @@ -2014,13 +2017,14 @@ impl ty::TyExpression {
}
};
let names_vec = names_vec.into_iter().rev().collect::<Vec<_>>();
let (ty_of_field, _ty_of_parent) = ctx.namespace.find_subfield_type(
handler,
ctx.engines(),
ctx.namespace,
&base_name,
&names_vec,
)?;
let (ty_of_field, _ty_of_parent) =
ctx.namespace.module().items().find_subfield_type(
handler,
ctx.engines(),
ctx.namespace,
&base_name,
&names_vec,
)?;
// type check the reassignment
let ctx = ctx.with_type_annotation(ty_of_field).with_help_text("");
let rhs_span = rhs.span();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,8 +439,7 @@ pub(crate) fn resolve_method_name(
.namespace
.find_module_path(&call_path_binding.inner.prefixes);
ctx.namespace
.root()
.check_submodule(handler, &type_info_prefix)?;
.check_absolute_path_to_submodule(handler, &type_info_prefix)?;

// find the method
let decl_ref = ctx.find_method_for_type(
Expand All @@ -464,7 +463,7 @@ pub(crate) fn resolve_method_name(
let mut module_path = call_path.prefixes.clone();
if let (Some(root_mod), Some(root_name)) = (
module_path.first().cloned(),
ctx.namespace.root().name.clone(),
ctx.namespace.root_module_name().clone(),
) {
if root_mod.as_str() == root_name.as_str() {
module_path.remove(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ pub(crate) fn struct_instantiation(
// find the module that the struct decl is in
let type_info_prefix = ctx.namespace.find_module_path(&prefixes);
ctx.namespace
.root()
.check_submodule(handler, &type_info_prefix)?;
.check_absolute_path_to_submodule(handler, &type_info_prefix)?;

// resolve the type of the struct decl
let type_id = ctx
Expand Down Expand Up @@ -255,6 +254,8 @@ pub(crate) fn struct_instantiation(
// but that would be a way too much of suggestions, and moreover, it is also not a design pattern/guideline
// that we wish to encourage.
namespace
.module()
.items()
.get_items_for_type(engines, struct_type_id)
.iter()
.filter_map(|item| match item {
Expand Down
4 changes: 3 additions & 1 deletion sway-core/src/semantic_analysis/ast_node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ impl ty::TyAstNode {
content: match node.content.clone() {
AstNodeContent::UseStatement(a) => {
let mut is_external = false;
if let Some(submodule) = ctx.namespace.submodule(&[a.call_path[0].clone()]) {
if let Some(submodule) =
ctx.namespace.module().submodule(&[a.call_path[0].clone()])
{
is_external = submodule.is_external;
}
let path = if is_external || a.is_absolute {
Expand Down
Loading

0 comments on commit 51e8c9d

Please sign in to comment.