From af7b9714e0ed3ac788922e1d9c0c841cc726c0a4 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Tue, 6 Nov 2018 23:14:40 +0100 Subject: [PATCH 1/5] WIP: Fetch input files per rustc compilation --- src/actions/post_build.rs | 2 +- src/build/cargo.rs | 23 ++++++++++++--------- src/build/external.rs | 2 +- src/build/mod.rs | 6 +++--- src/build/rustc.rs | 43 +++++++++++++++++++++++++++++---------- 5 files changed, 50 insertions(+), 26 deletions(-) diff --git a/src/actions/post_build.rs b/src/actions/post_build.rs index a7131fb5ed2..68ae6fc63e4 100644 --- a/src/actions/post_build.rs +++ b/src/actions/post_build.rs @@ -55,7 +55,7 @@ pub struct PostBuildHandler { impl PostBuildHandler { pub fn handle(self, result: BuildResult) { match result { - BuildResult::Success(cwd, messages, new_analysis, _) => { + BuildResult::Success(cwd, messages, new_analysis, _, _) => { trace!("build - Success"); self.notifier.notify_begin_diagnostics(); diff --git a/src/build/cargo.rs b/src/build/cargo.rs index 32aef76a177..d8fa25b1c82 100644 --- a/src/build/cargo.rs +++ b/src/build/cargo.rs @@ -93,7 +93,7 @@ pub(super) fn cargo( .unwrap() .into_inner() .unwrap(); - BuildResult::Success(cwd.clone(), diagnostics, analysis, true) + BuildResult::Success(cwd.clone(), diagnostics, analysis, vec![], true) } Err(error) => { let stdout = String::from_utf8(out_clone.lock().unwrap().to_owned()).unwrap(); @@ -559,18 +559,21 @@ impl Executor for RlsExecutor { cx.build_dir.clone().unwrap() }; - if let BuildResult::Success(_, mut messages, mut analysis, success) = super::rustc::rustc( - &self.vfs, - &args, - &envs, - cargo_cmd.get_cwd(), - &build_dir, - Arc::clone(&self.config), - &self.env_lock.as_facade(), - ) { + if let BuildResult::Success(_, mut messages, mut analysis, input_files, success) = + super::rustc::rustc( + &self.vfs, + &args, + &envs, + cargo_cmd.get_cwd(), + &build_dir, + Arc::clone(&self.config), + &self.env_lock.as_facade(), + ) { self.compiler_messages.lock().unwrap().append(&mut messages); self.analysis.lock().unwrap().append(&mut analysis); + // TODO: Cache input files in the plan! + if !success { return Err(format_err!("Build error")); } diff --git a/src/build/external.rs b/src/build/external.rs index 37c1950adf8..8c0f8ca459b 100644 --- a/src/build/external.rs +++ b/src/build/external.rs @@ -96,7 +96,7 @@ pub(super) fn build_with_external_cmd>( }; let plan = plan_from_analysis(&analyses, &build_dir); - (BuildResult::Success(build_dir, vec![], analyses, false), plan) + (BuildResult::Success(build_dir, vec![], analyses, vec![], false), plan) } /// Reads and deserializes given save-analysis JSON files into corresponding diff --git a/src/build/mod.rs b/src/build/mod.rs index f220662f28a..bb7eb51202f 100644 --- a/src/build/mod.rs +++ b/src/build/mod.rs @@ -100,10 +100,10 @@ struct Internals { #[derive(Debug)] pub enum BuildResult { /// Build was performed without any internal errors. The payload - /// contains current directory at the time, emitted raw diagnostics, and - /// Analysis data. + /// contains current directory at the time, emitted raw diagnostics, + /// Analysis data and list of input files to the compilation. /// Final bool is true if and only if compiler's exit code would be 0. - Success(PathBuf, Vec, Vec, bool), + Success(PathBuf, Vec, Vec, Vec, bool), /// Build was coalesced with another build. Squashed, /// There was an error attempting to build. diff --git a/src/build/rustc.rs b/src/build/rustc.rs index 469a2e66fc8..87ccad10801 100644 --- a/src/build/rustc.rs +++ b/src/build/rustc.rs @@ -113,8 +113,9 @@ crate fn rustc( args.to_owned() }; - let analysis = Arc::new(Mutex::new(None)); - let controller = Box::new(RlsRustcCalls::new(Arc::clone(&analysis), clippy_pref)); + let analysis = Arc::default(); + let input_files = Arc::default(); + let controller = Box::new(RlsRustcCalls::new(Arc::clone(&analysis), Arc::clone(&input_files), clippy_pref)); // rustc explicitly panics in run_compiler() on compile failure, regardless // if it encounters an ICE (internal compiler error) or not. @@ -144,14 +145,13 @@ crate fn rustc( .unwrap_or_else(Vec::new); log::debug!("rustc: analysis read successfully?: {}", !analysis.is_empty()); + let input_files = Arc::try_unwrap(input_files).unwrap().into_inner().unwrap(); + let cwd = cwd .unwrap_or_else(|| restore_env.get_old_cwd()) .to_path_buf(); - match result { - Ok(_) => BuildResult::Success(cwd, stderr_json_msgs, analysis, true), - Err(_) => BuildResult::Success(cwd, stderr_json_msgs, analysis, false), - } + BuildResult::Success(cwd, stderr_json_msgs, analysis, input_files, result.is_ok()) } // Our compiler controller. We mostly delegate to the default rustc @@ -160,17 +160,20 @@ crate fn rustc( struct RlsRustcCalls { default_calls: Box, analysis: Arc>>, + input_files: Arc>>, clippy_preference: ClippyPreference, } impl RlsRustcCalls { fn new( analysis: Arc>>, + input_files: Arc>>, clippy_preference: ClippyPreference, ) -> RlsRustcCalls { RlsRustcCalls { default_calls: Box::new(RustcDefaultCalls), analysis, + input_files, clippy_preference, } } @@ -267,17 +270,24 @@ impl<'a> CompilerCalls<'a> for RlsRustcCalls { matches: &getopts::Matches, ) -> CompileController<'a> { let analysis = self.analysis.clone(); + let input_files = self.input_files.clone(); #[cfg(feature = "clippy")] let clippy_preference = self.clippy_preference; let mut result = self.default_calls.build_controller(sess, matches); result.keep_ast = true; - #[cfg(feature = "clippy")] - { - if clippy_preference != ClippyPreference::Off { - result.after_parse.callback = Box::new(clippy_after_parse_callback); + result.after_parse.callback = Box::new(move |state| { + { + let files = fetch_input_files(state.session); + *input_files.lock().unwrap() = files; } - } + #[cfg(feature = "clippy")] + { + if clippy_preference != ClippyPreference::Off { + result.after_parse.callback = Box::new(clippy_after_parse_callback); + } + } + }); result.after_analysis.callback = Box::new(move |state| { // There are two ways to move the data from rustc to the RLS, either @@ -317,6 +327,17 @@ impl<'a> CompilerCalls<'a> for RlsRustcCalls { } } +fn fetch_input_files(sess: &Session) -> Vec { + sess.source_map() + .files() + .iter() + .filter(|fmap| fmap.is_real_file()) + .filter(|fmap| !fmap.is_imported()) + .map(|fmap| fmap.name.to_string()) + .map(PathBuf::from) + .collect() +} + /// Tries to read a file from a list of replacements, and if the file is not /// there, then reads it from disk, by delegating to `RealFileLoader`. struct ReplacedFileLoader { From bdf0d9253009c2de263e22a15d794ff65a05d1b1 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Wed, 7 Nov 2018 13:46:52 +0100 Subject: [PATCH 2/5] Cache input files from rustc invocations in Cargo runs --- src/build/cargo.rs | 7 ++- src/build/cargo_plan.rs | 106 ++++++++++++++++++++-------------------- src/build/external.rs | 11 +---- src/build/plan.rs | 4 +- src/build/rustc.rs | 12 ++++- 5 files changed, 73 insertions(+), 67 deletions(-) diff --git a/src/build/cargo.rs b/src/build/cargo.rs index d8fa25b1c82..fa04950e11e 100644 --- a/src/build/cargo.rs +++ b/src/build/cargo.rs @@ -572,7 +572,12 @@ impl Executor for RlsExecutor { self.compiler_messages.lock().unwrap().append(&mut messages); self.analysis.lock().unwrap().append(&mut analysis); - // TODO: Cache input files in the plan! + // Cache calculated input files for a given rustc invocation + { + let mut cx = self.compilation_cx.lock().unwrap(); + let plan = cx.build_plan.as_cargo_mut().unwrap(); + plan.cache_input_files(id, target, mode, input_files, cargo_cmd.get_cwd()); + } if !success { return Err(format_err!("Build error")); diff --git a/src/build/cargo_plan.rs b/src/build/cargo_plan.rs index cf70ec62188..4fb5698da72 100644 --- a/src/build/cargo_plan.rs +++ b/src/build/cargo_plan.rs @@ -39,6 +39,7 @@ use url::Url; use crate::build::PackageArg; use crate::build::plan::{BuildKey, BuildGraph, JobQueue, WorkStatus}; +use crate::build::rustc::src_path; use crate::lsp_data::parse_file_path; /// Main key type by which `Unit`s will be distinguished in the build plan. @@ -58,6 +59,9 @@ crate struct CargoPlan { crate rev_dep_graph: HashMap>, /// Cached compiler calls used when creating a compiler call queue. crate compiler_jobs: HashMap, + /// Calculated input files that unit depend on. + crate input_files: HashMap>, + crate file_key_mapping: HashMap>, // An object for finding the package which a file belongs to and this inferring // a package argument. package_map: Option, @@ -101,6 +105,40 @@ impl CargoPlan { self.compiler_jobs.insert(unit_key, cmd.clone()); } + crate fn cache_input_files( + &mut self, + id: &PackageId, + target: &Target, + mode: CompileMode, + input_files: Vec, + cwd: Option<&Path>, + ) { + let input_files: Vec<_> = input_files + .iter() + .filter_map(|file| src_path(cwd, file)) + .filter_map(|file| match std::fs::canonicalize(&file) { + Ok(file) => Some(file), + Err(err) => { + error!("Couldn't canonicalize `{}`: {}", file.display(), err); + None + } + }) + .collect(); + + let unit_key = (id.clone(), target.clone(), mode); + trace!("Caching these files: {:#?} for {:?} key", &input_files, &unit_key); + + // Create reverse file -> unit mapping (used for dirty unit calculation) + for file in &input_files { + self.file_key_mapping + .entry(file.to_path_buf()) + .or_default() + .insert(unit_key.clone()); + } + + self.input_files.insert(unit_key, input_files); + } + /// Emplace a given `Unit`, along with its `Unit` dependencies (recursively) /// into the dependency graph as long as the passed `Unit` isn't filtered /// out by the `filter` closure. @@ -159,6 +197,7 @@ impl CargoPlan { } } + /// TODO: Update TODO below /// TODO: Improve detecting dirty crate targets for a set of dirty file paths. /// This uses a lousy heuristic of checking path prefix for a given crate /// target to determine whether a given unit (crate target) is dirty. This @@ -168,7 +207,7 @@ impl CargoPlan { /// Because of that, build scripts are checked separately and only other /// crate targets are checked with path prefixes. fn fetch_dirty_units>(&self, files: &[T]) -> HashSet { - let mut result = HashSet::new(); + let files = files.iter().map(|f| f.as_ref()); let build_scripts: HashMap<&Path, UnitKey> = self .units @@ -178,59 +217,18 @@ impl CargoPlan { }) .map(|(key, unit)| (unit.target.src_path().path(), key.clone())) .collect(); - let other_targets: HashMap = self - .units - .iter() - .filter(|&(&(_, ref target, _), _)| *target.kind() != TargetKind::CustomBuild) - .map(|(key, unit)| { - ( - key.clone(), - unit.target - .src_path() - .path() - .parent() - .expect("no parent for src_path"), - ) - }).collect(); - - for modified in files.iter().map(|x| x.as_ref()) { - if let Some(unit) = build_scripts.get(modified) { - result.insert(unit.clone()); - } else { - // Not a build script, so we associate a dirty file with a - // package by finding longest (most specified) path prefix. - let matching_prefix_components = |a: &Path, b: &Path| -> usize { - assert!(a.is_absolute() && b.is_absolute()); - a.components().zip(b.components()) - .skip(1) // Skip RootDir - .take_while(|&(x, y)| x == y) - .count() - }; - // Since a package can correspond to many units (e.g. compiled - // as a regular binary or a test harness for unit tests), we - // collect every unit having the longest path prefix. - let max_matching_prefix = other_targets - .values() - .map(|src_dir| matching_prefix_components(modified, src_dir)) - .max(); - - match max_matching_prefix { - Some(0) => error!( - "Modified file {} didn't correspond to any buildable unit!", - modified.display() - ), - Some(max) => { - let dirty_units = other_targets - .iter() - .filter(|(_, dir)| max == matching_prefix_components(modified, dir)) - .map(|(unit, _)| unit); - - result.extend(dirty_units.cloned()); - } - None => {} // Possible that only build scripts were modified - } - } - } + + let mut result = HashSet::new(); + + let dirty_rustc_units = files + .clone() + .filter_map(|f| self.file_key_mapping.get(f)) + .flat_map(|units| units); + let dirty_build_scripts = files.filter_map(|f| build_scripts.get(f)); + + result.extend(dirty_rustc_units.cloned()); + result.extend(dirty_build_scripts.cloned()); + result } diff --git a/src/build/external.rs b/src/build/external.rs index 8c0f8ca459b..1c0d7285817 100644 --- a/src/build/external.rs +++ b/src/build/external.rs @@ -31,6 +31,7 @@ use std::process::{Command, Stdio}; use crate::build::BuildResult; use crate::build::plan::{BuildKey, BuildGraph, JobQueue, WorkStatus}; +use crate::build::rustc::src_path; use cargo::util::{process, ProcessBuilder}; use log::trace; @@ -453,16 +454,6 @@ fn guess_rustc_src_path(build_dir: &Path, cmd: &ProcessBuilder) -> Option, path: impl AsRef) -> Option { - let path = path.as_ref(); - - Some(match (cwd, path.is_absolute()) { - (_, true) => path.to_owned(), - (Some(cwd), _) => cwd.join(path), - (None, _) => std::env::current_dir().ok()?.join(path) - }) -} - #[cfg(test)] mod tests { use super::*; diff --git a/src/build/plan.rs b/src/build/plan.rs index f6aa9ce298c..f120d80227b 100644 --- a/src/build/plan.rs +++ b/src/build/plan.rs @@ -188,7 +188,7 @@ impl JobQueue { Arc::clone(&internals.config), &internals.env_lock.as_facade(), ) { - BuildResult::Success(c, mut messages, mut analysis, success) => { + BuildResult::Success(c, mut messages, mut analysis, _, success) => { compiler_messages.append(&mut messages); analyses.append(&mut analysis); cwd = Some(c); @@ -200,6 +200,7 @@ impl JobQueue { cwd.unwrap(), compiler_messages, analyses, + vec![], false, ); } @@ -216,6 +217,7 @@ impl JobQueue { cwd.unwrap_or_else(|| PathBuf::from(".")), compiler_messages, analyses, + vec![], true, ) } diff --git a/src/build/rustc.rs b/src/build/rustc.rs index 87ccad10801..2ab4b70df1a 100644 --- a/src/build/rustc.rs +++ b/src/build/rustc.rs @@ -284,7 +284,7 @@ impl<'a> CompilerCalls<'a> for RlsRustcCalls { #[cfg(feature = "clippy")] { if clippy_preference != ClippyPreference::Off { - result.after_parse.callback = Box::new(clippy_after_parse_callback); + clippy_after_parse_callback(state); } } }); @@ -391,3 +391,13 @@ pub(super) fn current_sysroot() -> Option { }) } } + +pub fn src_path(cwd: Option<&Path>, path: impl AsRef) -> Option { + let path = path.as_ref(); + + Some(match (cwd, path.is_absolute()) { + (_, true) => path.to_owned(), + (Some(cwd), _) => cwd.join(path), + (None, _) => std::env::current_dir().ok()?.join(path) + }) +} From d865784a0fcfd877a2007083f8699a9340a3fc32 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Wed, 7 Nov 2018 16:05:28 +0100 Subject: [PATCH 3/5] Return file -> crate mapping in build result This allows it to bubble up to InitContextAction. This is required, because action handler ideally shouldn't have access to build internals and this is something that is updated in a lock-step between builds, so it seems like a good place to go. --- src/actions/mod.rs | 21 ++++++++++++++++++- src/actions/post_build.rs | 14 ++++++++++--- src/actions/requests.rs | 14 +++++++++++++ src/build/cargo.rs | 24 +++++++++++++++++++-- src/build/external.rs | 2 +- src/build/mod.rs | 4 +++- src/build/plan.rs | 38 ++++++++++++++++++++++++++++++--- src/build/rustc.rs | 44 ++++++++++++++++++++++++++++----------- 8 files changed, 138 insertions(+), 23 deletions(-) diff --git a/src/actions/mod.rs b/src/actions/mod.rs index ae35c475e10..261848d7742 100644 --- a/src/actions/mod.rs +++ b/src/actions/mod.rs @@ -32,7 +32,7 @@ use crate::lsp_data::*; use crate::project_model::{ProjectModel, RacerFallbackModel, RacerProjectModel}; use crate::server::Output; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::io; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; @@ -143,6 +143,7 @@ pub struct InitActionContext { previous_build_results: Arc>, build_queue: BuildQueue, + file_to_crates: Arc>>>, // Keep a record of builds/post-build tasks currently in flight so that // mutating actions can block until the data is ready. active_build_count: Arc, @@ -212,6 +213,7 @@ impl InitActionContext { project_model: Arc::default(), previous_build_results: Arc::default(), build_queue, + file_to_crates: Arc::default(), active_build_count: Arc::new(AtomicUsize::new(0)), shown_cargo_error: Arc::new(AtomicBool::new(false)), quiescent: Arc::new(AtomicBool::new(false)), @@ -288,6 +290,22 @@ impl InitActionContext { FmtConfig::from(&self.current_project) } + fn file_edition(&self, file: PathBuf) -> Option { + let files_to_crates = self.file_to_crates.lock().unwrap(); + + let editions: HashSet<_> = files_to_crates + .get(&file)? + .iter() + .map(|c| c.edition) + .collect(); + + let mut iter = editions.into_iter(); + match (iter.next(), iter.next()) { + (ret @ Some(_), None) => ret, + _ => None + } + } + fn init(&self, init_options: &InitializationOptions, out: &O) { let current_project = self.current_project.clone(); let config = self.config.clone(); @@ -318,6 +336,7 @@ impl InitActionContext { analysis: self.analysis.clone(), analysis_queue: self.analysis_queue.clone(), previous_build_results: self.previous_build_results.clone(), + file_to_crates: self.file_to_crates.clone(), project_path: project_path.to_owned(), show_warnings: config.show_warnings, related_information_support: self.client_capabilities.related_information_support, diff --git a/src/actions/post_build.rs b/src/actions/post_build.rs index 68ae6fc63e4..093df90845a 100644 --- a/src/actions/post_build.rs +++ b/src/actions/post_build.rs @@ -13,17 +13,18 @@ #![allow(missing_docs)] use std::collections::hash_map::DefaultHasher; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::hash::{Hash, Hasher}; use std::panic::RefUnwindSafe; use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; use std::thread::{self, Thread}; +use std::ops::Deref; use crate::actions::diagnostics::{parse_diagnostics, Diagnostic, ParsedDiagnostics, Suggestion}; use crate::actions::progress::DiagnosticsNotifier; -use crate::build::BuildResult; +use crate::build::{BuildResult, Crate}; use crate::concurrency::JobToken; use crate::lsp_data::{Range, PublishDiagnosticsParams}; @@ -41,6 +42,7 @@ pub struct PostBuildHandler { pub analysis: Arc, pub analysis_queue: Arc, pub previous_build_results: Arc>, + pub file_to_crates: Arc>>>, pub project_path: PathBuf, pub show_warnings: bool, pub use_black_list: bool, @@ -55,7 +57,7 @@ pub struct PostBuildHandler { impl PostBuildHandler { pub fn handle(self, result: BuildResult) { match result { - BuildResult::Success(cwd, messages, new_analysis, _, _) => { + BuildResult::Success(cwd, messages, new_analysis, input_files, _) => { trace!("build - Success"); self.notifier.notify_begin_diagnostics(); @@ -63,6 +65,12 @@ impl PostBuildHandler { self.handle_messages(&cwd, &messages); let analysis_queue = self.analysis_queue.clone(); + { + let mut files_to_crates = self.file_to_crates.lock().unwrap(); + *files_to_crates = input_files; + trace!("Files to crates: {:#?}", files_to_crates.deref()); + } + let job = Job::new(self, new_analysis, cwd); analysis_queue.enqueue(job); } diff --git a/src/actions/requests.rs b/src/actions/requests.rs index c2fe90513c3..5bef87cc8f3 100644 --- a/src/actions/requests.rs +++ b/src/actions/requests.rs @@ -754,6 +754,20 @@ fn reformat( if !config.was_set().tab_spaces() { config.set().tab_spaces(opts.tab_size as usize); } + if !config.was_set().edition() { + match ctx.file_edition(path.clone()) { + Some(edition) => { + // TODO: Set me once the option in rustfmt is exposed + }, + None => { + debug!("Reformat failed: ambiguous edition for `{}`", path.display()); + return Err(ResponseError::Message( + ErrorCode::InternalError, + "Reformat failed to complete successfully".into(), + )); + } + } + } if let Some(r) = selection { let range_of_rls = ls_util::range_to_rls(r).one_indexed(); diff --git a/src/build/cargo.rs b/src/build/cargo.rs index fa04950e11e..bfef174712e 100644 --- a/src/build/cargo.rs +++ b/src/build/cargo.rs @@ -24,7 +24,7 @@ use serde_json; use crate::actions::progress::ProgressUpdate; use crate::build::cargo_plan::CargoPlan; use crate::build::environment::{self, Environment, EnvironmentLock}; -use crate::build::plan::BuildPlan; +use crate::build::plan::{BuildPlan, Crate}; use crate::build::{BufWriter, BuildResult, CompilationContext, Internals, PackageArg}; use crate::config::Config; use crate::lsp_data::{Position, Range}; @@ -58,6 +58,8 @@ pub(super) fn cargo( let diagnostics_clone = diagnostics.clone(); let analysis = Arc::new(Mutex::new(vec![])); let analysis_clone = analysis.clone(); + let input_files = Arc::new(Mutex::new(HashMap::new())); + let input_files_clone = input_files.clone(); let out = Arc::new(Mutex::new(vec![])); let out_clone = out.clone(); @@ -74,6 +76,7 @@ pub(super) fn cargo( env_lock, diagnostics, analysis, + input_files, out, progress_sender, ) @@ -93,7 +96,11 @@ pub(super) fn cargo( .unwrap() .into_inner() .unwrap(); - BuildResult::Success(cwd.clone(), diagnostics, analysis, vec![], true) + let input_files = Arc::try_unwrap(input_files_clone) + .unwrap() + .into_inner() + .unwrap(); + BuildResult::Success(cwd.clone(), diagnostics, analysis, input_files, true) } Err(error) => { let stdout = String::from_utf8(out_clone.lock().unwrap().to_owned()).unwrap(); @@ -123,6 +130,7 @@ fn run_cargo( env_lock: Arc, compiler_messages: Arc>>, analysis: Arc>>, + input_files: Arc>>>, out: Arc>>, progress_sender: Sender, ) -> Result { @@ -163,6 +171,7 @@ fn run_cargo( vfs, compiler_messages, analysis, + input_files, progress_sender, inner_lock, restore_env, @@ -180,6 +189,7 @@ fn run_cargo_ws( vfs: Arc, compiler_messages: Arc>>, analysis: Arc>>, + input_files: Arc>>>, progress_sender: Sender, inner_lock: environment::InnerLock, mut restore_env: Environment<'_>, @@ -275,6 +285,7 @@ fn run_cargo_ws( vfs, compiler_messages, analysis, + input_files, progress_sender, reached_primary.clone(), ); @@ -321,6 +332,7 @@ struct RlsExecutor { /// Packages which are directly a member of the workspace, for which /// analysis and diagnostics will be provided member_packages: Mutex>, + input_files: Arc>>>, /// JSON compiler messages emitted for each primary compiled crate compiler_messages: Arc>>, progress_sender: Mutex>, @@ -341,6 +353,7 @@ impl RlsExecutor { vfs: Arc, compiler_messages: Arc>>, analysis: Arc>>, + input_files: Arc>>>, progress_sender: Sender, reached_primary: Arc, ) -> RlsExecutor { @@ -352,6 +365,7 @@ impl RlsExecutor { env_lock, vfs, analysis, + input_files, member_packages: Mutex::new(member_packages), compiler_messages, progress_sender: Mutex::new(progress_sender), @@ -576,9 +590,15 @@ impl Executor for RlsExecutor { { let mut cx = self.compilation_cx.lock().unwrap(); let plan = cx.build_plan.as_cargo_mut().unwrap(); + let input_files = input_files.keys().cloned().collect(); plan.cache_input_files(id, target, mode, input_files, cargo_cmd.get_cwd()); } + let mut self_input_files = self.input_files.lock().unwrap(); + for (file, inputs) in input_files { + self_input_files.entry(file).or_default().extend(inputs); + } + if !success { return Err(format_err!("Build error")); } diff --git a/src/build/external.rs b/src/build/external.rs index 1c0d7285817..49f25eaee79 100644 --- a/src/build/external.rs +++ b/src/build/external.rs @@ -97,7 +97,7 @@ pub(super) fn build_with_external_cmd>( }; let plan = plan_from_analysis(&analyses, &build_dir); - (BuildResult::Success(build_dir, vec![], analyses, vec![], false), plan) + (BuildResult::Success(build_dir, vec![], analyses, HashMap::default(), false), plan) } /// Reads and deserializes given save-analysis JSON files into corresponding diff --git a/src/build/mod.rs b/src/build/mod.rs index bb7eb51202f..57f0b496e9b 100644 --- a/src/build/mod.rs +++ b/src/build/mod.rs @@ -32,6 +32,8 @@ use std::sync::{Arc, Mutex, RwLock}; use std::thread; use std::time::{Duration, Instant}; +pub use self::plan::{Crate, CrateKind, Edition}; + mod cargo; mod cargo_plan; pub mod environment; @@ -103,7 +105,7 @@ pub enum BuildResult { /// contains current directory at the time, emitted raw diagnostics, /// Analysis data and list of input files to the compilation. /// Final bool is true if and only if compiler's exit code would be 0. - Success(PathBuf, Vec, Vec, Vec, bool), + Success(PathBuf, Vec, Vec, HashMap>, bool), /// Build was coalesced with another build. Squashed, /// There was an error attempting to build. diff --git a/src/build/plan.rs b/src/build/plan.rs index f120d80227b..18bdc04fda3 100644 --- a/src/build/plan.rs +++ b/src/build/plan.rs @@ -5,6 +5,7 @@ //! * Cargo - used when we run Cargo in-process and intercept it //! * External - dependency graph between invocations +use std::collections::{HashMap, HashSet}; use std::hash::Hash; use std::path::{Path, PathBuf}; use std::ffi::OsStr; @@ -113,6 +114,7 @@ impl JobQueue { let mut compiler_messages = vec![]; let mut analyses = vec![]; + let mut input_files = HashMap::<_, HashSet<_>>::new(); let (build_dir, mut cwd) = { let comp_cx = internals.compilation_cx.lock().unwrap(); ( @@ -188,9 +190,13 @@ impl JobQueue { Arc::clone(&internals.config), &internals.env_lock.as_facade(), ) { - BuildResult::Success(c, mut messages, mut analysis, _, success) => { + BuildResult::Success(c, mut messages, mut analysis, files, success) => { compiler_messages.append(&mut messages); analyses.append(&mut analysis); + for (file, inputs) in files { + input_files.entry(file).or_default().extend(inputs); + } + cwd = Some(c); // This compilation failed, but the build as a whole does not @@ -200,7 +206,7 @@ impl JobQueue { cwd.unwrap(), compiler_messages, analyses, - vec![], + input_files, false, ); } @@ -217,8 +223,34 @@ impl JobQueue { cwd.unwrap_or_else(|| PathBuf::from(".")), compiler_messages, analyses, - vec![], + input_files, true, ) } } + +/// Build system-agnostic, basic compilation unit +#[derive(PartialEq, Eq, Hash, Debug, Clone)] +pub struct Crate { + pub name: String, + pub kind: CrateKind, + pub src_path: Option, + pub edition: Edition, + /// From rustc; mainly used to group other properties used to disambiguate a + /// given compilation unit. + pub disambiguator: (u64, u64), +} + +#[derive(PartialEq, Eq, Hash, Debug, PartialOrd, Ord, Copy, Clone)] +pub enum CrateKind { + Binary, + Library, + // ProcMacro? +} + +// Temporary, until Edition from rustfmt is available +#[derive(PartialEq, Eq, Hash, Debug, PartialOrd, Ord, Copy, Clone)] +pub enum Edition { + Edition2015, + Edition2018 +} \ No newline at end of file diff --git a/src/build/rustc.rs b/src/build/rustc.rs index 2ab4b70df1a..d986c868804 100644 --- a/src/build/rustc.rs +++ b/src/build/rustc.rs @@ -46,9 +46,10 @@ use self::syntax::source_map::{FileLoader, RealFileLoader}; use crate::build::environment::{Environment, EnvironmentLockFacade}; use crate::build::{BufWriter, BuildResult}; +use crate::build::plan::{Crate, CrateKind, Edition}; use crate::config::{ClippyPreference, Config}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::env; use std::ffi::OsString; use std::io; @@ -160,14 +161,14 @@ crate fn rustc( struct RlsRustcCalls { default_calls: Box, analysis: Arc>>, - input_files: Arc>>, + input_files: Arc>>>, clippy_preference: ClippyPreference, } impl RlsRustcCalls { fn new( analysis: Arc>>, - input_files: Arc>>, + input_files: Arc>>>, clippy_preference: ClippyPreference, ) -> RlsRustcCalls { RlsRustcCalls { @@ -276,16 +277,35 @@ impl<'a> CompilerCalls<'a> for RlsRustcCalls { let mut result = self.default_calls.build_controller(sess, matches); result.keep_ast = true; - result.after_parse.callback = Box::new(move |state| { - { - let files = fetch_input_files(state.session); - *input_files.lock().unwrap() = files; + #[cfg(feature = "clippy")] + { + if clippy_preference != ClippyPreference::Off { + result.after_parse.callback = Box::new(clippy_after_parse_callback); } - #[cfg(feature = "clippy")] - { - if clippy_preference != ClippyPreference::Off { - clippy_after_parse_callback(state); - } + } + + result.after_expand.callback = Box::new(move |state| { + let krate = Crate { + name: state.crate_name.expect("missing crate name").to_owned(), + src_path: match state.input { + Input::File(ref name) => Some(name.to_path_buf()), + Input::Str { .. } => None, + }, + kind: CrateKind::Library, // FIXME + disambiguator: state.session.local_crate_disambiguator().to_fingerprint().as_value(), + edition: match state.session.edition() { + syntax::edition::Edition::Edition2018 => Edition::Edition2018, + _ => Edition::Edition2015, + }, + }; + let files = fetch_input_files(state.session); + + let mut input_files = input_files.lock().unwrap(); + for file in &files { + input_files + .entry(file.to_path_buf()) + .or_default() + .insert(krate.clone()); } }); From bef52e3120483e343f644eb6b3349380056c295e Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Wed, 7 Nov 2018 16:17:53 +0100 Subject: [PATCH 4/5] Set rustfmt edition for collected files --- src/actions/mod.rs | 2 +- src/actions/requests.rs | 15 +++++++++++---- src/build/mod.rs | 2 +- src/build/plan.rs | 16 +++++++--------- src/build/rustc.rs | 25 +++++++++++++++++-------- 5 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/actions/mod.rs b/src/actions/mod.rs index 261848d7742..0e7e750412e 100644 --- a/src/actions/mod.rs +++ b/src/actions/mod.rs @@ -302,7 +302,7 @@ impl InitActionContext { let mut iter = editions.into_iter(); match (iter.next(), iter.next()) { (ret @ Some(_), None) => ret, - _ => None + _ => None, } } diff --git a/src/actions/requests.rs b/src/actions/requests.rs index 5bef87cc8f3..2187728abeb 100644 --- a/src/actions/requests.rs +++ b/src/actions/requests.rs @@ -12,12 +12,12 @@ use crate::actions::InitActionContext; use itertools::Itertools; -use log::{debug, trace}; +use log::{debug, trace, warn}; use racer; use rls_data as data; use rls_span as span; use rls_vfs::FileContents; -use rustfmt_nightly::{FileLines, FileName, Range as RustfmtRange}; +use rustfmt_nightly::{Edition as RustfmtEdition, FileLines, FileName, Range as RustfmtRange}; use serde_derive::{Deserialize, Serialize}; use serde_json; use url::Url; @@ -26,6 +26,7 @@ use crate::actions::hover; use crate::actions::run::collect_run_actions; use crate::actions::work_pool; use crate::actions::work_pool::WorkDescription; +use crate::build::Edition; use crate::lsp_data; use crate::lsp_data::*; use crate::server; @@ -757,10 +758,16 @@ fn reformat( if !config.was_set().edition() { match ctx.file_edition(path.clone()) { Some(edition) => { - // TODO: Set me once the option in rustfmt is exposed + let edition = match edition { + Edition::Edition2015 => RustfmtEdition::Edition2015, + Edition::Edition2018 => RustfmtEdition::Edition2018, + }; + config.set().edition(edition); + trace!("Detected edition {:?} for file `{}`", edition, path.display()); }, None => { - debug!("Reformat failed: ambiguous edition for `{}`", path.display()); + warn!("Reformat failed: ambiguous edition for `{}`", path.display()); + return Err(ResponseError::Message( ErrorCode::InternalError, "Reformat failed to complete successfully".into(), diff --git a/src/build/mod.rs b/src/build/mod.rs index 57f0b496e9b..416d02e5359 100644 --- a/src/build/mod.rs +++ b/src/build/mod.rs @@ -32,7 +32,7 @@ use std::sync::{Arc, Mutex, RwLock}; use std::thread; use std::time::{Duration, Instant}; -pub use self::plan::{Crate, CrateKind, Edition}; +pub use self::plan::{Crate, Edition}; mod cargo; mod cargo_plan; diff --git a/src/build/plan.rs b/src/build/plan.rs index 18bdc04fda3..77c15270574 100644 --- a/src/build/plan.rs +++ b/src/build/plan.rs @@ -233,7 +233,6 @@ impl JobQueue { #[derive(PartialEq, Eq, Hash, Debug, Clone)] pub struct Crate { pub name: String, - pub kind: CrateKind, pub src_path: Option, pub edition: Edition, /// From rustc; mainly used to group other properties used to disambiguate a @@ -241,16 +240,15 @@ pub struct Crate { pub disambiguator: (u64, u64), } -#[derive(PartialEq, Eq, Hash, Debug, PartialOrd, Ord, Copy, Clone)] -pub enum CrateKind { - Binary, - Library, - // ProcMacro? -} - // Temporary, until Edition from rustfmt is available #[derive(PartialEq, Eq, Hash, Debug, PartialOrd, Ord, Copy, Clone)] pub enum Edition { Edition2015, Edition2018 -} \ No newline at end of file +} + +impl Default for Edition { + fn default() -> Edition { + Edition::Edition2015 + } +} diff --git a/src/build/rustc.rs b/src/build/rustc.rs index d986c868804..f64a8389015 100644 --- a/src/build/rustc.rs +++ b/src/build/rustc.rs @@ -43,10 +43,11 @@ use self::rustc_save_analysis as save; use self::rustc_save_analysis::CallbackHandler; use self::syntax::ast; use self::syntax::source_map::{FileLoader, RealFileLoader}; +use self::syntax::edition::Edition as RustcEdition; use crate::build::environment::{Environment, EnvironmentLockFacade}; use crate::build::{BufWriter, BuildResult}; -use crate::build::plan::{Crate, CrateKind, Edition}; +use crate::build::plan::{Crate, Edition}; use crate::config::{ClippyPreference, Config}; use std::collections::{HashMap, HashSet}; @@ -285,17 +286,22 @@ impl<'a> CompilerCalls<'a> for RlsRustcCalls { } result.after_expand.callback = Box::new(move |state| { - let krate = Crate { - name: state.crate_name.expect("missing crate name").to_owned(), - src_path: match state.input { + let cwd = &state.session.working_dir.0; + + let src_path = match state.input { Input::File(ref name) => Some(name.to_path_buf()), Input::Str { .. } => None, - }, - kind: CrateKind::Library, // FIXME + }.and_then(|path| src_path(Some(cwd), path)); + + + let krate = Crate { + name: state.crate_name.expect("missing crate name").to_owned(), + src_path, disambiguator: state.session.local_crate_disambiguator().to_fingerprint().as_value(), edition: match state.session.edition() { - syntax::edition::Edition::Edition2018 => Edition::Edition2018, - _ => Edition::Edition2015, + RustcEdition::Edition2015 => Edition::Edition2015, + RustcEdition::Edition2018 => Edition::Edition2018, + _ => Edition::default(), }, }; let files = fetch_input_files(state.session); @@ -348,12 +354,15 @@ impl<'a> CompilerCalls<'a> for RlsRustcCalls { } fn fetch_input_files(sess: &Session) -> Vec { + let cwd = &sess.working_dir.0; + sess.source_map() .files() .iter() .filter(|fmap| fmap.is_real_file()) .filter(|fmap| !fmap.is_imported()) .map(|fmap| fmap.name.to_string()) + .map(|fmap| src_path(Some(cwd), fmap).unwrap()) .map(PathBuf::from) .collect() } From e2c8c2879b57cb7fd4d9865b560ce4f0ca9808f8 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Wed, 14 Nov 2018 16:18:04 +0100 Subject: [PATCH 5/5] Revert calculating dirty units from pre-cached inputs Reverts change made in bdf0d9253009c2de263e22a15d794ff65a05d1b1. There were some edge cases wrt relative/absolute paths and varying inputs cwd, so for now don't use the patch and rely on the old logic, instead. --- src/build/cargo_plan.rs | 70 ++++++++++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 15 deletions(-) diff --git a/src/build/cargo_plan.rs b/src/build/cargo_plan.rs index 4fb5698da72..245a33e932e 100644 --- a/src/build/cargo_plan.rs +++ b/src/build/cargo_plan.rs @@ -128,7 +128,7 @@ impl CargoPlan { let unit_key = (id.clone(), target.clone(), mode); trace!("Caching these files: {:#?} for {:?} key", &input_files, &unit_key); - // Create reverse file -> unit mapping (used for dirty unit calculation) + // Create reverse file -> unit mapping (to be used for dirty unit calculation) for file in &input_files { self.file_key_mapping .entry(file.to_path_buf()) @@ -197,7 +197,6 @@ impl CargoPlan { } } - /// TODO: Update TODO below /// TODO: Improve detecting dirty crate targets for a set of dirty file paths. /// This uses a lousy heuristic of checking path prefix for a given crate /// target to determine whether a given unit (crate target) is dirty. This @@ -207,7 +206,7 @@ impl CargoPlan { /// Because of that, build scripts are checked separately and only other /// crate targets are checked with path prefixes. fn fetch_dirty_units>(&self, files: &[T]) -> HashSet { - let files = files.iter().map(|f| f.as_ref()); + let mut result = HashSet::new(); let build_scripts: HashMap<&Path, UnitKey> = self .units @@ -217,18 +216,59 @@ impl CargoPlan { }) .map(|(key, unit)| (unit.target.src_path().path(), key.clone())) .collect(); - - let mut result = HashSet::new(); - - let dirty_rustc_units = files - .clone() - .filter_map(|f| self.file_key_mapping.get(f)) - .flat_map(|units| units); - let dirty_build_scripts = files.filter_map(|f| build_scripts.get(f)); - - result.extend(dirty_rustc_units.cloned()); - result.extend(dirty_build_scripts.cloned()); - + let other_targets: HashMap = self + .units + .iter() + .filter(|&(&(_, ref target, _), _)| *target.kind() != TargetKind::CustomBuild) + .map(|(key, unit)| { + ( + key.clone(), + unit.target + .src_path() + .path() + .parent() + .expect("no parent for src_path"), + ) + }).collect(); + + for modified in files.iter().map(|x| x.as_ref()) { + if let Some(unit) = build_scripts.get(modified) { + result.insert(unit.clone()); + } else { + // Not a build script, so we associate a dirty file with a + // package by finding longest (most specified) path prefix. + let matching_prefix_components = |a: &Path, b: &Path| -> usize { + assert!(a.is_absolute() && b.is_absolute()); + a.components().zip(b.components()) + .skip(1) // Skip RootDir + .take_while(|&(x, y)| x == y) + .count() + }; + // Since a package can correspond to many units (e.g. compiled + // as a regular binary or a test harness for unit tests), we + // collect every unit having the longest path prefix. + let max_matching_prefix = other_targets + .values() + .map(|src_dir| matching_prefix_components(modified, src_dir)) + .max(); + + match max_matching_prefix { + Some(0) => error!( + "Modified file {} didn't correspond to any buildable unit!", + modified.display() + ), + Some(max) => { + let dirty_units = other_targets + .iter() + .filter(|(_, dir)| max == matching_prefix_components(modified, dir)) + .map(|(unit, _)| unit); + + result.extend(dirty_units.cloned()); + } + None => {} // Possible that only build scripts were modified + } + } + } result }