Skip to content

Emit used rustc invocation in the save-analysis file #54356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2373,6 +2373,7 @@ dependencies = [
"rls-span 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc 0.0.0",
"rustc-serialize 0.3.24 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc_codegen_utils 0.0.0",
"rustc_data_structures 0.0.0",
"rustc_target 0.0.0",
"rustc_typeck 0.0.0",
Expand Down
12 changes: 2 additions & 10 deletions src/librustc_codegen_llvm/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ use std::str;
use syntax::attr;

pub use rustc_codegen_utils::link::{find_crate_name, filename_for_input, default_output_for_target,
invalid_output_for_target, out_filename, check_file_is_writeable};
invalid_output_for_target, out_filename, check_file_is_writeable,
filename_for_metadata};

// The third parameter is for env vars, used on windows to set up the
// path for MSVC to find its DLLs, and gcc to find its bundled
Expand Down Expand Up @@ -218,15 +219,6 @@ fn preserve_objects_for_their_debuginfo(sess: &Session) -> bool {
false
}

fn filename_for_metadata(sess: &Session, crate_name: &str, outputs: &OutputFilenames) -> PathBuf {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved it so I can only depend on librustc_codegen_utils in the save analysis crate (see here)

let out_filename = outputs.single_output_file.clone()
.unwrap_or(outputs
.out_directory
.join(&format!("lib{}{}.rmeta", crate_name, sess.opts.cg.extra_filename)));
check_file_is_writeable(&out_filename, sess);
out_filename
}

