diff --git a/external-crates/move/crates/move-analyzer/src/symbols/compilation.rs b/external-crates/move/crates/move-analyzer/src/symbols/compilation.rs index 2434f915a7f7c..7ec5504a5638f 100644 --- a/external-crates/move/crates/move-analyzer/src/symbols/compilation.rs +++ b/external-crates/move/crates/move-analyzer/src/symbols/compilation.rs @@ -44,7 +44,7 @@ use move_compiler::{ }; use move_ir_types::location::Loc; -use move_package_alt::{flavor::MoveFlavor, package::RootPackage, schema::PackageName}; +use move_package_alt::{flavor::MoveFlavor, package::RootPackage}; use move_package_alt_compilation::{ build_config::BuildConfig, build_plan::BuildPlan, @@ -339,13 +339,19 @@ pub fn get_compiled_pkg( let root_pkg = load_root_pkg::(&build_config, pkg_path)?; let root_pkg_name = Symbol::from(root_pkg.name().to_string()); + // the package's transitive dependencies + let mut dependencies: Vec<_> = root_pkg + .packages() + .into_iter() + .filter(|x| !x.is_root()) + .collect(); let build_plan = BuildPlan::create(&root_pkg, &build_config)?.set_compiler_vfs_root(overlay_fs_root.clone()); - let mapped_files_data = - compute_mapped_files(&root_pkg, &build_config, overlay_fs_root.clone())?; // Hash dependencies so we can check if something has changed. // TODO: do we still need this? + let mapped_files_data = + compute_mapped_files(&root_pkg, &build_config, overlay_fs_root.clone())?; let file_paths: Arc> = Arc::new( mapped_files_data .files @@ -361,12 +367,9 @@ pub fn get_compiled_pkg( let mut compiler_analysis_info_opt = None; let mut compiler_autocomplete_info_opt = None; - // TODO: we need to rework on loading the root pkg only once. - let dependencies = root_pkg.packages(); - let compiler_flags = compiler_flags(&build_config); let (mut caching_result, other_diags) = if let Ok(deps_package_paths) = - make_deps_for_compiler(&mut Vec::new(), dependencies, &build_config) + make_deps_for_compiler(&mut Vec::new(), dependencies.clone(), &build_config) { let src_deps: BTreeMap = deps_package_paths .into_iter() @@ -414,8 +417,13 @@ pub fn get_compiled_pkg( let caching_result = match cached_pkg_info_opt { Some(cached_pkg_info) => { - // TODO: do we need to do anything here? - // dependencies.remove_deps(cached_pkg_info.dep_names.clone()); + // remove dependencies that are already included in the cached package info to + // avoid recompiling them + dependencies.retain(|d| { + !cached_pkg_info + .dep_names + .contains(&Symbol::from(d.id().to_string())) + }); let deps = cached_pkg_info.deps.clone(); let analyzed_pkg_info = AnalyzedPkgInfo::new( @@ -435,8 +443,15 @@ pub fn get_compiled_pkg( ) } None => { - let sorted_deps = root_pkg.sorted_deps(); - let sorted_deps: Vec = sorted_deps.into_iter().cloned().collect(); + // get the topologically sorted dependencies, but use the package ids instead of + // package names. In the new pkg system, multiple packages with the same name can + // exist as the package system will assign unique package ids to them, before + // passing them to the compiler. + let sorted_deps: Vec = root_pkg + .sorted_deps_ids() + .into_iter() + .map(|x| Symbol::from(x.to_string())) + .collect(); if let Some((program_deps, dep_names)) = compute_pre_compiled_dep_data( &mut cached_packages.compiled_dep_pkgs, mapped_files_data.dep_pkg_paths, @@ -508,8 +523,8 @@ pub fn get_compiled_pkg( // (that no longer have failures/warnings) are reset let mut ide_diags = lsp_empty_diagnostics(mapped_files_data.files.file_name_mapping()); if full_compilation || !files_to_compile.is_empty() { - build_plan.compile_with_driver( - // dependencies, + build_plan.compile_with_driver_and_deps( + dependencies.into_iter().map(|x| x.id()).cloned().collect(), &mut std::io::sink(), |compiler| { let compiler = compiler.set_ide_mode(); @@ -548,7 +563,6 @@ pub fn get_compiled_pkg( let failure = true; diagnostics = Some((diags, failure)); eprintln!("typed AST compilation failed"); - eprintln!("diagnostics: {:#?}", diagnostics); return Ok((files, vec![])); } }; @@ -574,7 +588,6 @@ pub fn get_compiled_pkg( }); caching_result.edition = Some(compiler.compilation_env().edition(Some(root_pkg_name))); - // compile to CFGIR for accurate diags eprintln!("compiling to CFGIR"); let compilation_result = compiler.at_typing(typed_program).run::(); @@ -696,23 +709,24 @@ fn compute_pre_compiled_dep_data( mut dep_paths: BTreeMap, mut src_deps: BTreeMap, root_package_name: Symbol, - topological_order: &[PackageName], + topological_order: &[Symbol], compiler_flags: Flags, vfs_root: VfsPath, ) -> Option<(Arc, BTreeSet)> { let mut pre_compiled_modules = BTreeMap::new(); let mut pre_compiled_names = BTreeSet::new(); for pkg_name in topological_order.iter().rev() { - let pkg_name = Symbol::from(pkg_name.to_string()); - if pkg_name == root_package_name { + // both pkg_name and root_package_name are actually PackageIDs and generated by the pkg + // system + if pkg_name == &root_package_name { continue; } - let Some(dep_path) = dep_paths.remove(&pkg_name) else { + let Some(dep_path) = dep_paths.remove(pkg_name) else { eprintln!("no dep path for {pkg_name}, no caching"); // do non-cached path return None; }; - let Some(dep_info) = src_deps.remove(&pkg_name) else { + let Some(dep_info) = src_deps.remove(pkg_name) else { eprintln!("no dep info for {pkg_name}, no caching"); // do non-cached path return None; @@ -827,10 +841,7 @@ fn compute_mapped_files( if is_dep { hasher.update(fhash.0); dep_hashes.push(fhash); - dep_pkg_paths.insert( - rpkg.name().as_str().into(), - rpkg.path().path().to_path_buf(), - ); + dep_pkg_paths.insert(rpkg.id().clone().into(), rpkg.path().path().to_path_buf()); } // write to top layer of the overlay file system so that the content // is immutable for the duration of compilation and symbolication diff --git a/external-crates/move/crates/move-package-alt-compilation/src/build_plan.rs b/external-crates/move/crates/move-package-alt-compilation/src/build_plan.rs index a6797e9f3d167..f9f49b4acea62 100644 --- a/external-crates/move/crates/move-package-alt-compilation/src/build_plan.rs +++ b/external-crates/move/crates/move-package-alt-compilation/src/build_plan.rs @@ -27,7 +27,7 @@ use move_package_alt::{ errors::PackageResult, flavor::MoveFlavor, package::{RootPackage, layout::SourcePackageLayout}, - schema::PackageName, + schema::PackageID, }; use move_symbol_pool::Symbol; use toml_edit::{DocumentMut, value}; @@ -36,14 +36,18 @@ const EDITION_NAME: &str = "edition"; pub struct BuildPlan<'a, F: MoveFlavor> { root_pkg: &'a RootPackage, + sorted_deps_ids: Vec<&'a PackageID>, compiler_vfs_root: Option, build_config: BuildConfig, } impl<'a, F: MoveFlavor> BuildPlan<'a, F> { pub fn create(root_pkg: &'a RootPackage, build_config: &BuildConfig) -> PackageResult { + let mut sorted_deps_ids = root_pkg.sorted_deps_ids(); + sorted_deps_ids.reverse(); Ok(Self { root_pkg, + sorted_deps_ids, build_config: build_config.clone(), compiler_vfs_root: None, }) @@ -73,12 +77,52 @@ impl<'a, F: MoveFlavor> BuildPlan<'a, F> { Compiler, ) -> anyhow::Result<(MappedFiles, Vec)>, + ) -> anyhow::Result { + let program_info_hook = SaveHook::new([SaveFlag::TypingInfo]); + let dependencies: BTreeSet = self + .root_pkg + .packages() + .into_iter() + .filter(|x| !x.is_root()) + .map(|x| x.id().to_string()) + .collect(); + let compiled = build_all::( + writer, + self.compiler_vfs_root.clone(), + self.root_pkg, + dependencies, + &self.build_config, + |compiler| { + let compiler = compiler.add_save_hook(&program_info_hook); + compiler_driver(compiler) + }, + )?; + + let project_root = self.root_pkg.package_path(); + + self.clean( + &project_root.join(CompiledPackageLayout::Root.path()), + self.sorted_deps_ids.clone(), + )?; + + Ok(compiled) + } + + pub fn compile_with_driver_and_deps( + &self, + dependencies: BTreeSet, + writer: &mut W, + compiler_driver: impl FnOnce( + Compiler, + ) + -> anyhow::Result<(MappedFiles, Vec)>, ) -> anyhow::Result { let program_info_hook = SaveHook::new([SaveFlag::TypingInfo]); let compiled = build_all::( writer, self.compiler_vfs_root.clone(), self.root_pkg, + dependencies, &self.build_config, |compiler| { let compiler = compiler.add_save_hook(&program_info_hook); @@ -87,11 +131,10 @@ impl<'a, F: MoveFlavor> BuildPlan<'a, F> { )?; let project_root = self.root_pkg.package_path(); - let sorted_deps = self.root_pkg.sorted_deps().into_iter().cloned().collect(); self.clean( &project_root.join(CompiledPackageLayout::Root.path()), - sorted_deps, + self.sorted_deps_ids.clone(), )?; Ok(compiled) @@ -134,7 +177,7 @@ impl<'a, F: MoveFlavor> BuildPlan<'a, F> { // Clean out old packages that are no longer used, or no longer used under the current // compilation flags - fn clean(&self, project_root: &Path, keep_paths: BTreeSet) -> anyhow::Result<()> { + fn clean(&self, project_root: &Path, keep_paths: Vec<&PackageID>) -> anyhow::Result<()> { // Compute the actual build directory based on install_dir configuration let build_root = shared::get_build_output_path(project_root, &self.build_config); @@ -159,11 +202,18 @@ impl<'a, F: MoveFlavor> BuildPlan<'a, F> { /// Migrate the package from legacy to Move 2024 edition, if possible. pub fn migrate(&self, writer: &mut W) -> anyhow::Result> { let root_name = Symbol::from(self.root_pkg.name().to_string()); + let dependencies: BTreeSet<_> = self + .root_pkg + .sorted_deps_ids() + .into_iter() + .cloned() + .collect(); let (files, res) = build_for_driver( writer, None, &self.build_config, self.root_pkg, + dependencies, |compiler| compiler.generate_migration_patch(&root_name), )?; let migration = match res { @@ -183,11 +233,10 @@ impl<'a, F: MoveFlavor> BuildPlan<'a, F> { }; let project_root = self.root_pkg.package_path(); - let sorted_deps = self.root_pkg.sorted_deps().into_iter().cloned().collect(); self.clean( &project_root.join(CompiledPackageLayout::Root.path()), - sorted_deps, + self.sorted_deps_ids.clone(), )?; Ok(migration) diff --git a/external-crates/move/crates/move-package-alt-compilation/src/compilation.rs b/external-crates/move/crates/move-package-alt-compilation/src/compilation.rs index 912834f6075aa..54d2794663fc0 100644 --- a/external-crates/move/crates/move-package-alt-compilation/src/compilation.rs +++ b/external-crates/move/crates/move-package-alt-compilation/src/compilation.rs @@ -32,7 +32,10 @@ use move_compiler::{ }; use move_docgen::DocgenFlags; use move_package_alt::{ - flavor::MoveFlavor, graph::PackageInfo, package::RootPackage, schema::Environment, + flavor::MoveFlavor, + graph::PackageInfo, + package::RootPackage, + schema::{Environment, PackageID}, }; use move_symbol_pool::Symbol; use std::{collections::BTreeMap, io::Write, path::PathBuf, str::FromStr}; @@ -76,17 +79,24 @@ pub fn build_all( w: &mut W, vfs_root: Option, root_pkg: &RootPackage, + dependencies: BTreeSet, build_config: &BuildConfig, compiler_driver: impl FnOnce(Compiler) -> Result<(MappedFiles, Vec)>, ) -> Result { let project_root = root_pkg.package_path().to_path_buf(); let program_info_hook = SaveHook::new([SaveFlag::TypingInfo]); let package_name = Symbol::from(root_pkg.name().as_str()); - let (file_map, all_compiled_units) = - build_for_driver(w, vfs_root, build_config, root_pkg, |compiler| { + let (file_map, all_compiled_units) = build_for_driver( + w, + vfs_root, + build_config, + root_pkg, + dependencies, + |compiler| { let compiler = compiler.add_save_hook(&program_info_hook); compiler_driver(compiler) - })?; + }, + )?; let mut all_compiled_units_vec = vec![]; let mut root_compiled_units = vec![]; @@ -195,9 +205,15 @@ pub fn build_for_driver( vfs_root: Option, build_config: &BuildConfig, root_pkg: &RootPackage, + dependencies: BTreeSet, compiler_driver: impl FnOnce(Compiler) -> Result, ) -> Result { + // TODO: pkg-alt this seems to add more packages for the compiler than necessary. let packages = root_pkg.packages(); + let packages = packages + .into_iter() + .filter(|p| p.is_root() || dependencies.contains(p.id())) + .collect::>(); let package_paths = make_deps_for_compiler(w, packages, build_config)?; debug!("Package paths {:#?}", package_paths); @@ -212,7 +228,6 @@ pub fn build_for_driver( let lint_level = build_config.lint_flag.get(); let sui_mode = build_config.default_flavor == Some(Flavor::Sui); let flags = compiler_flags(build_config); - let mut compiler = Compiler::from_package_paths(vfs_root, package_paths, vec![]) .unwrap() .set_flags(flags); @@ -372,7 +387,7 @@ pub fn make_deps_for_compiler( warning_filter: WarningFiltersBuilder::new_for_source(), }; - // TODO: improve/rework this? Renaming the root pkg to have a unique name for the compiler + // Assign a unique name for the compiler for each package. let safe_name = Symbol::from(pkg.id().clone()); debug!("Package name {:?} -- Safe name {:?}", name, safe_name); diff --git a/external-crates/move/crates/move-package-alt/src/compatibility/mod.rs b/external-crates/move/crates/move-package-alt/src/compatibility/mod.rs index aad9c6f3f14d0..75296cfd0aba0 100644 --- a/external-crates/move/crates/move-package-alt/src/compatibility/mod.rs +++ b/external-crates/move/crates/move-package-alt/src/compatibility/mod.rs @@ -8,6 +8,7 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result, bail}; use move_core_types::account_address::{AccountAddress, AccountAddressParseError}; +use once_cell::sync::Lazy; use regex::Regex; use tracing::debug; @@ -34,6 +35,9 @@ pub enum LegacySubstOrRename { /// The regex to detect `module ::` on its different forms. const MODULE_REGEX: &str = r"\bmodule\s+([a-zA-Z_][\w]*)::([a-zA-Z_][\w]*)"; +// Compile regex once at program startup +static MODULE_REGEX_COMPILED: Lazy = Lazy::new(|| Regex::new(MODULE_REGEX).unwrap()); + /// This is a naive way to detect all module names that are part of the source code /// for a given package. /// @@ -111,18 +115,15 @@ fn find_files(files: &mut Vec, dir: &Path, extension: &str, max_depth: // Consider supporting the legacy `address { module {} }` format. fn parse_module_names(contents: &str) -> Result> { let clean = strip_comments(contents); - let mut set = HashSet::new(); + // This matches `module a::b {}`, and `module a::b;` cases. // In both cases, the match is the 2nd group (so `match.get(1)`) - let regex = Regex::new(MODULE_REGEX).unwrap(); - - for cap in regex.captures_iter(&clean) { - set.insert(cap[1].to_string()); - } - - Ok(set - .into_iter() - .filter(|name| !is_address_like(name.as_str())) + Ok(MODULE_REGEX_COMPILED + .captures_iter(&clean) + .filter_map(|cap| { + let name = &cap[1]; + (!is_address_like(name)).then(|| name.to_string()) + }) .collect()) } diff --git a/external-crates/move/crates/move-package-alt/src/graph/mod.rs b/external-crates/move/crates/move-package-alt/src/graph/mod.rs index d2fd0ecb51e00..f56cce263e4ea 100644 --- a/external-crates/move/crates/move-package-alt/src/graph/mod.rs +++ b/external-crates/move/crates/move-package-alt/src/graph/mod.rs @@ -26,7 +26,7 @@ use crate::{ errors::PackageResult, flavor::MoveFlavor, package::{Package, paths::PackagePath}, - schema::{Environment, PackageID, PackageName}, + schema::{Environment, PackageID}, }; use bimap::BiBTreeMap; use builder::PackageGraphBuilder; @@ -113,14 +113,10 @@ impl PackageGraph { .collect() } - /// Return the sorted list of dependencies' name - pub(crate) fn sorted_deps(&self) -> Vec<&PackageName> { + /// Return the list of all packages that are in the package graph, sorted in topological order. + pub fn sorted_packages(&self) -> Vec> { let sorted = toposort(&self.inner, None).expect("to sort the graph"); - sorted - .iter() - .flat_map(|x| self.inner.node_weight(*x)) - .map(|x| x.name()) - .collect() + sorted.iter().map(|x| self.package_info(*x)).collect() } /// For each entry in `overrides`, override the package publication in `self` for the diff --git a/external-crates/move/crates/move-package-alt/src/package/root_package.rs b/external-crates/move/crates/move-package-alt/src/package/root_package.rs index 179f7a3ba9d10..8e33e8c43d558 100644 --- a/external-crates/move/crates/move-package-alt/src/package/root_package.rs +++ b/external-crates/move/crates/move-package-alt/src/package/root_package.rs @@ -14,8 +14,8 @@ use crate::graph::PackageInfo; use crate::package::block_on; use crate::package::package_lock::PackageSystemLock; use crate::schema::{ - Environment, LocalPub, LockfileDependencyInfo, ModeName, PackageID, PackageName, - ParsedEphemeralPubs, ParsedPublishedFile, Publication, RenderToml, + Environment, LocalPub, LockfileDependencyInfo, ModeName, PackageID, ParsedEphemeralPubs, + ParsedPublishedFile, Publication, RenderToml, }; use crate::{ errors::{PackageError, PackageResult}, @@ -483,11 +483,14 @@ impl RootPackage { self.filtered_graph.root_package().publication() } - // *** PATHS RELATED FUNCTIONS *** - - /// Return a list of sorted package names - pub fn sorted_deps(&self) -> Vec<&PackageName> { - self.filtered_graph.sorted_deps() + /// Sorts topologically the dependency graph and returns the package IDs for each package. Note + /// that this will include the root package as well. + pub fn sorted_deps_ids(&self) -> Vec<&PackageID> { + self.filtered_graph + .sorted_packages() + .into_iter() + .map(|p| p.id()) + .collect() } }