Skip to content
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

Avoid declaring a fake dependency edge #68298

Merged
merged 2 commits into from
Jan 23, 2020
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
44 changes: 38 additions & 6 deletions src/librustc_metadata/locator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -656,14 +656,36 @@ impl<'a> CrateLocator<'a> {
dylibs: FxHashMap<PathBuf, PathKind>,
) -> Option<(Svh, Library)> {
let mut slot = None;
// Order here matters, rmeta should come first. See comment in
// `extract_one` below.
let source = CrateSource {
rlib: self.extract_one(rlibs, CrateFlavor::Rlib, &mut slot),
rmeta: self.extract_one(rmetas, CrateFlavor::Rmeta, &mut slot),
rlib: self.extract_one(rlibs, CrateFlavor::Rlib, &mut slot),
dylib: self.extract_one(dylibs, CrateFlavor::Dylib, &mut slot),
};
slot.map(|(svh, metadata)| (svh, Library { source, metadata }))
}

fn needs_crate_flavor(&self, flavor: CrateFlavor) -> bool {
if flavor == CrateFlavor::Dylib && self.is_proc_macro == Some(true) {
return true;
}

// The all loop is because `--crate-type=rlib --crate-type=rlib` is
// legal and produces both inside this type.
let is_rlib = self.sess.crate_types.borrow().iter().all(|c| *c == config::CrateType::Rlib);
let needs_object_code = self.sess.opts.output_types.should_codegen();
// If we're producing an rlib, then we don't need object code.
// Or, if we're not producing object code, then we don't need it either
// (e.g., if we're a cdylib but emitting just metadata).
if is_rlib || !needs_object_code {
flavor == CrateFlavor::Rmeta
} else {
// we need all flavors (perhaps not true, but what we do for now)
true
}
}

// Attempts to extract *one* library from the set `m`. If the set has no
// elements, `None` is returned. If the set has more than one element, then
// the errors and notes are emitted about the set of libraries.
Expand All @@ -681,12 +703,22 @@ impl<'a> CrateLocator<'a> {
let mut ret: Option<(PathBuf, PathKind)> = None;
let mut error = 0;

// If we are producing an rlib, and we've already loaded metadata, then
// we should not attempt to discover further crate sources (unless we're
// locating a proc macro; exact logic is in needs_crate_flavor). This means
// that under -Zbinary-dep-depinfo we will not emit a dependency edge on
// the *unused* rlib, and by returning `None` here immediately we
// guarantee that we do indeed not use it.
//
// See also #68149 which provides more detail on why emitting the
// dependency on the rlib is a bad thing.
//
// We currenty do not verify that these other sources are even in sync,
// and this is arguably a bug (see #10786), but because reading metadata
// is quite slow (especially from dylibs) we currently do not read it
// from the other crate sources.
if slot.is_some() {
// FIXME(#10786): for an optimization, we only read one of the
// libraries' metadata sections. In theory we should
// read both, but reading dylib metadata is quite
// slow.
if m.is_empty() {
if m.is_empty() || !self.needs_crate_flavor(flavor) {
return None;
} else if m.len() == 1 {
return Some(m.into_iter().next().unwrap());
Expand Down
9 changes: 7 additions & 2 deletions src/test/ui/rmeta-rpass.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
// run-pass
// Test that using rlibs and rmeta dep crates work together. Specifically, that
// there can be both an rmeta and an rlib file and rustc will prefer the rlib.
// there can be both an rmeta and an rlib file and rustc will prefer the rmeta
// file.
//
// This behavior is simply making sure this doesn't accidentally change; in this
// case we want to make sure that the rlib isn't being used as that would cause
// bugs in -Zbinary-dep-depinfo (see #68298).

// aux-build:rmeta-rmeta.rs
// aux-build:rmeta-rlib-rpass.rs
Expand All @@ -9,5 +14,5 @@ extern crate rmeta_aux;
use rmeta_aux::Foo;

pub fn main() {
let _ = Foo { field: 42 };
let _ = Foo { field2: 42 };
}