pub(crate) fn each_linked_rlib(sess: &Session,
info: &CrateInfo,
f: &mut dyn FnMut(CrateNum, &Path)) -> Result<(), String> {
Expand Down
13 changes: 13 additions & 0 deletions src/librustc_codegen_utils/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,19 @@ pub fn find_crate_name(sess: Option<&Session>,
"rust_out".to_string()
}

pub fn filename_for_metadata(sess: &Session,
crate_name: &str,
outputs: &OutputFilenames) -> PathBuf {
let libname = format!("{}{}", crate_name, sess.opts.cg.extra_filename);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed the formatting while moving to mimick filename_for_input


let out_filename = outputs.single_output_file.clone()
.unwrap_or(outputs.out_directory.join(&format!("lib{}.rmeta", libname)));

check_file_is_writeable(&out_filename, sess);

out_filename
}

pub fn filename_for_input(sess: &Session,
crate_type: config::CrateType,
crate_name: &str,
Expand Down
1 change: 1 addition & 0 deletions src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -980,6 +980,7 @@ pub fn enable_save_analysis(control: &mut CompileController) {
state.expanded_crate.unwrap(),
state.analysis.unwrap(),
state.crate_name.unwrap(),
state.input,
None,
DumpHandler::new(state.out_dir,
state.crate_name.unwrap()))
Expand Down
1 change: 1 addition & 0 deletions src/librustc_save_analysis/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ crate-type = ["dylib"]
log = "0.4"
rustc = { path = "../librustc" }
rustc_data_structures = { path = "../librustc_data_structures" }
rustc_codegen_utils = { path = "../librustc_codegen_utils" }
rustc_target = { path = "../librustc_target" }
rustc_typeck = { path = "../librustc_typeck" }
syntax = { path = "../libsyntax" }
Expand Down
54 changes: 52 additions & 2 deletions src/librustc_save_analysis/dump_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@

use rustc::hir::def::Def as HirDef;
use rustc::hir::def_id::DefId;
use rustc::session::config::Input;
use rustc::ty::{self, TyCtxt};
use rustc_data_structures::fx::FxHashSet;

use std::path::Path;
use std::env;

use syntax::ast::{self, Attribute, NodeId, PatKind, CRATE_NODE_ID};
use syntax::parse::token;
Expand All @@ -49,8 +51,8 @@ use json_dumper::{Access, DumpOutput, JsonDumper};
use span_utils::SpanUtils;
use sig;

use rls_data::{CratePreludeData, Def, DefKind, GlobalCrateId, Import, ImportKind, Ref, RefKind,
Relation, RelationKind, SpanData};
use rls_data::{CompilationOptions, CratePreludeData, Def, DefKind, GlobalCrateId, Import,
ImportKind, Ref, RefKind, Relation, RelationKind, SpanData};

macro_rules! down_cast_data {
($id:ident, $kind:ident, $sp:expr) => {
Expand Down Expand Up @@ -169,6 +171,54 @@ impl<'l, 'tcx: 'l, 'll, O: DumpOutput + 'll> DumpVisitor<'l, 'tcx, 'll, O> {
self.dumper.crate_prelude(data);
}

pub fn dump_compilation_options(&mut self, input: &Input, crate_name: &str) {
// Apply possible `remap-path-prefix` remapping to the input source file
// (and don't include remapping args anymore)
let (program, arguments) = {
let remap_arg_indices = {
let mut indices = FxHashSet();
// Args are guaranteed to be valid UTF-8 (checked early)
for (i, e) in env::args().enumerate() {
if e.starts_with("--remap-path-prefix=") {
indices.insert(i);
} else if e == "--remap-path-prefix" {
indices.insert(i);
indices.insert(i + 1);
}
}
indices
};

let mut args = env::args()
.enumerate()
.filter(|(i, _)| !remap_arg_indices.contains(i))
.map(|(_, arg)| {
match input {
Input::File(ref path) if path == Path::new(&arg) => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to propagate Input so that I can directly compare the argument to the non-mapped version (Input seems to be the only place where we can get input file before it's remapped, apart from scanning the cmd) to replace it with the mapped version.

I was also thinking about trying to remap every argument and see if it matches the already remapped local_crate_source_file but that seemed like it's considerably more mechanical work and more error-prone, so in the end I decided against it.

let mapped = &self.tcx.sess.local_crate_source_file;
mapped
.as_ref()
.unwrap()
.to_string_lossy()
.into()
},
_ => arg,
}
});

(args.next().unwrap(), args.collect())
};

let data = CompilationOptions {
directory: self.tcx.sess.working_dir.0.clone(),
program,
arguments,
output: self.save_ctxt.compilation_output(crate_name),
};

self.dumper.compilation_opts(data);
}

// Return all non-empty prefixes of a path.
// For each prefix, we return the span for the last segment in the prefix and
// a str representation of the entire prefix.
Expand Down
8 changes: 6 additions & 2 deletions src/librustc_save_analysis/json_dumper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ use std::io::Write;

use rustc_serialize::json::as_json;

use rls_data::{self, Analysis, CratePreludeData, Def, DefKind, Import, MacroRef, Ref, RefKind,
Relation, Impl};
use rls_data::config::Config;
use rls_data::{self, Analysis, CompilationOptions, CratePreludeData, Def, DefKind, Impl, Import,
MacroRef, Ref, RefKind, Relation};
use rls_span::{Column, Row};

#[derive(Debug)]
Expand Down Expand Up @@ -89,6 +89,10 @@ impl<'b, O: DumpOutput + 'b> JsonDumper<O> {
self.result.prelude = Some(data)
}

pub fn compilation_opts(&mut self, data: CompilationOptions) {
self.result.compilation = Some(data);
}

pub fn macro_use(&mut self, data: MacroRef) {
if self.config.pub_only || self.config.reachable_only {
return;
Expand Down
32 changes: 29 additions & 3 deletions src/librustc_save_analysis/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ extern crate rustc;
#[macro_use]
extern crate log;
extern crate rustc_data_structures;
extern crate rustc_codegen_utils;
extern crate rustc_serialize;
extern crate rustc_target;
extern crate rustc_typeck;
Expand All @@ -45,9 +46,10 @@ use rustc::hir::def::Def as HirDef;
use rustc::hir::Node;
use rustc::hir::def_id::{DefId, LOCAL_CRATE};
use rustc::middle::cstore::ExternCrate;
use rustc::session::config::CrateType;
use rustc::session::config::{CrateType, Input, OutputType};
use rustc::ty::{self, TyCtxt};
use rustc_typeck::hir_ty_to_ty;
use rustc_codegen_utils::link::{filename_for_metadata, out_filename};

use std::cell::Cell;
use std::default::Default;
Expand Down Expand Up @@ -110,6 +112,24 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
}
}

// Returns path to the compilation output (e.g. libfoo-12345678.rmeta)
pub fn compilation_output(&self, crate_name: &str) -> PathBuf {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought you said you were removing the output stuff? (Which seems like a good idea to me).

Copy link
Member Author

@Xanewok Xanewok Sep 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this doesn't use the file-based approach to figure out dependencies (crate disambiguators are used instead) but if we decide to somehow use save-analysis to lower this info further into low level file-based dep graph (potentially for Cargo build system integration? or maybe we shouldn't use save-analysis for that?) then we need some kind of related file input/output.

I removed CrateSource because one-of-dylib-rlib-rmeta-is-some format is clunky to work with but left this for the reasons explained above. Should I remove this as well?

Edit: CompilationOptions::output is now in rls-data 0.18.1 so we'd need to remove it from there as well.

let sess = &self.tcx.sess;
// Save-analysis is emitted per whole session, not per each crate type
let crate_type = sess.crate_types.borrow()[0];
let outputs = &*self.tcx.output_filenames(LOCAL_CRATE);

if outputs.outputs.contains_key(&OutputType::Metadata) {
filename_for_metadata(sess, crate_name, outputs)
} else if outputs.outputs.should_codegen() {
out_filename(sess, crate_type, outputs, crate_name)
} else {
// Otherwise it's only a DepInfo, in which case we return early and
// not even reach the analysis stage.
unreachable!()
}
}

// List external crates used by the current crate.
pub fn get_external_crates(&self) -> Vec<ExternalCrateData> {
let mut result = Vec::with_capacity(self.tcx.crates().len());
Expand All @@ -126,7 +146,7 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
result.push(ExternalCrateData {
// FIXME: change file_name field to PathBuf in rls-data
// https://github.com/nrc/rls-data/issues/7
file_name: self.span_utils.make_path_string(&lo_loc.file.name),
file_name: self.span_utils.make_filename_string(&lo_loc.file),
num: n.as_u32(),
id: GlobalCrateId {
name: self.tcx.crate_name(n).to_string(),
Expand Down Expand Up @@ -1015,6 +1035,7 @@ pub trait SaveHandler {
save_ctxt: SaveContext<'l, 'tcx>,
krate: &ast::Crate,
cratename: &str,
input: &'l Input,
);
}

Expand Down Expand Up @@ -1080,12 +1101,14 @@ impl<'a> SaveHandler for DumpHandler<'a> {
save_ctxt: SaveContext<'l, 'tcx>,
krate: &ast::Crate,
cratename: &str,
input: &'l Input,
) {
let output = &mut self.output_file(&save_ctxt);
let mut dumper = JsonDumper::new(output, save_ctxt.config.clone());
let mut visitor = DumpVisitor::new(save_ctxt, &mut dumper);

visitor.dump_crate_info(cratename, krate);
visitor.dump_compilation_options(input, cratename);
visit::walk_crate(&mut visitor, krate);
}
}
Expand All @@ -1101,6 +1124,7 @@ impl<'b> SaveHandler for CallbackHandler<'b> {
save_ctxt: SaveContext<'l, 'tcx>,
krate: &ast::Crate,
cratename: &str,
input: &'l Input,
) {
// We're using the JsonDumper here because it has the format of the
// save-analysis results that we will pass to the callback. IOW, we are
Expand All @@ -1111,6 +1135,7 @@ impl<'b> SaveHandler for CallbackHandler<'b> {
let mut visitor = DumpVisitor::new(save_ctxt, &mut dumper);

visitor.dump_crate_info(cratename, krate);
visitor.dump_compilation_options(input, cratename);
visit::walk_crate(&mut visitor, krate);
}
}
Expand All @@ -1120,6 +1145,7 @@ pub fn process_crate<'l, 'tcx, H: SaveHandler>(
krate: &ast::Crate,
analysis: &'l ty::CrateAnalysis,
cratename: &str,
input: &'l Input,
config: Option<Config>,
mut handler: H,
) {
Expand All @@ -1137,7 +1163,7 @@ pub fn process_crate<'l, 'tcx, H: SaveHandler>(
impl_counter: Cell::new(0),
};

handler.save(save_ctxt, krate, cratename)
handler.save(save_ctxt, krate, cratename, input)
})
}

Expand Down
26 changes: 18 additions & 8 deletions src/librustc_save_analysis/span_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,24 @@ impl<'a> SpanUtils<'a> {
}
}

pub fn make_path_string(&self, path: &FileName) -> String {
match *path {
FileName::Real(ref path) if !path.is_absolute() =>
self.sess.working_dir.0
.join(&path)
.display()
.to_string(),
_ => path.to_string(),
pub fn make_filename_string(&self, file: &SourceFile) -> String {
match &file.name {
FileName::Real(path) if !file.name_was_remapped => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice every file that's going through SourceMap will be remapped but here it seemed to be a missing case for not-yet-remapped absolute path so I included it for the sake of completeness.

if path.is_absolute() {
self.sess.source_map().path_mapping()
.map_prefix(path.clone()).0
.display()
.to_string()
} else {
self.sess.working_dir.0
.join(&path)
.display()
.to_string()
}
},
// If the file name is already remapped, we assume the user
// configured it the way they wanted to, so use that directly
filename => filename.to_string()
}
}

Expand Down