From e85089b63a2b67f4048da207b39cc265887e018f Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Thu, 3 Oct 2019 21:29:25 -0400 Subject: [PATCH 01/17] Remove -Zprofile-queries --- src/librustc/dep_graph/graph.rs | 9 - src/librustc/session/config.rs | 4 - src/librustc/session/mod.rs | 14 +- src/librustc/ty/query/on_disk_cache.rs | 2 +- src/librustc/ty/query/plumbing.rs | 49 ---- src/librustc/util/common.rs | 75 +----- src/librustc_codegen_llvm/back/lto.rs | 6 +- src/librustc_codegen_llvm/back/write.rs | 4 +- src/librustc_codegen_ssa/back/link.rs | 2 +- src/librustc_incremental/persist/load.rs | 2 +- src/librustc_interface/interface.rs | 13 +- src/librustc_interface/lib.rs | 1 - src/librustc_interface/profile/mod.rs | 297 ---------------------- src/librustc_interface/profile/trace.rs | 304 ----------------------- 14 files changed, 11 insertions(+), 771 deletions(-) delete mode 100644 src/librustc_interface/profile/mod.rs delete mode 100644 src/librustc_interface/profile/trace.rs diff --git a/src/librustc/dep_graph/graph.rs b/src/librustc/dep_graph/graph.rs index 0c56fc7914b4c..b08c710534a9b 100644 --- a/src/librustc/dep_graph/graph.rs +++ b/src/librustc/dep_graph/graph.rs @@ -9,7 +9,6 @@ use std::hash::Hash; use std::collections::hash_map::Entry; use std::mem; use crate::ty::{self, TyCtxt}; -use crate::util::common::{ProfileQueriesMsg, profq_msg}; use parking_lot::{Mutex, Condvar}; use crate::ich::{StableHashingContext, StableHashingContextProvider, Fingerprint}; @@ -260,10 +259,6 @@ impl DepGraph { // - we can get an idea of the runtime cost. let mut hcx = cx.get_stable_hashing_context(); - if cfg!(debug_assertions) { - profq_msg(hcx.sess(), ProfileQueriesMsg::TaskBegin(key.clone())) - }; - let result = if no_tcx { task(cx, arg) } else { @@ -279,10 +274,6 @@ impl DepGraph { }) }; - if cfg!(debug_assertions) { - profq_msg(hcx.sess(), ProfileQueriesMsg::TaskEnd) - }; - let current_fingerprint = hash_result(&mut hcx, &result); let dep_node_index = finish_task_and_alloc_depnode( diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 2771ce69b9e0d..ed8aafa25caa4 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -1316,10 +1316,6 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "dump the dependency graph to $RUST_DEP_GRAPH (default: /tmp/dep_graph.gv)"), query_dep_graph: bool = (false, parse_bool, [UNTRACKED], "enable queries of the dependency graph for regression testing"), - profile_queries: bool = (false, parse_bool, [UNTRACKED], - "trace and profile the queries of the incremental compilation framework"), - profile_queries_and_keys: bool = (false, parse_bool, [UNTRACKED], - "trace and profile the queries and keys of the incremental compilation framework"), no_analysis: bool = (false, parse_bool, [UNTRACKED], "parse and expand the source, but run no analysis"), extra_plugins: Vec = (Vec::new(), parse_list, [TRACKED], diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index f22445f5d4744..9d60221fa3d75 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -11,7 +11,6 @@ use crate::session::config::{OutputType, PrintRequest, SwitchWithOptPath}; use crate::session::search_paths::{PathKind, SearchPath}; use crate::util::nodemap::{FxHashMap, FxHashSet}; use crate::util::common::{duration_to_secs_str, ErrorReported}; -use crate::util::common::ProfileQueriesMsg; use rustc_data_structures::base_n; use rustc_data_structures::sync::{ @@ -46,7 +45,7 @@ use std::fmt; use std::io::Write; use std::path::PathBuf; use std::time::Duration; -use std::sync::{Arc, mpsc}; +use std::sync::Arc; mod code_stats; pub mod config; @@ -125,9 +124,6 @@ pub struct Session { /// `-Zquery-dep-graph` is specified. pub cgu_reuse_tracker: CguReuseTracker, - /// Used by `-Z profile-queries` in `util::common`. - pub profile_channel: Lock>>, - /// Used by `-Z self-profile`. pub prof: SelfProfilerRef, @@ -509,13 +505,6 @@ impl Session { pub fn time_extended(&self) -> bool { self.opts.debugging_opts.time_passes } - pub fn profile_queries(&self) -> bool { - self.opts.debugging_opts.profile_queries - || self.opts.debugging_opts.profile_queries_and_keys - } - pub fn profile_queries_and_keys(&self) -> bool { - self.opts.debugging_opts.profile_queries_and_keys - } pub fn instrument_mcount(&self) -> bool { self.opts.debugging_opts.instrument_mcount } @@ -1234,7 +1223,6 @@ fn build_session_( incr_comp_session: OneThread::new(RefCell::new(IncrCompSession::NotInitialized)), cgu_reuse_tracker, prof: SelfProfilerRef::new(self_profiler), - profile_channel: Lock::new(None), perf_stats: PerfStats { symbol_hash_time: Lock::new(Duration::from_secs(0)), decode_def_path_tables_time: Lock::new(Duration::from_secs(0)), diff --git a/src/librustc/ty/query/on_disk_cache.rs b/src/librustc/ty/query/on_disk_cache.rs index 97fafe341a311..1bba7fdd863ea 100644 --- a/src/librustc/ty/query/on_disk_cache.rs +++ b/src/librustc/ty/query/on_disk_cache.rs @@ -1075,7 +1075,7 @@ where let desc = &format!("encode_query_results for {}", ::std::any::type_name::()); - time_ext(tcx.sess.time_extended(), Some(tcx.sess), desc, || { + time_ext(tcx.sess.time_extended(), desc, || { let shards = Q::query_cache(tcx).lock_shards(); assert!(shards.iter().all(|shard| shard.active.is_empty())); for (key, entry) in shards.iter().flat_map(|shard| shard.results.iter()) { diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 955f1447c55b6..f7046c4ab26fa 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -9,8 +9,6 @@ use crate::ty::query::Query; use crate::ty::query::config::{QueryConfig, QueryDescription}; use crate::ty::query::job::{QueryJob, QueryResult, QueryInfo}; -use crate::util::common::{profq_msg, ProfileQueriesMsg, QueryMsg}; - use errors::DiagnosticBuilder; use errors::Level; use errors::Diagnostic; @@ -62,33 +60,6 @@ impl<'tcx, M: QueryConfig<'tcx>> Default for QueryCache<'tcx, M> { } } -// If enabled, sends a message to the profile-queries thread. -macro_rules! profq_msg { - ($tcx:expr, $msg:expr) => { - if cfg!(debug_assertions) { - if $tcx.sess.profile_queries() { - profq_msg($tcx.sess, $msg) - } - } - } -} - -// If enabled, formats a key using its debug string, which can be -// expensive to compute (in terms of time). -macro_rules! profq_query_msg { - ($query:expr, $tcx:expr, $key:expr) => {{ - let msg = if cfg!(debug_assertions) { - if $tcx.sess.profile_queries_and_keys() { - Some(format!("{:?}", $key)) - } else { None } - } else { None }; - QueryMsg { - query: $query, - msg, - } - }} -} - /// A type representing the responsibility to execute the job in the `job` field. /// This will poison the relevant query if dropped. pub(super) struct JobOwner<'a, 'tcx, Q: QueryDescription<'tcx>> { @@ -111,7 +82,6 @@ impl<'a, 'tcx, Q: QueryDescription<'tcx>> JobOwner<'a, 'tcx, Q> { loop { let mut lock = cache.get_shard_by_value(key).lock(); if let Some(value) = lock.results.get(key) { - profq_msg!(tcx, ProfileQueriesMsg::CacheHit); tcx.prof.query_cache_hit(Q::NAME); let result = (value.value.clone(), value.index); #[cfg(debug_assertions)] @@ -358,13 +328,6 @@ impl<'tcx> TyCtxt<'tcx> { key, span); - profq_msg!(self, - ProfileQueriesMsg::QueryBegin( - span.data(), - profq_query_msg!(Q::NAME.as_str(), self, key), - ) - ); - let job = match JobOwner::try_get(self, span, &key) { TryGetJob::NotYetStarted(job) => job, TryGetJob::Cycle(result) => return result, @@ -383,7 +346,6 @@ impl<'tcx> TyCtxt<'tcx> { if Q::ANON { - profq_msg!(self, ProfileQueriesMsg::ProviderBegin); let prof_timer = self.prof.query_provider(Q::NAME); let ((result, dep_node_index), diagnostics) = with_diagnostics(|diagnostics| { @@ -395,7 +357,6 @@ impl<'tcx> TyCtxt<'tcx> { }); drop(prof_timer); - profq_msg!(self, ProfileQueriesMsg::ProviderEnd); self.dep_graph.read_index(dep_node_index); @@ -468,7 +429,6 @@ impl<'tcx> TyCtxt<'tcx> { }; let result = if let Some(result) = result { - profq_msg!(self, ProfileQueriesMsg::CacheHit); result } else { // We could not load a result from the on-disk cache, so @@ -546,7 +506,6 @@ impl<'tcx> TyCtxt<'tcx> { - dep-node: {:?}", key, dep_node); - profq_msg!(self, ProfileQueriesMsg::ProviderBegin); let prof_timer = self.prof.query_provider(Q::NAME); let ((result, dep_node_index), diagnostics) = with_diagnostics(|diagnostics| { @@ -568,7 +527,6 @@ impl<'tcx> TyCtxt<'tcx> { }); drop(prof_timer); - profq_msg!(self, ProfileQueriesMsg::ProviderEnd); if unlikely!(self.sess.opts.debugging_opts.query_dep_graph) { self.dep_graph.mark_loaded_from_cache(dep_node_index, false); @@ -614,19 +572,12 @@ impl<'tcx> TyCtxt<'tcx> { let _ = self.get_query::(DUMMY_SP, key); } else { - profq_msg!(self, ProfileQueriesMsg::CacheHit); self.prof.query_cache_hit(Q::NAME); } } #[allow(dead_code)] fn force_query>(self, key: Q::Key, span: Span, dep_node: DepNode) { - profq_msg!( - self, - ProfileQueriesMsg::QueryBegin(span.data(), - profq_query_msg!(Q::NAME.as_str(), self, key)) - ); - // We may be concurrently trying both execute and force a query. // Ensure that only one of them runs the query. let job = match JobOwner::try_get(self, span, &key) { diff --git a/src/librustc/util/common.rs b/src/librustc/util/common.rs index 0f472126695e0..3e52a6aa50850 100644 --- a/src/librustc/util/common.rs +++ b/src/librustc/util/common.rs @@ -6,11 +6,8 @@ use std::cell::Cell; use std::fmt::Debug; use std::time::{Duration, Instant}; -use std::sync::mpsc::{Sender}; -use syntax_pos::{SpanData}; use syntax::symbol::{Symbol, sym}; use rustc_macros::HashStable; -use crate::dep_graph::{DepNode}; use crate::session::Session; #[cfg(test)] @@ -26,17 +23,6 @@ pub struct ErrorReported; thread_local!(static TIME_DEPTH: Cell = Cell::new(0)); -/// Parameters to the `Dump` variant of type `ProfileQueriesMsg`. -#[derive(Clone,Debug)] -pub struct ProfQDumpParams { - /// A base path for the files we will dump. - pub path:String, - /// To ensure that the compiler waits for us to finish our dumps. - pub ack:Sender<()>, - /// Toggle dumping a log file with every `ProfileQueriesMsg`. - pub dump_profq_msg_log:bool, -} - #[allow(nonstandard_style)] #[derive(Clone, Debug, PartialEq, Eq)] pub struct QueryMsg { @@ -44,53 +30,6 @@ pub struct QueryMsg { pub msg: Option, } -/// A sequence of these messages induce a trace of query-based incremental compilation. -// FIXME(matthewhammer): Determine whether we should include cycle detection here or not. -#[derive(Clone,Debug)] -pub enum ProfileQueriesMsg { - /// Begin a timed pass. - TimeBegin(String), - /// End a timed pass. - TimeEnd, - /// Begin a task (see `dep_graph::graph::with_task`). - TaskBegin(DepNode), - /// End a task. - TaskEnd, - /// Begin a new query. - /// Cannot use `Span` because queries are sent to other thread. - QueryBegin(SpanData, QueryMsg), - /// Query is satisfied by using an already-known value for the given key. - CacheHit, - /// Query requires running a provider; providers may nest, permitting queries to nest. - ProviderBegin, - /// Query is satisfied by a provider terminating with a value. - ProviderEnd, - /// Dump a record of the queries to the given path. - Dump(ProfQDumpParams), - /// Halt the profiling/monitoring background thread. - Halt -} - -/// If enabled, send a message to the profile-queries thread. -pub fn profq_msg(sess: &Session, msg: ProfileQueriesMsg) { - if let Some(s) = sess.profile_channel.borrow().as_ref() { - s.send(msg).unwrap() - } else { - // Do nothing. - } -} - -/// Set channel for profile queries channel. -pub fn profq_set_chan(sess: &Session, s: Sender) -> bool { - let mut channel = sess.profile_channel.borrow_mut(); - if channel.is_none() { - *channel = Some(s); - true - } else { - false - } -} - /// Read the current depth of `time()` calls. This is used to /// encourage indentation across threads. pub fn time_depth() -> usize { @@ -107,10 +46,10 @@ pub fn set_time_depth(depth: usize) { pub fn time(sess: &Session, what: &str, f: F) -> T where F: FnOnce() -> T, { - time_ext(sess.time_passes(), Some(sess), what, f) + time_ext(sess.time_passes(), what, f) } -pub fn time_ext(do_it: bool, sess: Option<&Session>, what: &str, f: F) -> T where +pub fn time_ext(do_it: bool, what: &str, f: F) -> T where F: FnOnce() -> T, { if !do_it { return f(); } @@ -121,19 +60,9 @@ pub fn time_ext(do_it: bool, sess: Option<&Session>, what: &str, f: F) -> r }); - if let Some(sess) = sess { - if cfg!(debug_assertions) { - profq_msg(sess, ProfileQueriesMsg::TimeBegin(what.to_string())) - } - } let start = Instant::now(); let rv = f(); let dur = start.elapsed(); - if let Some(sess) = sess { - if cfg!(debug_assertions) { - profq_msg(sess, ProfileQueriesMsg::TimeEnd) - } - } print_time_passes_entry(true, what, dur); diff --git a/src/librustc_codegen_llvm/back/lto.rs b/src/librustc_codegen_llvm/back/lto.rs index c4368d2cb8b45..da74c69d7d707 100644 --- a/src/librustc_codegen_llvm/back/lto.rs +++ b/src/librustc_codegen_llvm/back/lto.rs @@ -116,7 +116,7 @@ fn prepare_lto(cgcx: &CodegenContext, info!("adding bytecode {}", name); let bc_encoded = data.data(); - let (bc, id) = time_ext(cgcx.time_passes, None, &format!("decode {}", name), || { + let (bc, id) = time_ext(cgcx.time_passes, &format!("decode {}", name), || { match DecodedBytecode::new(bc_encoded) { Ok(b) => Ok((b.bytecode(), b.identifier().to_string())), Err(e) => Err(diag_handler.fatal(&e)), @@ -295,7 +295,7 @@ fn fat_lto(cgcx: &CodegenContext, for (bc_decoded, name) in serialized_modules { let _timer = cgcx.prof.generic_activity("LLVM_fat_lto_link_module"); info!("linking {:?}", name); - time_ext(cgcx.time_passes, None, &format!("ll link {:?}", name), || { + time_ext(cgcx.time_passes, &format!("ll link {:?}", name), || { let data = bc_decoded.data(); linker.add(&data).map_err(|()| { let msg = format!("failed to load bc of {:?}", name); @@ -590,7 +590,7 @@ pub(crate) fn run_pass_manager(cgcx: &CodegenContext, llvm::LLVMRustAddPass(pm, pass.unwrap()); } - time_ext(cgcx.time_passes, None, "LTO passes", || + time_ext(cgcx.time_passes, "LTO passes", || llvm::LLVMRunPassManager(pm, module.module_llvm.llmod())); llvm::LLVMDisposePassManager(pm); diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 78db90b57b53d..17cf9ed5ba769 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -427,7 +427,6 @@ pub(crate) unsafe fn optimize(cgcx: &CodegenContext, { let _timer = cgcx.prof.generic_activity("LLVM_module_optimize_function_passes"); time_ext(config.time_passes, - None, &format!("llvm function passes [{}]", module_name.unwrap()), || { llvm::LLVMRustRunFunctionPassManager(fpm, llmod) @@ -436,7 +435,6 @@ pub(crate) unsafe fn optimize(cgcx: &CodegenContext, { let _timer = cgcx.prof.generic_activity("LLVM_module_optimize_module_passes"); time_ext(config.time_passes, - None, &format!("llvm module passes [{}]", module_name.unwrap()), || { llvm::LLVMRunPassManager(mpm, llmod) @@ -538,7 +536,7 @@ pub(crate) unsafe fn codegen(cgcx: &CodegenContext, embed_bitcode(cgcx, llcx, llmod, None); } - time_ext(config.time_passes, None, &format!("codegen passes [{}]", module_name.unwrap()), + time_ext(config.time_passes, &format!("codegen passes [{}]", module_name.unwrap()), || -> Result<(), FatalError> { if config.emit_ir { let _timer = cgcx.prof.generic_activity("LLVM_module_codegen_emit_ir"); diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index 3b7ae5e33d5e7..1ce008e4d74d2 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -1527,7 +1527,7 @@ fn add_upstream_rust_crates<'a, B: ArchiveBuilder<'a>>( let name = cratepath.file_name().unwrap().to_str().unwrap(); let name = &name[3..name.len() - 5]; // chop off lib/.rlib - time_ext(sess.time_extended(), Some(sess), &format!("altering {}.rlib", name), || { + time_ext(sess.time_extended(), &format!("altering {}.rlib", name), || { let mut archive = ::new(sess, &dst, Some(cratepath)); archive.update_symbols(); diff --git a/src/librustc_incremental/persist/load.rs b/src/librustc_incremental/persist/load.rs index 90aefb0f32416..b7f4df62b494b 100644 --- a/src/librustc_incremental/persist/load.rs +++ b/src/librustc_incremental/persist/load.rs @@ -160,7 +160,7 @@ pub fn load_dep_graph(sess: &Session) -> DepGraphFuture { } MaybeAsync::Async(std::thread::spawn(move || { - time_ext(time_passes, None, "background load prev dep-graph", move || { + time_ext(time_passes, "background load prev dep-graph", move || { match load_data(report_incremental_info, &path) { LoadResult::DataOutOfDate => LoadResult::DataOutOfDate, LoadResult::Error { message } => LoadResult::Error { message }, diff --git a/src/librustc_interface/interface.rs b/src/librustc_interface/interface.rs index dae8fb242d58c..b26bd75c974c6 100644 --- a/src/librustc_interface/interface.rs +++ b/src/librustc_interface/interface.rs @@ -1,6 +1,5 @@ use crate::queries::Queries; use crate::util; -use crate::profile; pub use crate::passes::BoxedResolver; use rustc::lint; @@ -115,17 +114,7 @@ where compiler.sess.diagnostic().print_error_count(&util::diagnostics_registry()); }); - if compiler.sess.profile_queries() { - profile::begin(&compiler.sess); - } - - let r = f(&compiler); - - if compiler.sess.profile_queries() { - profile::dump(&compiler.sess, "profile_queries".to_string()) - } - - r + f(&compiler) } pub fn run_compiler(mut config: Config, f: F) -> R diff --git a/src/librustc_interface/lib.rs b/src/librustc_interface/lib.rs index 2e593d441553a..6be36e9b9001b 100644 --- a/src/librustc_interface/lib.rs +++ b/src/librustc_interface/lib.rs @@ -16,6 +16,5 @@ mod passes; mod queries; pub mod util; mod proc_macro_decls; -mod profile; pub use interface::{run_compiler, Config}; diff --git a/src/librustc_interface/profile/mod.rs b/src/librustc_interface/profile/mod.rs deleted file mode 100644 index 2e71d46f4154c..0000000000000 --- a/src/librustc_interface/profile/mod.rs +++ /dev/null @@ -1,297 +0,0 @@ -use log::debug; -use rustc::dep_graph::DepNode; -use rustc::session::Session; -use rustc::util::common::{ProfQDumpParams, ProfileQueriesMsg, profq_msg, profq_set_chan}; -use std::sync::mpsc::{Receiver}; -use std::io::{Write}; -use std::time::{Duration, Instant}; - -pub mod trace; - -/// begin a profile thread, if not already running -pub fn begin(sess: &Session) { - use std::thread; - use std::sync::mpsc::{channel}; - let (tx, rx) = channel(); - if profq_set_chan(sess, tx) { - thread::spawn(move || profile_queries_thread(rx)); - } -} - -/// dump files with profiling information to the given base path, and -/// wait for this dump to complete. -/// -/// wraps the RPC (send/recv channel logic) of requesting a dump. -pub fn dump(sess: &Session, path: String) { - use std::sync::mpsc::{channel}; - let (tx, rx) = channel(); - let params = ProfQDumpParams { - path, - ack: tx, - // FIXME: Add another compiler flag to toggle whether this log - // is written; false for now - dump_profq_msg_log: true, - }; - profq_msg(sess, ProfileQueriesMsg::Dump(params)); - let _ = rx.recv().unwrap(); -} - -// State for parsing recursive trace structure in separate thread, via messages -#[derive(Clone, Eq, PartialEq)] -enum ParseState { - // No (local) parse state; may be parsing a tree, focused on a - // sub-tree that could be anything. - Clear, - // Have Query information from the last message - HaveQuery(trace::Query, Instant), - // Have "time-begin" information from the last message (doit flag, and message) - HaveTimeBegin(String, Instant), - // Have "task-begin" information from the last message - HaveTaskBegin(DepNode, Instant), -} -struct StackFrame { - pub parse_st: ParseState, - pub traces: Vec, -} - -fn total_duration(traces: &[trace::Rec]) -> Duration { - Duration::new(0, 0) + traces.iter().map(|t| t.dur_total).sum() -} - -// profiling thread; retains state (in local variables) and dump traces, upon request. -fn profile_queries_thread(r: Receiver) { - use self::trace::*; - use std::fs::File; - - let mut profq_msgs: Vec = vec![]; - let mut frame: StackFrame = StackFrame { parse_st: ParseState::Clear, traces: vec![] }; - let mut stack: Vec = vec![]; - loop { - let msg = r.recv(); - if let Err(_recv_err) = msg { - // FIXME: Perhaps do something smarter than simply quitting? - break - }; - let msg = msg.unwrap(); - debug!("profile_queries_thread: {:?}", msg); - - // Meta-level versus _actual_ queries messages - match msg { - ProfileQueriesMsg::Halt => return, - ProfileQueriesMsg::Dump(params) => { - assert!(stack.is_empty()); - assert!(frame.parse_st == ParseState::Clear); - - // write log of all messages - if params.dump_profq_msg_log { - let mut log_file = - File::create(format!("{}.log.txt", params.path)).unwrap(); - for m in profq_msgs.iter() { - writeln!(&mut log_file, "{:?}", m).unwrap() - }; - } - - // write HTML file, and counts file - let html_path = format!("{}.html", params.path); - let mut html_file = File::create(&html_path).unwrap(); - - let counts_path = format!("{}.counts.txt", params.path); - let mut counts_file = File::create(&counts_path).unwrap(); - - writeln!(html_file, - "\n\n", - "profile_queries.css").unwrap(); - writeln!(html_file, "\n\n").unwrap(); - trace::write_traces(&mut html_file, &mut counts_file, &frame.traces); - writeln!(html_file, "\n").unwrap(); - - let ack_path = format!("{}.ack", params.path); - let ack_file = File::create(&ack_path).unwrap(); - drop(ack_file); - - // Tell main thread that we are done, e.g., so it can exit - params.ack.send(()).unwrap(); - } - // Actual query message: - msg => { - // Record msg in our log - profq_msgs.push(msg.clone()); - // Respond to the message, knowing that we've already handled Halt and Dump, above. - match (frame.parse_st.clone(), msg) { - (_, ProfileQueriesMsg::Halt) | (_, ProfileQueriesMsg::Dump(_)) => { - unreachable!(); - }, - // Parse State: Clear - (ParseState::Clear, - ProfileQueriesMsg::QueryBegin(span, querymsg)) => { - let start = Instant::now(); - frame.parse_st = ParseState::HaveQuery - (Query { span, msg: querymsg }, start) - }, - (ParseState::Clear, - ProfileQueriesMsg::CacheHit) => { - panic!("parse error: unexpected CacheHit; expected QueryBegin") - }, - (ParseState::Clear, - ProfileQueriesMsg::ProviderBegin) => { - panic!("parse error: expected QueryBegin before beginning a provider") - }, - (ParseState::Clear, - ProfileQueriesMsg::ProviderEnd) => { - let provider_extent = frame.traces; - match stack.pop() { - None => - panic!("parse error: expected a stack frame; found an empty stack"), - Some(old_frame) => { - match old_frame.parse_st { - ParseState::HaveQuery(q, start) => { - let duration = start.elapsed(); - frame = StackFrame{ - parse_st: ParseState::Clear, - traces: old_frame.traces - }; - let dur_extent = total_duration(&provider_extent); - let trace = Rec { - effect: Effect::QueryBegin(q, CacheCase::Miss), - extent: Box::new(provider_extent), - start: start, - dur_self: duration - dur_extent, - dur_total: duration, - }; - frame.traces.push( trace ); - }, - _ => panic!("internal parse error: malformed parse stack") - } - } - } - }, - (ParseState::Clear, - ProfileQueriesMsg::TimeBegin(msg)) => { - let start = Instant::now(); - frame.parse_st = ParseState::HaveTimeBegin(msg, start); - stack.push(frame); - frame = StackFrame{parse_st: ParseState::Clear, traces: vec![]}; - }, - (_, ProfileQueriesMsg::TimeBegin(_)) => { - panic!("parse error; did not expect time begin here"); - }, - (ParseState::Clear, - ProfileQueriesMsg::TimeEnd) => { - let provider_extent = frame.traces; - match stack.pop() { - None => - panic!("parse error: expected a stack frame; found an empty stack"), - Some(old_frame) => { - match old_frame.parse_st { - ParseState::HaveTimeBegin(msg, start) => { - let duration = start.elapsed(); - frame = StackFrame{ - parse_st: ParseState::Clear, - traces: old_frame.traces - }; - let dur_extent = total_duration(&provider_extent); - let trace = Rec { - effect: Effect::TimeBegin(msg), - extent: Box::new(provider_extent), - start: start, - dur_total: duration, - dur_self: duration - dur_extent, - }; - frame.traces.push( trace ); - }, - _ => panic!("internal parse error: malformed parse stack") - } - } - } - }, - (_, ProfileQueriesMsg::TimeEnd) => { - panic!("parse error") - }, - (ParseState::Clear, - ProfileQueriesMsg::TaskBegin(key)) => { - let start = Instant::now(); - frame.parse_st = ParseState::HaveTaskBegin(key, start); - stack.push(frame); - frame = StackFrame{ parse_st: ParseState::Clear, traces: vec![] }; - }, - (_, ProfileQueriesMsg::TaskBegin(_)) => { - panic!("parse error; did not expect time begin here"); - }, - (ParseState::Clear, - ProfileQueriesMsg::TaskEnd) => { - let provider_extent = frame.traces; - match stack.pop() { - None => - panic!("parse error: expected a stack frame; found an empty stack"), - Some(old_frame) => { - match old_frame.parse_st { - ParseState::HaveTaskBegin(key, start) => { - let duration = start.elapsed(); - frame = StackFrame{ - parse_st: ParseState::Clear, - traces: old_frame.traces - }; - let dur_extent = total_duration(&provider_extent); - let trace = Rec { - effect: Effect::TaskBegin(key), - extent: Box::new(provider_extent), - start: start, - dur_total: duration, - dur_self: duration - dur_extent, - }; - frame.traces.push( trace ); - }, - _ => panic!("internal parse error: malformed parse stack") - } - } - } - }, - (_, ProfileQueriesMsg::TaskEnd) => { - panic!("parse error") - }, - // Parse State: HaveQuery - (ParseState::HaveQuery(q,start), - ProfileQueriesMsg::CacheHit) => { - let duration = start.elapsed(); - let trace : Rec = Rec{ - effect: Effect::QueryBegin(q, CacheCase::Hit), - extent: Box::new(vec![]), - start: start, - dur_self: duration, - dur_total: duration, - }; - frame.traces.push( trace ); - frame.parse_st = ParseState::Clear; - }, - (ParseState::HaveQuery(_, _), - ProfileQueriesMsg::ProviderBegin) => { - stack.push(frame); - frame = StackFrame{ parse_st: ParseState::Clear, traces: vec![] }; - }, - - // Parse errors: - - (ParseState::HaveQuery(q, _), - ProfileQueriesMsg::ProviderEnd) => { - panic!("parse error: unexpected ProviderEnd; \ - expected something else to follow BeginQuery for {:?}", q) - }, - (ParseState::HaveQuery(q1, _), - ProfileQueriesMsg::QueryBegin(span2, querymsg2)) => { - panic!("parse error: unexpected QueryBegin; \ - earlier query is unfinished: {:?} and now {:?}", - q1, Query{span:span2, msg: querymsg2}) - }, - (ParseState::HaveTimeBegin(_, _), _) => { - unreachable!() - }, - (ParseState::HaveTaskBegin(_, _), _) => { - unreachable!() - }, - } - } - } - } -} diff --git a/src/librustc_interface/profile/trace.rs b/src/librustc_interface/profile/trace.rs deleted file mode 100644 index 95c4ea6ff2347..0000000000000 --- a/src/librustc_interface/profile/trace.rs +++ /dev/null @@ -1,304 +0,0 @@ -use super::*; -use syntax_pos::SpanData; -use rustc_data_structures::fx::FxHashMap; -use rustc::util::common::QueryMsg; -use std::fs::File; -use std::time::{Duration, Instant}; -use rustc::dep_graph::{DepNode}; - -#[derive(Debug, Clone, Eq, PartialEq)] -pub struct Query { - pub span: SpanData, - pub msg: QueryMsg, -} -pub enum Effect { - QueryBegin(Query, CacheCase), - TimeBegin(String), - TaskBegin(DepNode), -} -pub enum CacheCase { - Hit, Miss -} -/// Recursive trace structure -pub struct Rec { - pub effect: Effect, - pub start: Instant, - pub dur_self: Duration, - pub dur_total: Duration, - pub extent: Box>, -} -pub struct QueryMetric { - pub count: usize, - pub dur_self: Duration, - pub dur_total: Duration, -} - -fn cons(s: &str) -> String { - let first = s.split(|d| d == '(' || d == '{').next(); - assert!(first.is_some() && first != Some("")); - first.unwrap().to_owned() -} - -pub fn cons_of_query_msg(q: &trace::Query) -> String { - cons(&format!("{:?}", q.msg)) -} - -pub fn cons_of_key(k: &DepNode) -> String { - cons(&format!("{:?}", k)) -} - -// First return value is text; second return value is a CSS class -pub fn html_of_effect(eff: &Effect) -> (String, String) { - match *eff { - Effect::TimeBegin(ref msg) => { - (msg.clone(), - "time-begin".to_string()) - }, - Effect::TaskBegin(ref key) => { - let cons = cons_of_key(key); - (cons.clone(), format!("{} task-begin", cons)) - }, - Effect::QueryBegin(ref qmsg, ref cc) => { - let cons = cons_of_query_msg(qmsg); - (cons.clone(), - format!("{} {}", - cons, - match *cc { - CacheCase::Hit => "hit", - CacheCase::Miss => "miss", - })) - } - } -} - -// First return value is text; second return value is a CSS class -fn html_of_duration(_start: &Instant, dur: &Duration) -> (String, String) { - use rustc::util::common::duration_to_secs_str; - (duration_to_secs_str(dur.clone()), String::new()) -} - -fn html_of_fraction(frac: f64) -> (String, &'static str) { - let css = { - if frac > 0.50 { "frac-50" } - else if frac > 0.40 { "frac-40" } - else if frac > 0.30 { "frac-30" } - else if frac > 0.20 { "frac-20" } - else if frac > 0.10 { "frac-10" } - else if frac > 0.05 { "frac-05" } - else if frac > 0.02 { "frac-02" } - else if frac > 0.01 { "frac-01" } - else if frac > 0.001 { "frac-001" } - else { "frac-0" } - }; - let percent = frac * 100.0; - - if percent > 0.1 { - (format!("{:.1}%", percent), css) - } else { - ("< 0.1%".to_string(), css) - } -} - -fn total_duration(traces: &[Rec]) -> Duration { - Duration::new(0, 0) + traces.iter().map(|t| t.dur_total).sum() -} - -fn duration_div(nom: Duration, den: Duration) -> f64 { - fn to_nanos(d: Duration) -> u64 { - d.as_secs() * 1_000_000_000 + d.subsec_nanos() as u64 - } - - to_nanos(nom) as f64 / to_nanos(den) as f64 -} - -fn write_traces_rec(file: &mut File, traces: &[Rec], total: Duration, depth: usize) { - for t in traces { - let (eff_text, eff_css_classes) = html_of_effect(&t.effect); - let (dur_text, dur_css_classes) = html_of_duration(&t.start, &t.dur_total); - let fraction = duration_div(t.dur_total, total); - let percent = fraction * 100.0; - let (frc_text, frc_css_classes) = html_of_fraction(fraction); - writeln!(file, "
", - depth, - t.extent.len(), - /* Heuristic for 'important' CSS class: */ - if t.extent.len() > 5 || percent >= 1.0 { " important" } else { "" }, - eff_css_classes, - dur_css_classes, - frc_css_classes, - ).unwrap(); - writeln!(file, "
{}
", eff_text).unwrap(); - writeln!(file, "
{}
", dur_text).unwrap(); - writeln!(file, "
{}
", frc_text).unwrap(); - write_traces_rec(file, &t.extent, total, depth + 1); - writeln!(file, "
").unwrap(); - } -} - -fn compute_counts_rec(counts: &mut FxHashMap, traces: &[Rec]) { - counts.reserve(traces.len()); - for t in traces.iter() { - match t.effect { - Effect::TimeBegin(ref msg) => { - let qm = match counts.get(msg) { - Some(_qm) => panic!("TimeBegin with non-unique, repeat message"), - None => QueryMetric { - count: 1, - dur_self: t.dur_self, - dur_total: t.dur_total, - } - }; - counts.insert(msg.clone(), qm); - }, - Effect::TaskBegin(ref key) => { - let cons = cons_of_key(key); - let qm = match counts.get(&cons) { - Some(qm) => - QueryMetric { - count: qm.count + 1, - dur_self: qm.dur_self + t.dur_self, - dur_total: qm.dur_total + t.dur_total, - }, - None => QueryMetric { - count: 1, - dur_self: t.dur_self, - dur_total: t.dur_total, - } - }; - counts.insert(cons, qm); - }, - Effect::QueryBegin(ref qmsg, ref _cc) => { - let qcons = cons_of_query_msg(qmsg); - let qm = match counts.get(&qcons) { - Some(qm) => - QueryMetric { - count: qm.count + 1, - dur_total: qm.dur_total + t.dur_total, - dur_self: qm.dur_self + t.dur_self - }, - None => QueryMetric { - count: 1, - dur_total: t.dur_total, - dur_self: t.dur_self, - } - }; - counts.insert(qcons, qm); - } - } - compute_counts_rec(counts, &t.extent) - } -} - -pub fn write_counts(count_file: &mut File, counts: &mut FxHashMap) { - use rustc::util::common::duration_to_secs_str; - use std::cmp::Reverse; - - let mut data = counts.iter().map(|(ref cons, ref qm)| - (cons.clone(), qm.count.clone(), qm.dur_total.clone(), qm.dur_self.clone()) - ).collect::>(); - - data.sort_by_key(|k| Reverse(k.3)); - for (cons, count, dur_total, dur_self) in data { - writeln!(count_file, "{}, {}, {}, {}", - cons, count, - duration_to_secs_str(dur_total), - duration_to_secs_str(dur_self) - ).unwrap(); - } -} - -pub fn write_traces(html_file: &mut File, counts_file: &mut File, traces: &[Rec]) { - let capacity = traces.iter().fold(0, |acc, t| acc + 1 + t.extent.len()); - let mut counts = FxHashMap::with_capacity_and_hasher(capacity, Default::default()); - compute_counts_rec(&mut counts, traces); - write_counts(counts_file, &mut counts); - - let total: Duration = total_duration(traces); - write_traces_rec(html_file, traces, total, 0) -} - -pub fn write_style(html_file: &mut File) { - write!(html_file, "{}", " -body { - font-family: sans-serif; - background: black; -} -.trace { - color: black; - display: inline-block; - border-style: solid; - border-color: red; - border-width: 1px; - border-radius: 5px; - padding: 0px; - margin: 1px; - font-size: 0px; -} -.task-begin { - border-width: 1px; - color: white; - border-color: #ff8; - font-size: 0px; -} -.miss { - border-color: red; - border-width: 1px; -} -.extent-0 { - padding: 2px; -} -.time-begin { - border-width: 4px; - font-size: 12px; - color: white; - border-color: #afa; -} -.important { - border-width: 3px; - font-size: 12px; - color: white; - border-color: #f77; -} -.hit { - padding: 0px; - border-color: blue; - border-width: 3px; -} -.eff { - color: #fff; - display: inline-block; -} -.frc { - color: #7f7; - display: inline-block; -} -.dur { - display: none -} -.frac-50 { - padding: 10px; - border-width: 10px; - font-size: 32px; -} -.frac-40 { - padding: 8px; - border-width: 8px; - font-size: 24px; -} -.frac-30 { - padding: 6px; - border-width: 6px; - font-size: 18px; -} -.frac-20 { - padding: 4px; - border-width: 6px; - font-size: 16px; -} -.frac-10 { - padding: 2px; - border-width: 6px; - font-size: 14px; -} -").unwrap(); -} From 6c9f218f3e1f2bb9a6bc0cdf5a0aa16c220f1ee8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Fri, 4 Oct 2019 00:00:00 +0000 Subject: [PATCH 02/17] Disable stack probe when thread sanitizer is enabled When thread sanitizer instrumentation is enabled during compilation of stack probe function, the function will be miscompiled and trigger segmentation fault at runtime. Disable stack probes when tsan is enabled. --- src/librustc_codegen_llvm/attributes.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index 33b50401b22f1..a0bef5b781565 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/src/librustc_codegen_llvm/attributes.rs @@ -96,10 +96,12 @@ pub fn set_probestack(cx: &CodegenCx<'ll, '_>, llfn: &'ll Value) { } // Currently stack probes seem somewhat incompatible with the address - // sanitizer. With asan we're already protected from stack overflow anyway - // so we don't really need stack probes regardless. - if let Some(Sanitizer::Address) = cx.sess().opts.debugging_opts.sanitizer { - return + // sanitizer and thread sanitizer. With asan we're already protected from + // stack overflow anyway so we don't really need stack probes regardless. + match cx.sess().opts.debugging_opts.sanitizer { + Some(Sanitizer::Address) | + Some(Sanitizer::Thread) => return, + _ => {}, } // probestack doesn't play nice either with `-C profile-generate`. From fda0359f3951b4e604711e660da9fde9ba867f29 Mon Sep 17 00:00:00 2001 From: Mikko Rantanen Date: Thu, 3 Oct 2019 03:55:31 +0300 Subject: [PATCH 03/17] Add tests for libsyntax JSON format --- src/libsyntax/json.rs | 3 + src/libsyntax/json/tests.rs | 190 ++++++++++++++++++++++++++++++++++++ src/libsyntax/tests.rs | 4 +- 3 files changed, 195 insertions(+), 2 deletions(-) create mode 100644 src/libsyntax/json/tests.rs diff --git a/src/libsyntax/json.rs b/src/libsyntax/json.rs index 2423e1070fc3e..041c8c2c8cd4a 100644 --- a/src/libsyntax/json.rs +++ b/src/libsyntax/json.rs @@ -25,6 +25,9 @@ use std::sync::{Arc, Mutex}; use rustc_serialize::json::{as_json, as_pretty_json}; +#[cfg(test)] +mod tests; + pub struct JsonEmitter { dst: Box, registry: Option, diff --git a/src/libsyntax/json/tests.rs b/src/libsyntax/json/tests.rs new file mode 100644 index 0000000000000..5103cff6f9422 --- /dev/null +++ b/src/libsyntax/json/tests.rs @@ -0,0 +1,190 @@ +use super::*; + +use crate::tests::{Shared}; +use crate::json::JsonEmitter; +use crate::source_map::{SourceMap, FilePathMapping}; +use crate::with_default_globals; + +use errors::emitter::{HumanReadableErrorType, ColorConfig}; +use errors::Handler; +use rustc_serialize::json::decode; +use syntax_pos::{BytePos, Span}; + +use std::str; + +#[derive(RustcDecodable, Debug, PartialEq, Eq)] +struct TestData { + spans: Vec, +} + +#[derive(RustcDecodable, Debug, PartialEq, Eq)] +struct SpanTestData { + pub byte_start: u32, + pub byte_end: u32, + pub line_start: u32, + pub column_start: u32, + pub line_end: u32, + pub column_end: u32, +} + +/// Test the span yields correct positions in JSON. +fn test_positions(code: &str, span: (u32, u32), expected_output: SpanTestData) { + + let expected_output = TestData { spans: vec![expected_output] }; + + with_default_globals(|| { + let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); + sm.new_source_file(Path::new("test.rs").to_owned().into(), code.to_owned()); + + let output = Arc::new(Mutex::new(Vec::new())); + let je = JsonEmitter::new( + Box::new(Shared { data: output.clone() }), + None, + sm, + true, + HumanReadableErrorType::Short(ColorConfig::Never), + false + ); + + let span = Span::with_root_ctxt(BytePos(span.0), BytePos(span.1)); + let handler = Handler::with_emitter(true, None, Box::new(je)); + handler.span_err(span, "foo"); + + let bytes = output.lock().unwrap(); + let actual_output = str::from_utf8(&bytes).unwrap(); + let actual_output : TestData = decode(actual_output).unwrap(); + + println!("expected output:\n------\n{:#?}------", expected_output); + println!("actual output:\n------\n{:#?}------", actual_output); + + assert!(expected_output == actual_output) + }) +} + +#[test] +fn empty() { + test_positions( + " ", + (0, 1), + SpanTestData { + byte_start: 0, + byte_end: 1, + line_start: 1, + column_start: 1, + line_end: 1, + column_end: 2, + } + ) +} + +#[test] +fn bom() { + test_positions( + "\u{feff} ", + (0, 1), + SpanTestData { + byte_start: 3, + byte_end: 4, + line_start: 1, + column_start: 1, + line_end: 1, + column_end: 2, + } + ) +} + +#[test] +fn lf_newlines() { + test_positions( + "\nmod foo;\nmod bar;\n", + (5, 12), + SpanTestData { + byte_start: 5, + byte_end: 12, + line_start: 2, + column_start: 5, + line_end: 3, + column_end: 3, + } + ) +} + +#[test] +fn crlf_newlines() { + test_positions( + "\r\nmod foo;\r\nmod bar;\r\n", + (5, 12), + SpanTestData { + byte_start: 6, + byte_end: 14, + line_start: 2, + column_start: 5, + line_end: 3, + column_end: 3, + } + ) +} + +#[test] +fn crlf_newlines_with_bom() { + test_positions( + "\u{feff}\r\nmod foo;\r\nmod bar;\r\n", + (5, 12), + SpanTestData { + byte_start: 9, + byte_end: 17, + line_start: 2, + column_start: 5, + line_end: 3, + column_end: 3, + } + ) +} + +#[test] +fn span_before_crlf() { + test_positions( + "foo\r\nbar", + (2, 3), + SpanTestData { + byte_start: 2, + byte_end: 3, + line_start: 1, + column_start: 3, + line_end: 1, + column_end: 4, + } + ) +} + +#[test] +fn span_on_crlf() { + test_positions( + "foo\r\nbar", + (3, 4), + SpanTestData { + byte_start: 3, + byte_end: 5, + line_start: 1, + column_start: 4, + line_end: 2, + column_end: 1, + } + ) +} + +#[test] +fn span_after_crlf() { + test_positions( + "foo\r\nbar", + (4, 5), + SpanTestData { + byte_start: 5, + byte_end: 6, + line_start: 2, + column_start: 1, + line_end: 2, + column_end: 2, + } + ) +} diff --git a/src/libsyntax/tests.rs b/src/libsyntax/tests.rs index f510ac9273d09..a95880d962004 100644 --- a/src/libsyntax/tests.rs +++ b/src/libsyntax/tests.rs @@ -110,8 +110,8 @@ struct SpanLabel { label: &'static str, } -struct Shared { - data: Arc>, +crate struct Shared { + pub data: Arc>, } impl Write for Shared { From dbdfa6ef1d30b3a521876edb95ef436d96760a9e Mon Sep 17 00:00:00 2001 From: Mikko Rantanen Date: Thu, 3 Oct 2019 03:56:16 +0300 Subject: [PATCH 04/17] Fix byte offsets after normalization --- src/librustc/ich/impls_syntax.rs | 19 +++++++++++++++ src/librustc_metadata/decoder.rs | 7 +++++- src/libsyntax/json.rs | 18 ++++++++++++-- src/libsyntax/source_map.rs | 6 +++++ src/libsyntax_pos/lib.rs | 41 ++++++++++++++++++++++++++++---- src/libsyntax_pos/tests.rs | 31 ++++++++++++++---------- 6 files changed, 102 insertions(+), 20 deletions(-) diff --git a/src/librustc/ich/impls_syntax.rs b/src/librustc/ich/impls_syntax.rs index 23a2f115e05e2..d2c715c439b6d 100644 --- a/src/librustc/ich/impls_syntax.rs +++ b/src/librustc/ich/impls_syntax.rs @@ -424,6 +424,7 @@ impl<'a> HashStable> for SourceFile { ref lines, ref multibyte_chars, ref non_narrow_chars, + ref normalized_pos, } = *self; (name_hash as u64).hash_stable(hcx, hasher); @@ -452,6 +453,12 @@ impl<'a> HashStable> for SourceFile { for &char_pos in non_narrow_chars.iter() { stable_non_narrow_char(char_pos, start_pos).hash_stable(hcx, hasher); } + + normalized_pos.len().hash_stable(hcx, hasher); + for &char_pos in normalized_pos.iter() { + stable_normalized_pos(char_pos, start_pos).hash_stable(hcx, hasher); + } + } } @@ -481,6 +488,18 @@ fn stable_non_narrow_char(swc: ::syntax_pos::NonNarrowChar, (pos.0 - source_file_start.0, width as u32) } +fn stable_normalized_pos(np: ::syntax_pos::NormalizedPos, + source_file_start: ::syntax_pos::BytePos) + -> (u32, u32) { + let ::syntax_pos::NormalizedPos { + pos, + diff + } = np; + + (pos.0 - source_file_start.0, diff as u32) +} + + impl<'tcx> HashStable> for feature_gate::Features { fn hash_stable(&self, hcx: &mut StableHashingContext<'tcx>, hasher: &mut StableHasher) { // Unfortunately we cannot exhaustively list fields here, since the diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 9fcc310154599..18e601b0b2d68 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -1330,6 +1330,7 @@ impl<'a, 'tcx> CrateMetadata { mut lines, mut multibyte_chars, mut non_narrow_chars, + mut normalized_pos, name_hash, .. } = source_file_to_import; @@ -1349,6 +1350,9 @@ impl<'a, 'tcx> CrateMetadata { for swc in &mut non_narrow_chars { *swc = *swc - start_pos; } + for np in &mut normalized_pos { + np.pos = np.pos - start_pos; + } let local_version = local_source_map.new_imported_source_file(name, name_was_remapped, @@ -1358,7 +1362,8 @@ impl<'a, 'tcx> CrateMetadata { source_length, lines, multibyte_chars, - non_narrow_chars); + non_narrow_chars, + normalized_pos); debug!("CrateMetaData::imported_source_files alloc \ source_file {:?} original (start_pos {:?} end_pos {:?}) \ translated (start_pos {:?} end_pos {:?})", diff --git a/src/libsyntax/json.rs b/src/libsyntax/json.rs index 041c8c2c8cd4a..6db482cd585ba 100644 --- a/src/libsyntax/json.rs +++ b/src/libsyntax/json.rs @@ -333,10 +333,24 @@ impl DiagnosticSpan { }) }); + let file_np = &start.file.normalized_pos; + let np_start_diff = match file_np.binary_search_by( + |np| np.pos.cmp(&span.lo())) { + Ok(i) => file_np[i].diff, + Err(i) if i == 0 => 0, + Err(i) => file_np[i-1].diff, + }; + let np_end_diff = match file_np.binary_search_by( + |np| np.pos.cmp(&span.hi())) { + Ok(i) => file_np[i].diff, + Err(i) if i == 0 => 0, + Err(i) => file_np[i-1].diff, + }; + DiagnosticSpan { file_name: start.file.name.to_string(), - byte_start: span.lo().0 - start.file.start_pos.0, - byte_end: span.hi().0 - start.file.start_pos.0, + byte_start: span.lo().0 - start.file.start_pos.0 + np_start_diff, + byte_end: span.hi().0 - start.file.start_pos.0 + np_end_diff, line_start: start.line, line_end: end.line, column_start: start.col.0 + 1, diff --git a/src/libsyntax/source_map.rs b/src/libsyntax/source_map.rs index 7d0d2392945e5..359b595716795 100644 --- a/src/libsyntax/source_map.rs +++ b/src/libsyntax/source_map.rs @@ -283,6 +283,7 @@ impl SourceMap { mut file_local_lines: Vec, mut file_local_multibyte_chars: Vec, mut file_local_non_narrow_chars: Vec, + mut file_local_normalized_pos: Vec, ) -> Lrc { let start_pos = self.next_start_pos(); @@ -301,6 +302,10 @@ impl SourceMap { *swc = *swc + start_pos; } + for nc in &mut file_local_normalized_pos { + nc.pos = nc.pos + start_pos; + } + let source_file = Lrc::new(SourceFile { name: filename, name_was_remapped, @@ -314,6 +319,7 @@ impl SourceMap { lines: file_local_lines, multibyte_chars: file_local_multibyte_chars, non_narrow_chars: file_local_non_narrow_chars, + normalized_pos: file_local_normalized_pos, name_hash, }); diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs index 6e83e1d3f8d4f..8516997a9c3df 100644 --- a/src/libsyntax_pos/lib.rs +++ b/src/libsyntax_pos/lib.rs @@ -849,6 +849,15 @@ impl Sub for NonNarrowChar { } } +/// Identifies an offset of a character that was normalized away from `SourceFile`. +#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Eq, PartialEq, Debug)] +pub struct NormalizedPos { + /// The absolute offset of the character in the `SourceMap`. + pub pos: BytePos, + /// The difference between original and normalized string at position. + pub diff: u32, +} + /// The state of the lazy external source loading mechanism of a `SourceFile`. #[derive(PartialEq, Eq, Clone)] pub enum ExternalSource { @@ -912,6 +921,8 @@ pub struct SourceFile { pub multibyte_chars: Vec, /// Width of characters that are not narrow in the source code. pub non_narrow_chars: Vec, + /// Locations of characters removed during normalization. + pub normalized_pos: Vec, /// A hash of the filename, used for speeding up hashing in incremental compilation. pub name_hash: u128, } @@ -978,6 +989,9 @@ impl Encodable for SourceFile { })?; s.emit_struct_field("name_hash", 8, |s| { self.name_hash.encode(s) + })?; + s.emit_struct_field("normalized_pos", 9, |s| { + self.normalized_pos.encode(s) }) }) } @@ -1028,6 +1042,8 @@ impl Decodable for SourceFile { d.read_struct_field("non_narrow_chars", 7, |d| Decodable::decode(d))?; let name_hash: u128 = d.read_struct_field("name_hash", 8, |d| Decodable::decode(d))?; + let normalized_pos: Vec = + d.read_struct_field("normalized_pos", 9, |d| Decodable::decode(d))?; Ok(SourceFile { name, name_was_remapped, @@ -1044,6 +1060,7 @@ impl Decodable for SourceFile { lines, multibyte_chars, non_narrow_chars, + normalized_pos, name_hash, }) }) @@ -1062,8 +1079,7 @@ impl SourceFile { unmapped_path: FileName, mut src: String, start_pos: BytePos) -> Result { - remove_bom(&mut src); - normalize_newlines(&mut src); + let normalized_pos = normalize_src(&mut src); let src_hash = { let mut hasher: StableHasher = StableHasher::new(); @@ -1096,6 +1112,7 @@ impl SourceFile { lines, multibyte_chars, non_narrow_chars, + normalized_pos, name_hash, }) } @@ -1224,10 +1241,19 @@ impl SourceFile { } } +/// Normalizes the source code and records the normalizations. +fn normalize_src(src: &mut String) -> Vec { + let mut normalized_pos = vec![]; + remove_bom(src, &mut normalized_pos); + normalize_newlines(src, &mut normalized_pos); + normalized_pos +} + /// Removes UTF-8 BOM, if any. -fn remove_bom(src: &mut String) { +fn remove_bom(src: &mut String, normalized_pos: &mut Vec) { if src.starts_with("\u{feff}") { src.drain(..3); + normalized_pos.push(NormalizedPos { pos: BytePos(0), diff: 3 }); } } @@ -1235,7 +1261,7 @@ fn remove_bom(src: &mut String) { /// Replaces `\r\n` with `\n` in-place in `src`. /// /// Returns error if there's a lone `\r` in the string -fn normalize_newlines(src: &mut String) { +fn normalize_newlines(src: &mut String, normalized_pos: &mut Vec) { if !src.as_bytes().contains(&b'\r') { return; } @@ -1248,6 +1274,8 @@ fn normalize_newlines(src: &mut String) { let mut buf = std::mem::replace(src, String::new()).into_bytes(); let mut gap_len = 0; let mut tail = buf.as_mut_slice(); + let mut cursor = 0; + let original_gap = normalized_pos.last().map_or(0, |l| l.diff); loop { let idx = match find_crlf(&tail[gap_len..]) { None => tail.len(), @@ -1258,7 +1286,12 @@ fn normalize_newlines(src: &mut String) { if tail.len() == gap_len { break; } + cursor += idx - gap_len; gap_len += 1; + normalized_pos.push(NormalizedPos { + pos: BytePos::from_usize(cursor + 1), + diff: original_gap + gap_len as u32, + }); } // Account for removed `\r`. diff --git a/src/libsyntax_pos/tests.rs b/src/libsyntax_pos/tests.rs index 6bd6016020a27..87cc3505e389b 100644 --- a/src/libsyntax_pos/tests.rs +++ b/src/libsyntax_pos/tests.rs @@ -19,20 +19,25 @@ fn test_lookup_line() { #[test] fn test_normalize_newlines() { - fn check(before: &str, after: &str) { + fn check(before: &str, after: &str, expected_positions: &[u32]) { let mut actual = before.to_string(); - normalize_newlines(&mut actual); + let mut actual_positions = vec![]; + normalize_newlines(&mut actual, &mut actual_positions); + let actual_positions : Vec<_> = actual_positions + .into_iter() + .map(|nc| nc.pos.0).collect(); assert_eq!(actual.as_str(), after); + assert_eq!(actual_positions, expected_positions); } - check("", ""); - check("\n", "\n"); - check("\r", "\r"); - check("\r\r", "\r\r"); - check("\r\n", "\n"); - check("hello world", "hello world"); - check("hello\nworld", "hello\nworld"); - check("hello\r\nworld", "hello\nworld"); - check("\r\nhello\r\nworld\r\n", "\nhello\nworld\n"); - check("\r\r\n", "\r\n"); - check("hello\rworld", "hello\rworld"); + check("", "", &[]); + check("\n", "\n", &[]); + check("\r", "\r", &[]); + check("\r\r", "\r\r", &[]); + check("\r\n", "\n", &[1]); + check("hello world", "hello world", &[]); + check("hello\nworld", "hello\nworld", &[]); + check("hello\r\nworld", "hello\nworld", &[6]); + check("\r\nhello\r\nworld\r\n", "\nhello\nworld\n", &[1, 7, 13]); + check("\r\r\n", "\r\n", &[2]); + check("hello\rworld", "hello\rworld", &[]); } From ce57648c17c57953bd595a8b9f0036390106bb27 Mon Sep 17 00:00:00 2001 From: Mikko Rantanen Date: Fri, 4 Oct 2019 20:54:35 +0300 Subject: [PATCH 05/17] Move relative pos calculation to SourceFile --- src/libsyntax/json.rs | 18 ++---------------- src/libsyntax_pos/lib.rs | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/libsyntax/json.rs b/src/libsyntax/json.rs index 6db482cd585ba..aa0b41171744e 100644 --- a/src/libsyntax/json.rs +++ b/src/libsyntax/json.rs @@ -333,24 +333,10 @@ impl DiagnosticSpan { }) }); - let file_np = &start.file.normalized_pos; - let np_start_diff = match file_np.binary_search_by( - |np| np.pos.cmp(&span.lo())) { - Ok(i) => file_np[i].diff, - Err(i) if i == 0 => 0, - Err(i) => file_np[i-1].diff, - }; - let np_end_diff = match file_np.binary_search_by( - |np| np.pos.cmp(&span.hi())) { - Ok(i) => file_np[i].diff, - Err(i) if i == 0 => 0, - Err(i) => file_np[i-1].diff, - }; - DiagnosticSpan { file_name: start.file.name.to_string(), - byte_start: span.lo().0 - start.file.start_pos.0 + np_start_diff, - byte_end: span.hi().0 - start.file.start_pos.0 + np_end_diff, + byte_start: start.file.original_relative_byte_pos(span.lo()).0, + byte_end: start.file.original_relative_byte_pos(span.hi()).0, line_start: start.line, line_end: end.line, column_start: start.col.0 + 1, diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs index 8516997a9c3df..4c7855f524222 100644 --- a/src/libsyntax_pos/lib.rs +++ b/src/libsyntax_pos/lib.rs @@ -1239,6 +1239,23 @@ impl SourceFile { pub fn contains(&self, byte_pos: BytePos) -> bool { byte_pos >= self.start_pos && byte_pos <= self.end_pos } + + /// Calculates the original byte position relative to the start of the file + /// based on the given byte position. + pub fn original_relative_byte_pos(&self, pos: BytePos) -> BytePos { + + // Diff before any records is 0. Otherwise use the previously recorded + // diff as that applies to the following characters until a new diff + // is recorded. + let diff = match self.normalized_pos.binary_search_by( + |np| np.pos.cmp(&pos)) { + Ok(i) => self.normalized_pos[i].diff, + Err(i) if i == 0 => 0, + Err(i) => self.normalized_pos[i-1].diff, + }; + + BytePos::from_u32(pos.0 - self.start_pos.0 + diff) + } } /// Normalizes the source code and records the normalizations. From e492ca8b891a0f02fbcd307799b9e5d4ab803268 Mon Sep 17 00:00:00 2001 From: Mikko Rantanen Date: Fri, 4 Oct 2019 20:55:24 +0300 Subject: [PATCH 06/17] Add integration test --- src/test/ui/.gitattributes | 1 + src/test/ui/json-bom-plus-crlf.rs | 20 +++++++ src/test/ui/json-bom-plus-crlf.stderr | 86 +++++++++++++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 src/test/ui/json-bom-plus-crlf.rs create mode 100644 src/test/ui/json-bom-plus-crlf.stderr diff --git a/src/test/ui/.gitattributes b/src/test/ui/.gitattributes index 489dc8ad1118c..7474e4dd6387c 100644 --- a/src/test/ui/.gitattributes +++ b/src/test/ui/.gitattributes @@ -1,3 +1,4 @@ lexer-crlf-line-endings-string-literal-doc-comment.rs -text +json-bom-plus-crlf.rs -text trailing-carriage-return-in-string.rs -text *.bin -text diff --git a/src/test/ui/json-bom-plus-crlf.rs b/src/test/ui/json-bom-plus-crlf.rs new file mode 100644 index 0000000000000..264f8a2aded04 --- /dev/null +++ b/src/test/ui/json-bom-plus-crlf.rs @@ -0,0 +1,20 @@ +// build-fail +// compile-flags: --json=diagnostic-short --error-format=json +// ignore-tidy-cr + +// N.B., this file needs CRLF line endings. The .gitattributes file in +// this directory should enforce it. + +fn main() { + + let s : String = 1; // Error in the middle of line. + + let s : String = 1 + ; // Error before the newline. + + let s : String = +1; // Error after the newline. + + let s : String = ( + ); // Error spanning the newline. +} diff --git a/src/test/ui/json-bom-plus-crlf.stderr b/src/test/ui/json-bom-plus-crlf.stderr new file mode 100644 index 0000000000000..43ca081ec3ca5 --- /dev/null +++ b/src/test/ui/json-bom-plus-crlf.stderr @@ -0,0 +1,86 @@ +{"message":"mismatched types","code":{"code":"E0308","explanation":" +This error occurs when the compiler was unable to infer the concrete type of a +variable. It can occur for several cases, the most common of which is a +mismatch in the expected type that the compiler inferred for a variable's +initializing expression, and the actual type explicitly assigned to the +variable. + +For example: + +```compile_fail,E0308 +let x: i32 = \"I am not a number!\"; +// ~~~ ~~~~~~~~~~~~~~~~~~~~ +// | | +// | initializing expression; +// | compiler infers type `&str` +// | +// type `i32` assigned to variable `x` +``` +"},"level":"error","spans":[{"file_name":"$DIR/json-bom-plus-crlf.rs","byte_start":250,"byte_end":251,"line_start":10,"line_end":10,"column_start":22,"column_end":23,"is_primary":true,"text":[{"text":" let s : String = 1; // Error in the middle of line.","highlight_start":22,"highlight_end":23}],"label":"expected struct `std::string::String`, found integer","suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"expected type `std::string::String` + found type `{integer}`","code":null,"level":"note","spans":[],"children":[],"rendered":null},{"message":"try using a conversion method","code":null,"level":"help","spans":[{"file_name":"$DIR/json-bom-plus-crlf.rs","byte_start":250,"byte_end":251,"line_start":10,"line_end":10,"column_start":22,"column_end":23,"is_primary":true,"text":[{"text":" let s : String = 1; // Error in the middle of line.","highlight_start":22,"highlight_end":23}],"label":null,"suggested_replacement":"1.to_string()","suggestion_applicability":"MaybeIncorrect","expansion":null}],"children":[],"rendered":null}],"rendered":"$DIR/json-bom-plus-crlf.rs:10:22: error[E0308]: mismatched types +"} +{"message":"mismatched types","code":{"code":"E0308","explanation":" +This error occurs when the compiler was unable to infer the concrete type of a +variable. It can occur for several cases, the most common of which is a +mismatch in the expected type that the compiler inferred for a variable's +initializing expression, and the actual type explicitly assigned to the +variable. + +For example: + +```compile_fail,E0308 +let x: i32 = \"I am not a number!\"; +// ~~~ ~~~~~~~~~~~~~~~~~~~~ +// | | +// | initializing expression; +// | compiler infers type `&str` +// | +// type `i32` assigned to variable `x` +``` +"},"level":"error","spans":[{"file_name":"$DIR/json-bom-plus-crlf.rs","byte_start":310,"byte_end":311,"line_start":12,"line_end":12,"column_start":22,"column_end":23,"is_primary":true,"text":[{"text":" let s : String = 1","highlight_start":22,"highlight_end":23}],"label":"expected struct `std::string::String`, found integer","suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"expected type `std::string::String` + found type `{integer}`","code":null,"level":"note","spans":[],"children":[],"rendered":null},{"message":"try using a conversion method","code":null,"level":"help","spans":[{"file_name":"$DIR/json-bom-plus-crlf.rs","byte_start":310,"byte_end":311,"line_start":12,"line_end":12,"column_start":22,"column_end":23,"is_primary":true,"text":[{"text":" let s : String = 1","highlight_start":22,"highlight_end":23}],"label":null,"suggested_replacement":"1.to_string()","suggestion_applicability":"MaybeIncorrect","expansion":null}],"children":[],"rendered":null}],"rendered":"$DIR/json-bom-plus-crlf.rs:12:22: error[E0308]: mismatched types +"} +{"message":"mismatched types","code":{"code":"E0308","explanation":" +This error occurs when the compiler was unable to infer the concrete type of a +variable. It can occur for several cases, the most common of which is a +mismatch in the expected type that the compiler inferred for a variable's +initializing expression, and the actual type explicitly assigned to the +variable. + +For example: + +```compile_fail,E0308 +let x: i32 = \"I am not a number!\"; +// ~~~ ~~~~~~~~~~~~~~~~~~~~ +// | | +// | initializing expression; +// | compiler infers type `&str` +// | +// type `i32` assigned to variable `x` +``` +"},"level":"error","spans":[{"file_name":"$DIR/json-bom-plus-crlf.rs","byte_start":374,"byte_end":375,"line_start":16,"line_end":16,"column_start":1,"column_end":2,"is_primary":true,"text":[{"text":"1; // Error after the newline.","highlight_start":1,"highlight_end":2}],"label":"expected struct `std::string::String`, found integer","suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"expected type `std::string::String` + found type `{integer}`","code":null,"level":"note","spans":[],"children":[],"rendered":null},{"message":"try using a conversion method","code":null,"level":"help","spans":[{"file_name":"$DIR/json-bom-plus-crlf.rs","byte_start":374,"byte_end":375,"line_start":16,"line_end":16,"column_start":1,"column_end":2,"is_primary":true,"text":[{"text":"1; // Error after the newline.","highlight_start":1,"highlight_end":2}],"label":null,"suggested_replacement":"1.to_string()","suggestion_applicability":"MaybeIncorrect","expansion":null}],"children":[],"rendered":null}],"rendered":"$DIR/json-bom-plus-crlf.rs:16:1: error[E0308]: mismatched types +"} +{"message":"mismatched types","code":{"code":"E0308","explanation":" +This error occurs when the compiler was unable to infer the concrete type of a +variable. It can occur for several cases, the most common of which is a +mismatch in the expected type that the compiler inferred for a variable's +initializing expression, and the actual type explicitly assigned to the +variable. + +For example: + +```compile_fail,E0308 +let x: i32 = \"I am not a number!\"; +// ~~~ ~~~~~~~~~~~~~~~~~~~~ +// | | +// | initializing expression; +// | compiler infers type `&str` +// | +// type `i32` assigned to variable `x` +``` +"},"level":"error","spans":[{"file_name":"$DIR/json-bom-plus-crlf.rs","byte_start":430,"byte_end":438,"line_start":18,"line_end":19,"column_start":22,"column_end":6,"is_primary":true,"text":[{"text":" let s : String = (","highlight_start":22,"highlight_end":23},{"text":" ); // Error spanning the newline.","highlight_start":1,"highlight_end":6}],"label":"expected struct `std::string::String`, found ()","suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"expected type `std::string::String` + found type `()`","code":null,"level":"note","spans":[],"children":[],"rendered":null}],"rendered":"$DIR/json-bom-plus-crlf.rs:18:22: error[E0308]: mismatched types +"} +{"message":"aborting due to 4 previous errors","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to 4 previous errors +"} From 4414068cc49803b0dacba133b7788b8b95b0b473 Mon Sep 17 00:00:00 2001 From: AnthonyMikh Date: Sat, 5 Oct 2019 03:08:05 +0300 Subject: [PATCH 07/17] Correctly estimate required space for string `.len()` returns length in bytes so it overestimates the required space --- src/librustc_errors/styled_buffer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_errors/styled_buffer.rs b/src/librustc_errors/styled_buffer.rs index 6e03618d2b0b5..b12ab9e457602 100644 --- a/src/librustc_errors/styled_buffer.rs +++ b/src/librustc_errors/styled_buffer.rs @@ -111,7 +111,7 @@ impl StyledBuffer { pub fn prepend(&mut self, line: usize, string: &str, style: Style) { self.ensure_lines(line); - let string_len = string.len(); + let string_len = string.chars().count(); // Push the old content over to make room for new content for _ in 0..string_len { From b27089d6a373043105ca807d69614854f2d2ffcb Mon Sep 17 00:00:00 2001 From: Mikko Rantanen Date: Sat, 5 Oct 2019 20:36:15 +0300 Subject: [PATCH 08/17] Fix format --- src/libsyntax/json/tests.rs | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/libsyntax/json/tests.rs b/src/libsyntax/json/tests.rs index 5103cff6f9422..0daff6bdcb80d 100644 --- a/src/libsyntax/json/tests.rs +++ b/src/libsyntax/json/tests.rs @@ -1,11 +1,11 @@ use super::*; -use crate::tests::{Shared}; use crate::json::JsonEmitter; -use crate::source_map::{SourceMap, FilePathMapping}; +use crate::source_map::{FilePathMapping, SourceMap}; +use crate::tests::Shared; use crate::with_default_globals; -use errors::emitter::{HumanReadableErrorType, ColorConfig}; +use errors::emitter::{ColorConfig, HumanReadableErrorType}; use errors::Handler; use rustc_serialize::json::decode; use syntax_pos::{BytePos, Span}; @@ -29,7 +29,6 @@ struct SpanTestData { /// Test the span yields correct positions in JSON. fn test_positions(code: &str, span: (u32, u32), expected_output: SpanTestData) { - let expected_output = TestData { spans: vec![expected_output] }; with_default_globals(|| { @@ -43,7 +42,7 @@ fn test_positions(code: &str, span: (u32, u32), expected_output: SpanTestData) { sm, true, HumanReadableErrorType::Short(ColorConfig::Never), - false + false, ); let span = Span::with_root_ctxt(BytePos(span.0), BytePos(span.1)); @@ -52,7 +51,7 @@ fn test_positions(code: &str, span: (u32, u32), expected_output: SpanTestData) { let bytes = output.lock().unwrap(); let actual_output = str::from_utf8(&bytes).unwrap(); - let actual_output : TestData = decode(actual_output).unwrap(); + let actual_output: TestData = decode(actual_output).unwrap(); println!("expected output:\n------\n{:#?}------", expected_output); println!("actual output:\n------\n{:#?}------", actual_output); @@ -73,7 +72,7 @@ fn empty() { column_start: 1, line_end: 1, column_end: 2, - } + }, ) } @@ -89,7 +88,7 @@ fn bom() { column_start: 1, line_end: 1, column_end: 2, - } + }, ) } @@ -105,7 +104,7 @@ fn lf_newlines() { column_start: 5, line_end: 3, column_end: 3, - } + }, ) } @@ -121,7 +120,7 @@ fn crlf_newlines() { column_start: 5, line_end: 3, column_end: 3, - } + }, ) } @@ -137,7 +136,7 @@ fn crlf_newlines_with_bom() { column_start: 5, line_end: 3, column_end: 3, - } + }, ) } @@ -153,7 +152,7 @@ fn span_before_crlf() { column_start: 3, line_end: 1, column_end: 4, - } + }, ) } @@ -169,7 +168,7 @@ fn span_on_crlf() { column_start: 4, line_end: 2, column_end: 1, - } + }, ) } @@ -185,6 +184,6 @@ fn span_after_crlf() { column_start: 1, line_end: 2, column_end: 2, - } + }, ) } From dfc008f28c0a26bed3b94a984e5ed6abe52dc23f Mon Sep 17 00:00:00 2001 From: Mikko Rantanen Date: Sat, 5 Oct 2019 20:38:29 +0300 Subject: [PATCH 09/17] Remove useless cast --- src/librustc/ich/impls_syntax.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/ich/impls_syntax.rs b/src/librustc/ich/impls_syntax.rs index d2c715c439b6d..93a3c1f60324e 100644 --- a/src/librustc/ich/impls_syntax.rs +++ b/src/librustc/ich/impls_syntax.rs @@ -496,7 +496,7 @@ fn stable_normalized_pos(np: ::syntax_pos::NormalizedPos, diff } = np; - (pos.0 - source_file_start.0, diff as u32) + (pos.0 - source_file_start.0, diff) } From 5497ba1690a1cbf65db8cd1a018bf7fc8b82a73a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 5 Oct 2019 16:11:36 -0700 Subject: [PATCH 10/17] When suggesting assoc function with type params, include turbofish --- src/librustc_typeck/check/method/suggest.rs | 27 +++++++++++++------ .../suggest-assoc-fn-call-with-turbofish.rs | 11 ++++++++ ...uggest-assoc-fn-call-with-turbofish.stderr | 22 +++++++++++++++ 3 files changed, 52 insertions(+), 8 deletions(-) create mode 100644 src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish.rs create mode 100644 src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish.stderr diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs index 2b34c24b266d0..7ce855c73c7c0 100644 --- a/src/librustc_typeck/check/method/suggest.rs +++ b/src/librustc_typeck/check/method/suggest.rs @@ -462,15 +462,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } if static_sources.len() == 1 { if let SelfSource::MethodCall(expr) = source { - err.span_suggestion(expr.span.to(span), - "use associated function syntax instead", - format!("{}::{}", - self.ty_to_string(actual), - item_name), - Applicability::MachineApplicable); + err.span_suggestion( + expr.span.to(span), + "use associated function syntax instead", + format!("{}::{}", self.ty_to_value_string(actual), item_name), + Applicability::MachineApplicable, + ); } else { - err.help(&format!("try with `{}::{}`", - self.ty_to_string(actual), item_name)); + err.help(&format!( + "try with `{}::{}`", + self.ty_to_value_string(actual), + item_name, + )); } report_candidates(span, &mut err, static_sources); @@ -579,6 +582,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { None } + /// Print out the type for use in value namespace. + fn ty_to_value_string(&self, ty: Ty<'tcx>) -> String { + match ty.kind { + ty::Adt(def, substs) => format!("{}", ty::Instance::new(def.did, substs)), + _ => self.ty_to_string(ty), + } + } + fn suggest_use_candidates(&self, err: &mut DiagnosticBuilder<'_>, mut msg: String, diff --git a/src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish.rs b/src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish.rs new file mode 100644 index 0000000000000..ef4b38de94732 --- /dev/null +++ b/src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish.rs @@ -0,0 +1,11 @@ +struct GenericAssocMethod(T); + +impl GenericAssocMethod { + fn default_hello() {} +} + +fn main() { + let x = GenericAssocMethod(33i32); + x.default_hello(); + //~^ ERROR no method named `default_hello` found for type `GenericAssocMethod` +} diff --git a/src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish.stderr b/src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish.stderr new file mode 100644 index 0000000000000..8cfa7de08bb38 --- /dev/null +++ b/src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish.stderr @@ -0,0 +1,22 @@ +error[E0599]: no method named `default_hello` found for type `GenericAssocMethod` in the current scope + --> $DIR/suggest-assoc-fn-call-with-turbofish.rs:9:7 + | +LL | struct GenericAssocMethod(T); + | -------------------------------- method `default_hello` not found for this +... +LL | x.default_hello(); + | --^^^^^^^^^^^^^ + | | | + | | this is an associated function, not a method + | help: use associated function syntax instead: `GenericAssocMethod::::default_hello` + | + = note: found the following associated functions; to be used as methods, functions must have a `self` parameter +note: the candidate is defined in an impl for the type `GenericAssocMethod<_>` + --> $DIR/suggest-assoc-fn-call-with-turbofish.rs:4:5 + | +LL | fn default_hello() {} + | ^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0599`. From 3166ce81ec664a0982937050fcedeb3d8538f686 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 5 Oct 2019 16:57:14 -0700 Subject: [PATCH 11/17] Account for derefs when suggesting assoc function --- src/librustc_typeck/check/method/suggest.rs | 21 +++++++++++++++++-- src/test/ui/issues/issue-3707.stderr | 2 +- ...oc-fn-call-with-turbofish-through-deref.rs | 13 ++++++++++++ ...n-call-with-turbofish-through-deref.stderr | 19 +++++++++++++++++ 4 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish-through-deref.rs create mode 100644 src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish-through-deref.stderr diff --git a/src/librustc_typeck/check/method/suggest.rs b/src/librustc_typeck/check/method/suggest.rs index 7ce855c73c7c0..621124541d11d 100644 --- a/src/librustc_typeck/check/method/suggest.rs +++ b/src/librustc_typeck/check/method/suggest.rs @@ -461,17 +461,34 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err.span_label(span, "this is an associated function, not a method"); } if static_sources.len() == 1 { + let ty_str = if let Some(CandidateSource::ImplSource( + impl_did, + )) = static_sources.get(0) { + // When the "method" is resolved through dereferencing, we really want the + // original type that has the associated function for accurate suggestions. + // (#61411) + let ty = self.impl_self_ty(span, *impl_did).ty; + match (&ty.peel_refs().kind, &actual.peel_refs().kind) { + (ty::Adt(def, _), ty::Adt(def_actual, _)) if def == def_actual => { + // Use `actual` as it will have more `substs` filled in. + self.ty_to_value_string(actual.peel_refs()) + } + _ => self.ty_to_value_string(ty.peel_refs()), + } + } else { + self.ty_to_value_string(actual.peel_refs()) + }; if let SelfSource::MethodCall(expr) = source { err.span_suggestion( expr.span.to(span), "use associated function syntax instead", - format!("{}::{}", self.ty_to_value_string(actual), item_name), + format!("{}::{}", ty_str, item_name), Applicability::MachineApplicable, ); } else { err.help(&format!( "try with `{}::{}`", - self.ty_to_value_string(actual), + ty_str, item_name, )); } diff --git a/src/test/ui/issues/issue-3707.stderr b/src/test/ui/issues/issue-3707.stderr index 05c8ce4c3f11e..b98bc572a397c 100644 --- a/src/test/ui/issues/issue-3707.stderr +++ b/src/test/ui/issues/issue-3707.stderr @@ -5,7 +5,7 @@ LL | self.boom(); | -----^^^^ | | | | | this is an associated function, not a method - | help: use associated function syntax instead: `&Obj::boom` + | help: use associated function syntax instead: `Obj::boom` | = note: found the following associated functions; to be used as methods, functions must have a `self` parameter note: the candidate is defined in an impl for the type `Obj` diff --git a/src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish-through-deref.rs b/src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish-through-deref.rs new file mode 100644 index 0000000000000..5480adb31015a --- /dev/null +++ b/src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish-through-deref.rs @@ -0,0 +1,13 @@ +use std::cell::RefCell; + +struct HasAssocMethod; + +impl HasAssocMethod { + fn hello() {} +} +fn main() { + let shared_state = RefCell::new(HasAssocMethod); + let state = shared_state.borrow_mut(); + state.hello(); + //~^ ERROR no method named `hello` found for type `std::cell::RefMut<'_, HasAssocMethod>` +} diff --git a/src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish-through-deref.stderr b/src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish-through-deref.stderr new file mode 100644 index 0000000000000..a1c0126146e73 --- /dev/null +++ b/src/test/ui/suggestions/suggest-assoc-fn-call-with-turbofish-through-deref.stderr @@ -0,0 +1,19 @@ +error[E0599]: no method named `hello` found for type `std::cell::RefMut<'_, HasAssocMethod>` in the current scope + --> $DIR/suggest-assoc-fn-call-with-turbofish-through-deref.rs:11:11 + | +LL | state.hello(); + | ------^^^^^ + | | | + | | this is an associated function, not a method + | help: use associated function syntax instead: `HasAssocMethod::hello` + | + = note: found the following associated functions; to be used as methods, functions must have a `self` parameter +note: the candidate is defined in an impl for the type `HasAssocMethod` + --> $DIR/suggest-assoc-fn-call-with-turbofish-through-deref.rs:6:5 + | +LL | fn hello() {} + | ^^^^^^^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0599`. From bbf262d9fcf6e54a26268214ed2c967d4218d3a4 Mon Sep 17 00:00:00 2001 From: Mikko Rantanen Date: Sun, 6 Oct 2019 03:17:14 +0300 Subject: [PATCH 12/17] Tweak test output on failure --- src/libsyntax/json/tests.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libsyntax/json/tests.rs b/src/libsyntax/json/tests.rs index 0daff6bdcb80d..eb0d9ef3947c8 100644 --- a/src/libsyntax/json/tests.rs +++ b/src/libsyntax/json/tests.rs @@ -53,10 +53,7 @@ fn test_positions(code: &str, span: (u32, u32), expected_output: SpanTestData) { let actual_output = str::from_utf8(&bytes).unwrap(); let actual_output: TestData = decode(actual_output).unwrap(); - println!("expected output:\n------\n{:#?}------", expected_output); - println!("actual output:\n------\n{:#?}------", actual_output); - - assert!(expected_output == actual_output) + assert_eq!(expected_output, actual_output) }) } From 4c32d475a8bd2c68f82120af9182c41730a81a82 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sun, 6 Oct 2019 08:38:23 -0400 Subject: [PATCH 13/17] Remove loaded_from_cache map from DepGraph It's now unused, even with -Zquery-dep-graph --- src/librustc/dep_graph/graph.rs | 23 ----------------------- src/librustc/ty/query/plumbing.rs | 8 -------- 2 files changed, 31 deletions(-) diff --git a/src/librustc/dep_graph/graph.rs b/src/librustc/dep_graph/graph.rs index 0c56fc7914b4c..08c456547cec3 100644 --- a/src/librustc/dep_graph/graph.rs +++ b/src/librustc/dep_graph/graph.rs @@ -75,9 +75,6 @@ struct DepGraphData { previous_work_products: FxHashMap, dep_node_debug: Lock>, - - // Used for testing, only populated when -Zquery-dep-graph is specified. - loaded_from_cache: Lock>, } pub fn hash_result(hcx: &mut StableHashingContext<'_>, result: &R) -> Option @@ -104,7 +101,6 @@ impl DepGraph { emitting_diagnostics_cond_var: Condvar::new(), previous: prev_graph, colors: DepNodeColorMap::new(prev_graph_node_count), - loaded_from_cache: Default::default(), })), } } @@ -874,25 +870,6 @@ impl DepGraph { } } } - - pub fn mark_loaded_from_cache(&self, dep_node_index: DepNodeIndex, state: bool) { - debug!("mark_loaded_from_cache({:?}, {})", - self.data.as_ref().unwrap().current.borrow().data[dep_node_index].node, - state); - - self.data - .as_ref() - .unwrap() - .loaded_from_cache - .borrow_mut() - .insert(dep_node_index, state); - } - - pub fn was_loaded_from_cache(&self, dep_node: &DepNode) -> Option { - let data = self.data.as_ref().unwrap(); - let dep_node_index = data.current.borrow().node_to_node_index[dep_node]; - data.loaded_from_cache.borrow().get(&dep_node_index).cloned() - } } /// A "work product" is an intermediate result that we save into the diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 955f1447c55b6..7cab8fe60ee6a 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -489,10 +489,6 @@ impl<'tcx> TyCtxt<'tcx> { self.incremental_verify_ich::(&result, dep_node, dep_node_index); } - if unlikely!(self.sess.opts.debugging_opts.query_dep_graph) { - self.dep_graph.mark_loaded_from_cache(dep_node_index, true); - } - result } @@ -570,10 +566,6 @@ impl<'tcx> TyCtxt<'tcx> { drop(prof_timer); profq_msg!(self, ProfileQueriesMsg::ProviderEnd); - if unlikely!(self.sess.opts.debugging_opts.query_dep_graph) { - self.dep_graph.mark_loaded_from_cache(dep_node_index, false); - } - if unlikely!(!diagnostics.is_empty()) { if dep_node.kind != crate::dep_graph::DepKind::Null { self.queries.on_disk_cache From 90c456e7320d3b4c7c25f4fe16753070b6b68223 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 2 Oct 2019 14:32:26 +1000 Subject: [PATCH 14/17] Remove `force!`. --- src/librustc/ty/query/plumbing.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 955f1447c55b6..8958c10479d0b 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -1218,10 +1218,6 @@ pub fn force_from_dep_node(tcx: TyCtxt<'_>, dep_node: &DepNode) -> bool { } }; - macro_rules! force { - ($query:ident, $key:expr) => { force_ex!(tcx, $query, $key) } - }; - rustc_dep_node_force!([dep_node, tcx] // These are inputs that are expected to be pre-allocated and that // should therefore always be red or green already. @@ -1240,7 +1236,7 @@ pub fn force_from_dep_node(tcx: TyCtxt<'_>, dep_node: &DepNode) -> bool { bug!("force_from_dep_node: encountered {:?}", dep_node) } - DepKind::Analysis => { force!(analysis, krate!()); } + DepKind::Analysis => { force_ex!(tcx, analysis, krate!()); } ); true From 8f707c3a98367c43455797e781793baef4a3579b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 2 Oct 2019 14:33:30 +1000 Subject: [PATCH 15/17] Remove `krate!`. --- src/librustc/ty/query/plumbing.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 8958c10479d0b..e0ebb071ed2f2 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -1202,10 +1202,6 @@ pub fn force_from_dep_node(tcx: TyCtxt<'_>, dep_node: &DepNode) -> bool { } }; - macro_rules! krate { - () => { (def_id!()).krate } - }; - macro_rules! force_ex { ($tcx:expr, $query:ident, $key:expr) => { { @@ -1236,7 +1232,7 @@ pub fn force_from_dep_node(tcx: TyCtxt<'_>, dep_node: &DepNode) -> bool { bug!("force_from_dep_node: encountered {:?}", dep_node) } - DepKind::Analysis => { force_ex!(tcx, analysis, krate!()); } + DepKind::Analysis => { force_ex!(tcx, analysis, def_id!().krate); } ); true From 1183d60cd5427a8369fb406d8bbac7fcc7a21fc1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 2 Oct 2019 14:39:36 +1000 Subject: [PATCH 16/17] Remove `def_id!`. --- src/librustc/ty/query/plumbing.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index e0ebb071ed2f2..2590f96f855a8 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -1191,17 +1191,6 @@ pub fn force_from_dep_node(tcx: TyCtxt<'_>, dep_node: &DepNode) -> bool { return false } - macro_rules! def_id { - () => { - if let Some(def_id) = dep_node.extract_def_id(tcx) { - def_id - } else { - // Return from the whole function. - return false - } - } - }; - macro_rules! force_ex { ($tcx:expr, $query:ident, $key:expr) => { { @@ -1232,7 +1221,15 @@ pub fn force_from_dep_node(tcx: TyCtxt<'_>, dep_node: &DepNode) -> bool { bug!("force_from_dep_node: encountered {:?}", dep_node) } - DepKind::Analysis => { force_ex!(tcx, analysis, def_id!().krate); } + DepKind::Analysis => { + let def_id = if let Some(def_id) = dep_node.extract_def_id(tcx) { + def_id + } else { + // Return from the whole function. + return false + }; + force_ex!(tcx, analysis, def_id.krate); + } ); true From 9267d9fe5b1f99f98da83a90e84de706cf8cc150 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 7 Oct 2019 11:59:28 +1100 Subject: [PATCH 17/17] Remove `force_ex!`. --- src/librustc/ty/query/plumbing.rs | 18 +++++------------- src/librustc_macros/src/query.rs | 6 +++++- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index 2590f96f855a8..2a51e01ac848d 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -1191,18 +1191,6 @@ pub fn force_from_dep_node(tcx: TyCtxt<'_>, dep_node: &DepNode) -> bool { return false } - macro_rules! force_ex { - ($tcx:expr, $query:ident, $key:expr) => { - { - $tcx.force_query::>( - $key, - DUMMY_SP, - *dep_node - ); - } - } - }; - rustc_dep_node_force!([dep_node, tcx] // These are inputs that are expected to be pre-allocated and that // should therefore always be red or green already. @@ -1228,7 +1216,11 @@ pub fn force_from_dep_node(tcx: TyCtxt<'_>, dep_node: &DepNode) -> bool { // Return from the whole function. return false }; - force_ex!(tcx, analysis, def_id.krate); + tcx.force_query::>( + def_id.krate, + DUMMY_SP, + *dep_node + ); } ); diff --git a/src/librustc_macros/src/query.rs b/src/librustc_macros/src/query.rs index 9a68dd0f5e3ce..139e1b554cf90 100644 --- a/src/librustc_macros/src/query.rs +++ b/src/librustc_macros/src/query.rs @@ -495,7 +495,11 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream { dep_node_force_stream.extend(quote! { DepKind::#name => { if let Some(key) = RecoverKey::recover($tcx, $dep_node) { - force_ex!($tcx, #name, key); + $tcx.force_query::>( + key, + DUMMY_SP, + *$dep_node + ); } else { return false; }