Skip to content

Commit

Permalink
Refactor: Imports must use absolute paths (#5697)
Browse files Browse the repository at this point in the history
## Description

This PR contains the final bit of refactoring before I start fixing the
import problems mentioned in #5498.

The semantics of imports has so far been implemented in the `Module`
struct, and has assumed that the destination path of an import was
relative to `self`. In particular, this allows imports to any submodule
of the current module.

However, whenever that logic was triggered by the compiler the
destination path was always absolute. The only reason this worked was
that the logic was only ever triggered on the root module.

To reflect this behavior I have therefore moved the import semantics to
the `Root` struct, and made it clear in the comments that the paths are
assumed to be absolute.

This also reflects the fact that once the module structure has been
populated with the imported entities we don't actually need the import
logic anymore, so it shouldn't be located in the `Module` struct. (Name
resolution should obviously stay in the `Module` struct, so that logic
has not been moved).

I have left a couple of TODOs in the code to remind myself where I need
to make changes when I start implementing `pub use` (see #3113 ).

## 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>
  • Loading branch information
jjcnn and IGI-111 committed Mar 8, 2024
1 parent a0b8062 commit 13731d9
Show file tree
Hide file tree
Showing 13 changed files with 501 additions and 534 deletions.
26 changes: 13 additions & 13 deletions forc-pkg/src/pkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1595,11 +1595,11 @@ pub fn dependency_namespace(
engines: &Engines,
contract_id_value: Option<ContractIdConst>,
experimental: sway_core::ExperimentalFlags,
) -> Result<namespace::Module, vec1::Vec1<CompileError>> {
) -> Result<namespace::Root, vec1::Vec1<CompileError>> {
// TODO: Clean this up when config-time constants v1 are removed.
let node_idx = &graph[node];
let name = Some(Ident::new_no_span(node_idx.name.clone()));
let mut namespace = if let Some(contract_id_value) = contract_id_value {
let mut root_module = if let Some(contract_id_value) = contract_id_value {
namespace::Module::default_with_contract_id(
engines,
name.clone(),
Expand All @@ -1610,9 +1610,9 @@ pub fn dependency_namespace(
namespace::Module::default()
};

namespace.is_external = true;
namespace.name = name;
namespace.visibility = Visibility::Public;
root_module.is_external = true;
root_module.name = name;
root_module.visibility = Visibility::Public;

// Add direct dependencies.
let mut core_added = false;
Expand Down Expand Up @@ -1647,7 +1647,7 @@ pub fn dependency_namespace(
ns
}
};
namespace.insert_submodule(dep_name, dep_namespace);
root_module.insert_submodule(dep_name, dep_namespace);
let dep = &graph[dep_node];
if dep.name == CORE {
core_added = true;
Expand All @@ -1658,29 +1658,29 @@ pub fn dependency_namespace(
if !core_added {
if let Some(core_node) = find_core_dep(graph, node) {
let core_namespace = &lib_namespace_map[&core_node];
namespace.insert_submodule(CORE.to_string(), core_namespace.clone());
root_module.insert_submodule(CORE.to_string(), core_namespace.clone());
}
}

let _ = namespace.star_import_with_reexports(
let mut root = namespace::Root::from(root_module);

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

if has_std_dep(graph, node) {
let _ = namespace.star_import_with_reexports(
let _ = root.star_import_with_reexports(
&Handler::default(),
engines,
&[STD, PRELUDE].map(|s| Ident::new_no_span(s.into())),
&[],
true,
);
}

Ok(namespace)
Ok(root)
}

/// Find the `std` dependency, if it is a direct one, of the given node.
Expand Down Expand Up @@ -1757,7 +1757,7 @@ pub fn compile(
pkg: &PackageDescriptor,
profile: &BuildProfile,
engines: &Engines,
namespace: namespace::Module,
namespace: namespace::Root,
source_map: &mut SourceMap,
) -> Result<CompiledPackage> {
let mut metrics = PerformanceData::default();
Expand Down
4 changes: 2 additions & 2 deletions sway-core/src/ir_generation/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1185,12 +1185,12 @@ mod tests {
let handler = Handler::default();
let mut context = Context::new(engines.se(), sway_ir::ExperimentalFlags::default());
let mut md_mgr = MetadataManager::default();
let core_lib = namespace::Module {
let core_lib = namespace::Root::from(namespace::Module {
name: Some(sway_types::Ident::new_no_span(
"assert_is_constant_test".to_string(),
)),
..Default::default()
};
});

let r = crate::compile_to_ast(
&handler,
Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/language/call_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ impl CallPath {
.get(&self.suffix)
{
synonym_prefixes = use_synonym.0.clone();
is_absolute = use_synonym.3;
is_absolute = true;
let submodule = namespace.module().submodule(&[use_synonym.0[0].clone()]);
if let Some(submodule) = submodule {
is_external = submodule.is_external;
Expand Down
8 changes: 4 additions & 4 deletions sway-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ pub fn parsed_to_ast(
handler: &Handler,
engines: &Engines,
parse_program: &parsed::ParseProgram,
initial_namespace: namespace::Module,
initial_namespace: namespace::Root,
build_config: Option<&BuildConfig>,
package_name: &str,
retrigger_compilation: Option<Arc<AtomicBool>>,
Expand Down Expand Up @@ -638,7 +638,7 @@ pub fn compile_to_ast(
handler: &Handler,
engines: &Engines,
input: Arc<str>,
initial_namespace: namespace::Module,
initial_namespace: namespace::Root,
build_config: Option<&BuildConfig>,
package_name: &str,
retrigger_compilation: Option<Arc<AtomicBool>>,
Expand Down Expand Up @@ -735,7 +735,7 @@ pub fn compile_to_asm(
handler: &Handler,
engines: &Engines,
input: Arc<str>,
initial_namespace: namespace::Module,
initial_namespace: namespace::Root,
build_config: BuildConfig,
package_name: &str,
) -> Result<CompiledAsm, ErrorEmitted> {
Expand Down Expand Up @@ -892,7 +892,7 @@ pub fn compile_to_bytecode(
handler: &Handler,
engines: &Engines,
input: Arc<str>,
initial_namespace: namespace::Module,
initial_namespace: namespace::Root,
build_config: BuildConfig,
source_map: &mut SourceMap,
package_name: &str,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2433,7 +2433,8 @@ mod tests {
type_annotation: TypeId,
experimental: ExperimentalFlags,
) -> Result<ty::TyExpression, ErrorEmitted> {
let mut namespace = Namespace::init_root(namespace::Module::default());
let root_module = namespace::Root::from(namespace::Module::default());
let mut namespace = Namespace::init_root(root_module);
let ctx = TypeCheckContext::from_namespace(&mut namespace, engines, experimental)
.with_type_annotation(type_annotation);
ty::TyExpression::type_check(handler, ctx, expr)
Expand Down
16 changes: 4 additions & 12 deletions sway-core/src/semantic_analysis/ast_node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ impl ty::TyAstNode {
ImportType::Star => {
// try a standard starimport first
let star_import_handler = Handler::default();
let import =
ctx.star_import(&star_import_handler, &path, a.is_absolute);
let import = ctx.star_import(&star_import_handler, &path);
if import.is_ok() {
handler.append(star_import_handler);
import
Expand All @@ -62,7 +61,6 @@ impl ty::TyAstNode {
&variant_import_handler,
path,
enum_name,
a.is_absolute,
);
if variant_import.is_ok() {
handler.append(variant_import_handler);
Expand All @@ -78,18 +76,13 @@ impl ty::TyAstNode {
}
}
ImportType::SelfImport(_) => {
ctx.self_import(handler, &path, a.alias.clone(), a.is_absolute)
ctx.self_import(handler, &path, a.alias.clone())
}
ImportType::Item(ref s) => {
// try a standard item import first
let item_import_handler = Handler::default();
let import = ctx.item_import(
&item_import_handler,
&path,
s,
a.alias.clone(),
a.is_absolute,
);
let import =
ctx.item_import(&item_import_handler, &path, s, a.alias.clone());

if import.is_ok() {
handler.append(item_import_handler);
Expand All @@ -104,7 +97,6 @@ impl ty::TyAstNode {
enum_name,
s,
a.alias.clone(),
a.is_absolute,
);
if variant_import.is_ok() {
handler.append(variant_import_handler);
Expand Down
7 changes: 3 additions & 4 deletions sway-core/src/semantic_analysis/namespace/lexical_scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ pub(crate) enum GlobImport {
}

pub(super) type SymbolMap = im::OrdMap<Ident, ty::TyDecl>;
// The final `bool` field of `UseSynonyms` is true if the `Vec<Ident>` path is absolute.
pub(super) type UseSynonyms = im::HashMap<Ident, (Vec<Ident>, GlobImport, ty::TyDecl, bool)>;
// The `Vec<Ident>` path is absolute.
pub(super) type UseSynonyms = im::HashMap<Ident, (Vec<Ident>, GlobImport, ty::TyDecl)>;
pub(super) type UseAliases = im::HashMap<String, Ident>;

/// Represents a lexical scope integer-based identifier, which can be used to reference
Expand Down Expand Up @@ -261,8 +261,7 @@ impl Items {
append_shadowing_error(ident, decl, false, false, &item, const_shadowing_mode);
}

if let Some((ident, (_, GlobImport::No, decl, _))) = self.use_synonyms.get_key_value(&name)
{
if let Some((ident, (_, GlobImport::No, decl))) = self.use_synonyms.get_key_value(&name) {
append_shadowing_error(
ident,
decl,
Expand Down
Loading

0 comments on commit 13731d9

Please sign in to comment.