auto merge of #13284 : pnkfelix/rust/more-fs-info-on-crate-mismatch, r=alexcrichton

Fix #13266.

There is a little bit of acrobatics in the definition of `crate_paths`
to avoid calling `clone()` on the dylib/rlib unless we actually are
going to need them.

The other oddity is that I have replaced the `root_ident: Option<&str>`
parameter with a `root: &Option<CratePaths>`, which may surprise one
who was expecting to see something like: `root: Option<&CratePaths>`.
I went with the approach here because I could not come up with code for
the alternative that was acceptable to the borrow checker.

Author: bors

src/librustc/driver/session.rs

@@ -232,6 +232,9 @@ impl Session {
     pub fn span_end_note(&self, sp: Span, msg: &str) {
         self.diagnostic().span_end_note(sp, msg)
     }
+    pub fn fileline_note(&self, sp: Span, msg: &str) {
+        self.diagnostic().fileline_note(sp, msg)
+    }
     pub fn note(&self, msg: &str) {
         self.diagnostic().handler().note(msg)
     }

src/librustc/metadata/creader.rs

@@ -21,6 +21,7 @@ use metadata::cstore;
 use metadata::decoder;
 use metadata::loader;
 use metadata::loader::Os;
+use metadata::loader::CratePaths;
 
 use std::cell::RefCell;
 use std::rc::Rc;
@@ -141,7 +142,7 @@ fn visit_view_item(e: &mut Env, i: &ast::ViewItem) {
 
     match extract_crate_info(e, i) {
         Some(info) => {
-            let cnum = resolve_crate(e, None, info.ident, &info.crate_id, None,
+            let cnum = resolve_crate(e, &None, info.ident, &info.crate_id, None,
                                      i.span);
             e.sess.cstore.add_extern_mod_stmt_cnum(info.id, cnum);
         }
@@ -278,13 +279,13 @@ fn existing_match(e: &Env, crate_id: &CrateId,
     None
 }
 
-fn resolve_crate(e: &mut Env,
-                 root_ident: Option<&str>,
-                 ident: &str,
-                 crate_id: &CrateId,
-                 hash: Option<&Svh>,
-                 span: Span)
-              -> ast::CrateNum {
+fn resolve_crate<'a>(e: &mut Env,
+                     root: &Option<CratePaths>,
+                     ident: &str,
+                     crate_id: &CrateId,
+                     hash: Option<&Svh>,
+                     span: Span)
+                     -> ast::CrateNum {
     match existing_match(e, crate_id, hash) {
         None => {
             let id_hash = link::crate_id_hash(crate_id);
@@ -297,11 +298,11 @@ fn resolve_crate(e: &mut Env,
                 hash: hash.map(|a| &*a),
                 os: e.os,
                 intr: e.intr.clone(),
-                rejected_via_hash: false,
+                rejected_via_hash: vec!(),
             };
             let loader::Library {
                 dylib, rlib, metadata
-            } = load_ctxt.load_library_crate(root_ident);
+            } = load_ctxt.load_library_crate(root);
 
             let crate_id = decoder::get_crate_id(metadata.as_slice());
             let hash = decoder::get_crate_hash(metadata.as_slice());
@@ -316,15 +317,22 @@ fn resolve_crate(e: &mut Env,
             });
             e.next_crate_num += 1;
 
-            // Maintain a reference to the top most crate.
-            let root_crate = match root_ident {
-                Some(c) => c,
-                None => load_ctxt.ident.clone()
+            // Stash paths for top-most crate locally if necessary.
+            let crate_paths = if root.is_none() {
+                Some(CratePaths {
+                    ident: load_ctxt.ident.to_owned(),
+                    dylib: dylib.clone(),
+                    rlib:  rlib.clone(),
+                })
+            } else {
+                None
             };
+            // Maintain a reference to the top most crate.
+            let root = if root.is_some() { root } else { &crate_paths };
 
             // Now resolve the crates referenced by this crate
             let cnum_map = resolve_crate_deps(e,
-            Some(root_crate),
+            root,
             metadata.as_slice(),
             span);
 
@@ -349,7 +357,7 @@ fn resolve_crate(e: &mut Env,
 
 // Go through the crate metadata and load any crates that it references
 fn resolve_crate_deps(e: &mut Env,
-                      root_ident: Option<&str>,
+                      root: &Option<CratePaths>,
                       cdata: &[u8], span : Span)
                    -> cstore::cnum_map {
     debug!("resolving deps of external crate");
@@ -360,7 +368,7 @@ fn resolve_crate_deps(e: &mut Env,
     for dep in r.iter() {
         let extrn_cnum = dep.cnum;
         debug!("resolving dep crate {} hash: `{}`", dep.crate_id, dep.hash);
-        let local_cnum = resolve_crate(e, root_ident,
+        let local_cnum = resolve_crate(e, root,
                                        dep.crate_id.name.as_slice(),
                                        &dep.crate_id,
                                        Some(&dep.hash),
@@ -393,7 +401,7 @@ impl<'a> Loader<'a> {
 impl<'a> CrateLoader for Loader<'a> {
     fn load_crate(&mut self, krate: &ast::ViewItem) -> MacroCrate {
         let info = extract_crate_info(&self.env, krate).unwrap();
-        let cnum = resolve_crate(&mut self.env, None, info.ident,
+        let cnum = resolve_crate(&mut self.env, &None, info.ident,
                                  &info.crate_id, None, krate.span);
         let library = self.env.sess.cstore.get_used_crate_source(cnum).unwrap();
         MacroCrate {

src/librustc/metadata/loader.rs

@@ -46,6 +46,10 @@ pub enum Os {
     OsFreebsd
 }
 
+pub struct HashMismatch {
+    path: Path,
+}
+
 pub struct Context<'a> {
     pub sess: &'a Session,
     pub span: Span,
@@ -55,7 +59,7 @@ pub struct Context<'a> {
     pub hash: Option<&'a Svh>,
     pub os: Os,
     pub intr: Rc<IdentInterner>,
-    pub rejected_via_hash: bool,
+    pub rejected_via_hash: Vec<HashMismatch>
 }
 
 pub struct Library {
@@ -70,6 +74,23 @@ pub struct ArchiveMetadata {
     data: &'static [u8],
 }
 
+pub struct CratePaths {
+    pub ident: ~str,
+    pub dylib: Option<Path>,
+    pub rlib: Option<Path>
+}
+
+impl CratePaths {
+    fn paths(&self) -> Vec<Path> {
+        match (&self.dylib, &self.rlib) {
+            (&None,    &None)              => vec!(),
+            (&Some(ref p), &None) |
+            (&None, &Some(ref p))          => vec!(p.clone()),
+            (&Some(ref p1), &Some(ref p2)) => vec!(p1.clone(), p2.clone()),
+        }
+    }
+}
+
 // FIXME(#11857) this should be a "real" realpath
 fn realpath(p: &Path) -> Path {
     use std::os;
@@ -83,26 +104,43 @@ fn realpath(p: &Path) -> Path {
 }
 
 impl<'a> Context<'a> {
-    pub fn load_library_crate(&mut self, root_ident: Option<&str>) -> Library {
+    pub fn load_library_crate(&mut self, root: &Option<CratePaths>) -> Library {
         match self.find_library_crate() {
             Some(t) => t,
             None => {
                 self.sess.abort_if_errors();
-                let message = if self.rejected_via_hash {
+                let message = if self.rejected_via_hash.len() > 0 {
                     format!("found possibly newer version of crate `{}`",
                             self.ident)
                 } else {
                     format!("can't find crate for `{}`", self.ident)
                 };
-                let message = match root_ident {
-                    None => message,
-                    Some(c) => format!("{} which `{}` depends on", message, c),
+                let message = match root {
+                    &None => message,
+                    &Some(ref r) => format!("{} which `{}` depends on",
+                                            message, r.ident)
                 };
                 self.sess.span_err(self.span, message);
 
-                if self.rejected_via_hash {
+                if self.rejected_via_hash.len() > 0 {
                     self.sess.span_note(self.span, "perhaps this crate needs \
                                                     to be recompiled?");
+                    let mismatches = self.rejected_via_hash.iter();
+                    for (i, &HashMismatch{ ref path }) in mismatches.enumerate() {
+                        self.sess.fileline_note(self.span,
+                            format!("crate `{}` path \\#{}: {}",
+                                    self.ident, i+1, path.display()));
+                    }
+                    match root {
+                        &None => {}
+                        &Some(ref r) => {
+                            for (i, path) in r.paths().iter().enumerate() {
+                                self.sess.fileline_note(self.span,
+                                    format!("crate `{}` path \\#{}: {}",
+                                            r.ident, i+1, path.display()));
+                            }
+                        }
+                    }
                 }
                 self.sess.abort_if_errors();
                 unreachable!()
@@ -292,7 +330,7 @@ impl<'a> Context<'a> {
             info!("{} reading metadata from: {}", flavor, lib.display());
             let metadata = match get_metadata_section(self.os, &lib) {
                 Ok(blob) => {
-                    if self.crate_matches(blob.as_slice()) {
+                    if self.crate_matches(blob.as_slice(), &lib) {
                         blob
                     } else {
                         info!("metadata mismatch");
@@ -327,7 +365,7 @@ impl<'a> Context<'a> {
         return if error > 0 {None} else {ret}
     }
 
-    fn crate_matches(&mut self, crate_data: &[u8]) -> bool {
+    fn crate_matches(&mut self, crate_data: &[u8], libpath: &Path) -> bool {
         match decoder::maybe_get_crate_id(crate_data) {
             Some(ref id) if self.crate_id.matches(id) => {}
             _ => return false
@@ -339,7 +377,7 @@ impl<'a> Context<'a> {
             None => true,
             Some(myhash) => {
                 if *myhash != hash {
-                    self.rejected_via_hash = true;
+                    self.rejected_via_hash.push(HashMismatch{ path: libpath.clone() });
                     false
                 } else {
                     true

src/libsyntax/diagnostic.rs

@@ -20,11 +20,37 @@ use term;
 // maximum number of lines we will print for each error; arbitrary.
 static MAX_LINES: uint = 6u;
 
+#[deriving(Clone)]
+pub enum RenderSpan {
+    /// A FullSpan renders with both with an initial line for the
+    /// message, prefixed by file:linenum, followed by a summary of
+    /// the source code covered by the span.
+    FullSpan(Span),
+
+    /// A FileLine renders with just a line for the message prefixed
+    /// by file:linenum.
+    FileLine(Span),
+}
+
+impl RenderSpan {
+    fn span(self) -> Span {
+        match self {
+            FullSpan(s) | FileLine(s) => s
+        }
+    }
+    fn is_full_span(&self) -> bool {
+        match self {
+            &FullSpan(..) => true,
+            &FileLine(..) => false,
+        }
+    }
+}
+
 pub trait Emitter {
     fn emit(&mut self, cmsp: Option<(&codemap::CodeMap, Span)>,
             msg: &str, lvl: Level);
     fn custom_emit(&mut self, cm: &codemap::CodeMap,
-                   sp: Span, msg: &str, lvl: Level);
+                   sp: RenderSpan, msg: &str, lvl: Level);
 }
 
 /// This structure is used to signify that a task has failed with a fatal error
@@ -60,7 +86,10 @@ impl SpanHandler {
         self.handler.emit(Some((&self.cm, sp)), msg, Note);
     }
     pub fn span_end_note(&self, sp: Span, msg: &str) {
-        self.handler.custom_emit(&self.cm, sp, msg, Note);
+        self.handler.custom_emit(&self.cm, FullSpan(sp), msg, Note);
+    }
+    pub fn fileline_note(&self, sp: Span, msg: &str) {
+        self.handler.custom_emit(&self.cm, FileLine(sp), msg, Note);
     }
     pub fn span_bug(&self, sp: Span, msg: &str) -> ! {
         self.handler.emit(Some((&self.cm, sp)), msg, Bug);
@@ -132,7 +161,7 @@ impl Handler {
         self.emit.borrow_mut().emit(cmsp, msg, lvl);
     }
     pub fn custom_emit(&self, cm: &codemap::CodeMap,
-                       sp: Span, msg: &str, lvl: Level) {
+                       sp: RenderSpan, msg: &str, lvl: Level) {
         self.emit.borrow_mut().custom_emit(cm, sp, msg, lvl);
     }
 }
@@ -258,7 +287,7 @@ impl Emitter for EmitterWriter {
             msg: &str,
             lvl: Level) {
         let error = match cmsp {
-            Some((cm, sp)) => emit(self, cm, sp, msg, lvl, false),
+            Some((cm, sp)) => emit(self, cm, FullSpan(sp), msg, lvl, false),
             None => print_diagnostic(self, "", lvl, msg),
         };
 
@@ -269,16 +298,17 @@ impl Emitter for EmitterWriter {
     }
 
     fn custom_emit(&mut self, cm: &codemap::CodeMap,
-                   sp: Span, msg: &str, lvl: Level) {
+                   sp: RenderSpan, msg: &str, lvl: Level) {
         match emit(self, cm, sp, msg, lvl, true) {
             Ok(()) => {}
             Err(e) => fail!("failed to print diagnostics: {}", e),
         }
     }
 }
 
-fn emit(dst: &mut EmitterWriter, cm: &codemap::CodeMap, sp: Span,
+fn emit(dst: &mut EmitterWriter, cm: &codemap::CodeMap, rsp: RenderSpan,
         msg: &str, lvl: Level, custom: bool) -> io::IoResult<()> {
+    let sp = rsp.span();
     let ss = cm.span_to_str(sp);
     let lines = cm.span_to_lines(sp);
     if custom {
@@ -288,10 +318,14 @@ fn emit(dst: &mut EmitterWriter, cm: &codemap::CodeMap, sp: Span,
         let span_end = Span { lo: sp.hi, hi: sp.hi, expn_info: sp.expn_info};
         let ses = cm.span_to_str(span_end);
         try!(print_diagnostic(dst, ses, lvl, msg));
-        try!(custom_highlight_lines(dst, cm, sp, lvl, lines));
+        if rsp.is_full_span() {
+            try!(custom_highlight_lines(dst, cm, sp, lvl, lines));
+        }
     } else {
         try!(print_diagnostic(dst, ss, lvl, msg));
-        try!(highlight_lines(dst, cm, sp, lvl, lines));
+        if rsp.is_full_span() {
+            try!(highlight_lines(dst, cm, sp, lvl, lines));
+        }
     }
     print_macro_backtrace(dst, cm, sp)
 }

src/test/run-make/many-crates-but-no-match/Makefile

@@ -0,0 +1,34 @@
+-include ../tools.mk
+
+# Modelled after compile-fail/changing-crates test, but this one puts
+# more than one (mismatching) candidate crate into the search path,
+# which did not appear directly expressible in compile-fail/aux-build
+# infrastructure.
+#
+# Note that we move the built libraries into target direcrtories rather than
+# use the `--out-dir` option because the `../tools.mk` file already bakes a
+# use of `--out-dir` into the definition of $(RUSTC).
+
+A1=$(TMPDIR)/a1
+A2=$(TMPDIR)/a2
+A3=$(TMPDIR)/a3
+
+# A hack to match distinct lines of output from a single run.
+LOG=$(TMPDIR)/log.txt
+
+all: 
+	mkdir -p $(A1) $(A2) $(A3)
+	$(RUSTC) --crate-type=rlib crateA1.rs
+	mv $(TMPDIR)/$(call RLIB_GLOB,crateA) $(A1)
+	$(RUSTC) --crate-type=rlib -L$(A1) crateB.rs
+	$(RUSTC) --crate-type=rlib crateA2.rs
+	mv $(TMPDIR)/$(call RLIB_GLOB,crateA) $(A2)
+	$(RUSTC) --crate-type=rlib crateA3.rs
+	mv $(TMPDIR)/$(call RLIB_GLOB,crateA) $(A3)
+	# Ensure crateC fails to compile since A1 is "missing" and A2/A3 hashes do not match
+	$(RUSTC) -L$(A2) -L$(A3) crateC.rs >$(LOG) 2>&1 || true
+	grep "error: found possibly newer version of crate \`crateA\` which \`crateB\` depends on" $(LOG)
+	grep "note: perhaps this crate needs to be recompiled?" $(LOG)
+	grep "note: crate \`crateA\` path #1:" $(LOG)
+	grep "note: crate \`crateA\` path #2:" $(LOG)
+	grep "note: crate \`crateB\` path #1:" $(LOG)

src/test/run-make/many-crates-but-no-match/crateA1.rs

@@ -0,0 +1,14 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![crate_id="crateA"]
+
+// Base crate
+pub fn func<T>() {}