From 8213e184477c19eba75840e9310266a6529de20a Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 24 Feb 2014 18:13:51 -0800 Subject: [PATCH 1/3] rustc: Simplify crate loading constraints The previous code passed around a {name,version} pair everywhere, but this is better expressed as a CrateId. This patch changes these paths to store and pass around crate ids instead of these pairs of name/version. This also prepares the code to change the type of hash that is stored in crates. --- src/librustc/metadata/common.rs | 8 +- src/librustc/metadata/creader.rs | 228 ++++++++++++------------------ src/librustc/metadata/cstore.rs | 40 +----- src/librustc/metadata/decoder.rs | 40 +++--- src/librustc/metadata/encoder.rs | 19 +-- src/librustc/metadata/loader.rs | 54 +++---- src/librustc/middle/trans/base.rs | 2 +- src/libsyntax/crateid.rs | 9 ++ 8 files changed, 160 insertions(+), 240 deletions(-) diff --git a/src/librustc/metadata/common.rs b/src/librustc/metadata/common.rs index 9cf4df287d2f4..62f1dcedab458 100644 --- a/src/librustc/metadata/common.rs +++ b/src/librustc/metadata/common.rs @@ -70,12 +70,12 @@ pub static tag_crate_deps: uint = 0x18; pub static tag_crate_dep: uint = 0x19; pub static tag_crate_hash: uint = 0x1a; +pub static tag_crate_crateid: uint = 0x1b; -pub static tag_parent_item: uint = 0x1b; +pub static tag_parent_item: uint = 0x1c; -pub static tag_crate_dep_name: uint = 0x1c; -pub static tag_crate_dep_hash: uint = 0x1d; -pub static tag_crate_dep_vers: uint = 0x1e; +pub static tag_crate_dep_crateid: uint = 0x1d; +pub static tag_crate_dep_hash: uint = 0x1e; pub static tag_mod_impl: uint = 0x1f; diff --git a/src/librustc/metadata/creader.rs b/src/librustc/metadata/creader.rs index 9f14b571d8225..1108917cdb1d5 100644 --- a/src/librustc/metadata/creader.rs +++ b/src/librustc/metadata/creader.rs @@ -79,7 +79,7 @@ struct cache_entry { cnum: ast::CrateNum, span: Span, hash: ~str, - crateid: CrateId, + crate_id: CrateId, } fn dump_crates(crate_cache: &[cache_entry]) { @@ -95,10 +95,10 @@ fn warn_if_multiple_versions(e: &mut Env, diag: @SpanHandler, crate_cache: &[cache_entry]) { if crate_cache.len() != 0u { - let name = crate_cache[crate_cache.len() - 1].crateid.name.clone(); + let name = crate_cache[crate_cache.len() - 1].crate_id.name.clone(); let (matches, non_matches) = crate_cache.partitioned(|entry| - name == entry.crateid.name); + name == entry.crate_id.name); assert!(!matches.is_empty()); @@ -107,7 +107,7 @@ fn warn_if_multiple_versions(e: &mut Env, format!("using multiple versions of crate `{}`", name)); for match_ in matches.iter() { diag.span_note(match_.span, "used here"); - loader::note_crateid_attr(diag, &match_.crateid); + loader::note_crateid_attr(diag, &match_.crate_id); } } @@ -146,14 +146,9 @@ fn visit_view_item(e: &mut Env, i: &ast::ViewItem) { return; } - match extract_crate_info(i) { + match extract_crate_info(e, i) { Some(info) => { - let cnum = resolve_crate(e, - None, - info.ident.clone(), - info.name.clone(), - info.version.clone(), - ~"", + let cnum = resolve_crate(e, None, info.ident, &info.crate_id, "", i.span); e.sess.cstore.add_extern_mod_stmt_cnum(info.id, cnum); } @@ -163,38 +158,33 @@ fn visit_view_item(e: &mut Env, i: &ast::ViewItem) { struct CrateInfo { ident: ~str, - name: ~str, - version: ~str, + crate_id: CrateId, id: ast::NodeId, } -fn extract_crate_info(i: &ast::ViewItem) -> Option { +fn extract_crate_info(e: &Env, i: &ast::ViewItem) -> Option { match i.node { ast::ViewItemExternMod(ident, ref path_opt, id) => { let ident = token::get_ident(ident); debug!("resolving extern crate stmt. ident: {:?} path_opt: {:?}", ident, path_opt); - let (name, version) = match *path_opt { + let crate_id = match *path_opt { Some((ref path_str, _)) => { let crateid: Option = from_str(path_str.get()); match crateid { - None => (~"", ~""), - Some(crateid) => { - let version = match crateid.version { - None => ~"", - Some(ref ver) => ver.to_str(), - }; - (crateid.name.to_str(), version) + None => { + e.sess.span_err(i.span, "malformed crate id"); + return None } + Some(id) => id } } - None => (ident.get().to_str(), ~""), + None => from_str(ident.get().to_str()).unwrap() }; Some(CrateInfo { - ident: ident.get().to_str(), - name: name, - version: version, - id: id, + ident: ident.get().to_str(), + crate_id: crate_id, + id: id, }) } _ => None @@ -285,100 +275,93 @@ fn visit_item(e: &Env, i: &ast::Item) { } } -fn existing_match(e: &Env, name: &str, version: &str, hash: &str) -> Option { +fn existing_match(e: &Env, crate_id: &CrateId, + hash: &str) -> Option { let crate_cache = e.crate_cache.borrow(); for c in crate_cache.get().iter() { - let crateid_version = match c.crateid.version { - None => ~"0.0", - Some(ref ver) => ver.to_str(), - }; - if (name.is_empty() || name == c.crateid.name) && - (version.is_empty() || version == crateid_version) && - (hash.is_empty() || hash == c.hash) { - return Some(c.cnum); + if crate_id.matches(&c.crate_id) && + (hash.is_empty() || hash == c.hash.as_slice()) { + return Some(c.cnum) } } None } fn resolve_crate(e: &mut Env, - root_ident: Option<~str>, - ident: ~str, - name: ~str, - version: ~str, - hash: ~str, + root_ident: Option<&str>, + ident: &str, + crate_id: &CrateId, + hash: &str, span: Span) -> ast::CrateNum { - match existing_match(e, name, version, hash) { - None => { - let load_ctxt = loader::Context { - sess: e.sess, - span: span, - ident: ident, - name: name, - version: version, - hash: hash, - os: e.os, - intr: e.intr - }; - let loader::Library { - dylib, rlib, metadata - } = load_ctxt.load_library_crate(root_ident.clone()); - - let attrs = decoder::get_crate_attributes(metadata.as_slice()); - let crateid = attr::find_crateid(attrs).unwrap(); - let hash = decoder::get_crate_hash(metadata.as_slice()); - - // Claim this crate number and cache it - let cnum = e.next_crate_num; - { - let mut crate_cache = e.crate_cache.borrow_mut(); - crate_cache.get().push(cache_entry { - cnum: cnum, + match existing_match(e, crate_id, hash) { + None => { + let load_ctxt = loader::Context { + sess: e.sess, span: span, + ident: ident, + crate_id: crate_id, hash: hash, - crateid: crateid, - }); - } - 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() - }; + os: e.os, + intr: e.intr + }; + let loader::Library { + dylib, rlib, metadata + } = load_ctxt.load_library_crate(root_ident); + + let crate_id = decoder::get_crate_id(metadata.as_slice()); + let hash = decoder::get_crate_hash(metadata.as_slice()); + + // Claim this crate number and cache it + let cnum = e.next_crate_num; + { + let mut crate_cache = e.crate_cache.borrow_mut(); + crate_cache.get().push(cache_entry { + cnum: cnum, + span: span, + hash: hash, + crate_id: crate_id, + }); + } + e.next_crate_num += 1; - // Now resolve the crates referenced by this crate - let cnum_map = resolve_crate_deps(e, - Some(root_crate), - metadata.as_slice(), - span); + // Maintain a reference to the top most crate. + let root_crate = match root_ident { + Some(c) => c, + None => load_ctxt.ident.clone() + }; - let cmeta = @cstore::crate_metadata { - name: load_ctxt.name, - data: metadata, - cnum_map: cnum_map, - cnum: cnum - }; + // Now resolve the crates referenced by this crate + let cnum_map = resolve_crate_deps(e, + Some(root_crate), + metadata.as_slice(), + span); + + let cmeta = @cstore::crate_metadata { + name: load_ctxt.crate_id.name.to_owned(), + data: metadata, + cnum_map: cnum_map, + cnum: cnum + }; - let cstore = e.sess.cstore; - cstore.set_crate_data(cnum, cmeta); - cstore.add_used_crate_source(cstore::CrateSource { - dylib: dylib, - rlib: rlib, - cnum: cnum, - }); - return cnum; - } - Some(cnum) => { - return cnum; - } + let cstore = e.sess.cstore; + cstore.set_crate_data(cnum, cmeta); + cstore.add_used_crate_source(cstore::CrateSource { + dylib: dylib, + rlib: rlib, + cnum: cnum, + }); + return cnum; + } + Some(cnum) => { + return cnum; + } } } // Go through the crate metadata and load any crates that it references fn resolve_crate_deps(e: &mut Env, - root_ident: Option<~str>, + root_ident: Option<&str>, cdata: &[u8], span : Span) -> cstore::cnum_map { debug!("resolving deps of external crate"); @@ -388,31 +371,13 @@ fn resolve_crate_deps(e: &mut Env, let r = decoder::get_crate_deps(cdata); for dep in r.iter() { let extrn_cnum = dep.cnum; - let cname_str = token::get_ident(dep.name); - debug!("resolving dep crate {} ver: {} hash: {}", - cname_str, dep.vers, dep.hash); - match existing_match(e, - cname_str.get(), - dep.vers, - dep.hash) { - Some(local_cnum) => { - debug!("already have it"); - // We've already seen this crate - cnum_map.insert(extrn_cnum, local_cnum); - } - None => { - debug!("need to load it"); - // This is a new one so we've got to load it - let local_cnum = resolve_crate(e, - root_ident.clone(), - cname_str.get().to_str(), - cname_str.get().to_str(), - dep.vers.clone(), - dep.hash.clone(), - span); - cnum_map.insert(extrn_cnum, local_cnum); - } - } + debug!("resolving dep crate {} hash: `{}`", dep.crate_id, dep.hash); + let local_cnum = resolve_crate(e, root_ident, + dep.crate_id.name.as_slice(), + &dep.crate_id, + dep.hash, + span); + cnum_map.insert(extrn_cnum, local_cnum); } return @RefCell::new(cnum_map); } @@ -439,14 +404,9 @@ impl Loader { impl CrateLoader for Loader { fn load_crate(&mut self, krate: &ast::ViewItem) -> MacroCrate { - let info = extract_crate_info(krate).unwrap(); - let cnum = resolve_crate(&mut self.env, - None, - info.ident.clone(), - info.name.clone(), - info.version.clone(), - ~"", - krate.span); + let info = extract_crate_info(&self.env, krate).unwrap(); + let cnum = resolve_crate(&mut self.env, None, info.ident, + &info.crate_id, "", krate.span); let library = self.env.sess.cstore.get_used_crate_source(cnum).unwrap(); MacroCrate { lib: library.dylib, diff --git a/src/librustc/metadata/cstore.rs b/src/librustc/metadata/cstore.rs index ce6b5af8d0e07..12461ddbe7154 100644 --- a/src/librustc/metadata/cstore.rs +++ b/src/librustc/metadata/cstore.rs @@ -21,6 +21,7 @@ use collections::HashMap; use extra::c_vec::CVec; use syntax::ast; use syntax::parse::token::IdentInterner; +use syntax::crateid::CrateId; // A map from external crate numbers (as decoded from some crate file) to // local crate numbers (as generated during this session). Each external @@ -96,9 +97,9 @@ impl CStore { decoder::get_crate_hash(cdata.data()) } - pub fn get_crate_vers(&self, cnum: ast::CrateNum) -> ~str { + pub fn get_crate_id(&self, cnum: ast::CrateNum) -> CrateId { let cdata = self.get_crate_data(cnum); - decoder::get_crate_vers(cdata.data()) + decoder::get_crate_id(cdata.data()) } pub fn set_crate_data(&self, cnum: ast::CrateNum, data: @crate_metadata) { @@ -191,41 +192,6 @@ impl CStore { let extern_mod_crate_map = self.extern_mod_crate_map.borrow(); extern_mod_crate_map.get().find(&emod_id).map(|x| *x) } - - // returns hashes of crates directly used by this crate. Hashes are sorted by - // (crate name, crate version, crate hash) in lexicographic order (not semver) - pub fn get_dep_hashes(&self) -> ~[~str] { - let mut result = ~[]; - - let extern_mod_crate_map = self.extern_mod_crate_map.borrow(); - for (_, &cnum) in extern_mod_crate_map.get().iter() { - let cdata = self.get_crate_data(cnum); - let hash = decoder::get_crate_hash(cdata.data()); - let vers = decoder::get_crate_vers(cdata.data()); - debug!("Add hash[{}]: {} {}", cdata.name, vers, hash); - result.push(crate_hash { - name: cdata.name.clone(), - vers: vers, - hash: hash - }); - } - - result.sort(); - - debug!("sorted:"); - for x in result.iter() { - debug!(" hash[{}]: {}", x.name, x.hash); - } - - result.move_iter().map(|crate_hash { hash, ..}| hash).collect() - } -} - -#[deriving(Clone, TotalEq, TotalOrd)] -struct crate_hash { - name: ~str, - vers: ~str, - hash: ~str, } impl crate_metadata { diff --git a/src/librustc/metadata/decoder.rs b/src/librustc/metadata/decoder.rs index f9f55fbc1bb01..f365ddef94f54 100644 --- a/src/librustc/metadata/decoder.rs +++ b/src/librustc/metadata/decoder.rs @@ -17,7 +17,6 @@ use metadata::common::*; use metadata::csearch::StaticMethodInfo; use metadata::csearch; use metadata::cstore; -use metadata::decoder; use metadata::tydecode::{parse_ty_data, parse_def_id, parse_type_param_def_data, parse_bare_fn_ty_data, parse_trait_ref_data}; @@ -44,6 +43,7 @@ use syntax::parse::token; use syntax::print::pprust; use syntax::ast; use syntax::codemap; +use syntax::crateid::CrateId; type Cmd = @crate_metadata; @@ -1108,9 +1108,8 @@ pub fn get_crate_attributes(data: &[u8]) -> ~[ast::Attribute] { #[deriving(Clone)] pub struct CrateDep { cnum: ast::CrateNum, - name: ast::Ident, - vers: ~str, - hash: ~str + crate_id: CrateId, + hash: ~str, } pub fn get_crate_deps(data: &[u8]) -> ~[CrateDep] { @@ -1123,10 +1122,13 @@ pub fn get_crate_deps(data: &[u8]) -> ~[CrateDep] { d.as_str_slice().to_str() } reader::tagged_docs(depsdoc, tag_crate_dep, |depdoc| { - deps.push(CrateDep {cnum: crate_num, - name: token::str_to_ident(docstr(depdoc, tag_crate_dep_name)), - vers: docstr(depdoc, tag_crate_dep_vers), - hash: docstr(depdoc, tag_crate_dep_hash)}); + let crate_id = from_str(docstr(depdoc, tag_crate_dep_crateid)).unwrap(); + let hash = docstr(depdoc, tag_crate_dep_hash); + deps.push(CrateDep { + cnum: crate_num, + crate_id: crate_id, + hash: hash, + }); crate_num += 1; true }); @@ -1135,17 +1137,9 @@ pub fn get_crate_deps(data: &[u8]) -> ~[CrateDep] { fn list_crate_deps(data: &[u8], out: &mut io::Writer) -> io::IoResult<()> { try!(write!(out, "=External Dependencies=\n")); - - let r = get_crate_deps(data); - for dep in r.iter() { - try!(write!(out, - "{} {}-{}-{}\n", - dep.cnum, - token::get_ident(dep.name), - dep.hash, - dep.vers)); + for dep in get_crate_deps(data).iter() { + try!(write!(out, "{} {}-{}\n", dep.cnum, dep.crate_id, dep.hash)); } - try!(write!(out, "\n")); Ok(()) } @@ -1156,12 +1150,10 @@ pub fn get_crate_hash(data: &[u8]) -> ~str { hashdoc.as_str_slice().to_str() } -pub fn get_crate_vers(data: &[u8]) -> ~str { - let attrs = decoder::get_crate_attributes(data); - match attr::find_crateid(attrs) { - None => ~"0.0", - Some(crateid) => crateid.version_or_default().to_str(), - } +pub fn get_crate_id(data: &[u8]) -> CrateId { + let cratedoc = reader::Doc(data); + let hashdoc = reader::get_doc(cratedoc, tag_crate_crateid); + from_str(hashdoc.as_str_slice()).unwrap() } pub fn list_crate_metadata(bytes: &[u8], out: &mut io::Writer) -> io::IoResult<()> { diff --git a/src/librustc/metadata/encoder.rs b/src/librustc/metadata/encoder.rs index 285a7411270ff..976c1ee92d3de 100644 --- a/src/librustc/metadata/encoder.rs +++ b/src/librustc/metadata/encoder.rs @@ -40,6 +40,7 @@ use syntax::ast_util::*; use syntax::ast_util; use syntax::attr::AttrMetaMethods; use syntax::attr; +use syntax::crateid::CrateId; use syntax::diagnostic::SpanHandler; use syntax::parse::token::InternedString; use syntax::parse::token::special_idents; @@ -1510,8 +1511,7 @@ fn encode_crate_deps(ebml_w: &mut writer::Encoder, cstore: &cstore::CStore) { cstore.iter_crate_data(|key, val| { let dep = decoder::CrateDep { cnum: key, - name: token::str_to_ident(val.name), - vers: decoder::get_crate_vers(val.data()), + crate_id: decoder::get_crate_id(val.data()), hash: decoder::get_crate_hash(val.data()) }; deps.push(dep); @@ -1729,12 +1729,8 @@ fn encode_misc_info(ecx: &EncodeContext, fn encode_crate_dep(ebml_w: &mut writer::Encoder, dep: decoder::CrateDep) { ebml_w.start_tag(tag_crate_dep); - ebml_w.start_tag(tag_crate_dep_name); - let s = token::get_ident(dep.name); - ebml_w.writer.write(s.get().as_bytes()); - ebml_w.end_tag(); - ebml_w.start_tag(tag_crate_dep_vers); - ebml_w.writer.write(dep.vers.as_bytes()); + ebml_w.start_tag(tag_crate_dep_crateid); + ebml_w.writer.write(dep.crate_id.to_str().as_bytes()); ebml_w.end_tag(); ebml_w.start_tag(tag_crate_dep_hash); ebml_w.writer.write(dep.hash.as_bytes()); @@ -1748,6 +1744,12 @@ fn encode_hash(ebml_w: &mut writer::Encoder, hash: &str) { ebml_w.end_tag(); } +fn encode_crate_id(ebml_w: &mut writer::Encoder, crate_id: &CrateId) { + ebml_w.start_tag(tag_crate_crateid); + ebml_w.writer.write(crate_id.to_str().as_bytes()); + ebml_w.end_tag(); +} + // NB: Increment this as you change the metadata encoding version. pub static metadata_encoding_version : &'static [u8] = &[0x72, //'r' as u8, @@ -1806,6 +1808,7 @@ fn encode_metadata_inner(wr: &mut MemWriter, parms: EncodeParams, krate: &Crate) let mut ebml_w = writer::Encoder(wr); + encode_crate_id(&mut ebml_w, &ecx.link_meta.crateid); encode_hash(&mut ebml_w, ecx.link_meta.crate_hash); let mut i = ebml_w.writer.tell().unwrap(); diff --git a/src/librustc/metadata/loader.rs b/src/librustc/metadata/loader.rs index 1f5b76953dcde..faef2412e7802 100644 --- a/src/librustc/metadata/loader.rs +++ b/src/librustc/metadata/loader.rs @@ -44,13 +44,12 @@ pub enum Os { OsFreebsd } -pub struct Context { +pub struct Context<'a> { sess: Session, span: Span, - ident: ~str, - name: ~str, - version: ~str, - hash: ~str, + ident: &'a str, + crate_id: &'a CrateId, + hash: &'a str, os: Os, intr: @IdentInterner } @@ -79,8 +78,8 @@ fn realpath(p: &Path) -> Path { } } -impl Context { - pub fn load_library_crate(&self, root_ident: Option<~str>) -> Library { +impl<'a> Context<'a> { + pub fn load_library_crate(&self, root_ident: Option<&str>) -> Library { match self.find_library_crate() { Some(t) => t, None => { @@ -101,8 +100,8 @@ impl Context { let (dyprefix, dysuffix) = self.dylibname(); // want: crate_name.dir_part() + prefix + crate_name.file_part + "-" - let dylib_prefix = format!("{}{}-", dyprefix, self.name); - let rlib_prefix = format!("lib{}-", self.name); + let dylib_prefix = format!("{}{}-", dyprefix, self.crate_id.name); + let rlib_prefix = format!("lib{}-", self.crate_id.name); let mut candidates = HashMap::new(); @@ -196,7 +195,8 @@ impl Context { 1 => Some(libraries[0]), _ => { self.sess.span_err(self.span, - format!("multiple matching crates for `{}`", self.name)); + format!("multiple matching crates for `{}`", + self.crate_id.name)); self.sess.note("candidates:"); for lib in libraries.iter() { match lib.dylib { @@ -243,11 +243,12 @@ impl Context { debug!("matching -- {}, hash: {}", file, hash); let vers = match parts.next() { Some(v) => v, None => return None }; debug!("matching -- {}, vers: {}", file, vers); - if !self.version.is_empty() && self.version.as_slice() != vers { - return None + match self.crate_id.version { + Some(ref version) if version.as_slice() != vers => return None, + Some(..) | None => {} } debug!("matching -- {}, vers ok (requested {})", file, - self.version); + self.crate_id.version); // hashes in filenames are prefixes of the "true hash" if self.hash.is_empty() || self.hash.starts_with(hash) { debug!("matching -- {}, hash ok (requested {})", file, self.hash); @@ -275,7 +276,7 @@ impl Context { if m.len() > 1 { self.sess.span_err(self.span, format!("multiple {} candidates for `{}` \ - found", flavor, self.name)); + found", flavor, self.crate_id.name)); for (i, path) in m.iter().enumerate() { self.sess.span_note(self.span, format!(r"candidate \#{}: {}", i + 1, @@ -289,8 +290,7 @@ impl Context { info!("{} reading meatadata from: {}", flavor, lib.display()); match get_metadata_section(self.os, &lib) { Some(blob) => { - if crate_matches(blob.as_slice(), self.name, - self.version, self.hash) { + if crate_matches(blob.as_slice(), self.crate_id, self.hash){ *slot = Some(blob); } else { info!("metadata mismatch"); @@ -323,23 +323,13 @@ pub fn note_crateid_attr(diag: @SpanHandler, crateid: &CrateId) { diag.handler().note(format!("crate_id: {}", crateid.to_str())); } -fn crate_matches(crate_data: &[u8], - name: &str, - version: &str, - hash: &str) -> bool { - let attrs = decoder::get_crate_attributes(crate_data); - match attr::find_crateid(attrs) { - None => false, - Some(crateid) => { - if !hash.is_empty() { - let chash = decoder::get_crate_hash(crate_data); - if chash.as_slice() != hash { return false; } - } - name == crateid.name && - (version.is_empty() || - crateid.version_or_default() == version) - } +fn crate_matches(crate_data: &[u8], crate_id: &CrateId, hash: &str) -> bool { + let other_id = decoder::get_crate_id(crate_data); + if !crate_id.matches(&other_id) { return false } + if hash != "" && hash != decoder::get_crate_hash(crate_data).as_slice() { + return false } + return true; } impl ArchiveMetadata { diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 6a7694b2b81a1..553dbe2ae6e36 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -2488,7 +2488,7 @@ pub fn fill_crate_map(ccx: @CrateContext, map: ValueRef) { let cdata = cstore.get_crate_data(i); let nm = symname(format!("_rust_crate_map_{}", cdata.name), cstore.get_crate_hash(i), - cstore.get_crate_vers(i)); + cstore.get_crate_id(i).version_or_default()); let cr = nm.with_c_str(|buf| { unsafe { llvm::LLVMAddGlobal(ccx.llmod, ccx.int_type.to_ref(), buf) diff --git a/src/libsyntax/crateid.rs b/src/libsyntax/crateid.rs index 659cd13c94dd1..b5f02fb7e6441 100644 --- a/src/libsyntax/crateid.rs +++ b/src/libsyntax/crateid.rs @@ -107,6 +107,15 @@ impl CrateId { pub fn short_name_with_version(&self) -> ~str { format!("{}-{}", self.name, self.version_or_default()) } + + pub fn matches(&self, other: &CrateId) -> bool { + // FIXME: why does this not match on `path`? + if self.name != other.name { return false } + match (&self.version, &other.version) { + (&Some(ref v1), &Some(ref v2)) => v1 == v2, + _ => true, + } + } } #[test] From ec57db083ff10fc4da0a2f25df5acf1d4398e560 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 24 Feb 2014 19:45:20 -0800 Subject: [PATCH 2/3] rustc: Add the concept of a Strict Version Hash This new SVH is used to uniquely identify all crates as a snapshot in time of their ABI/API/publicly reachable state. This current calculation is just a hash of the entire crate's AST. This is obviously incorrect, but it is currently the reality for today. This change threads through the new Svh structure which originates from crate dependencies. The concept of crate id hash is preserved to provide efficient matching on filenames for crate loading. The inspected hash once crate metadata is opened has been changed to use the new Svh. The goal of this hash is to identify when upstream crates have changed but downstream crates have not been recompiled. This will prevent the def-id drift problem where upstream crates were recompiled, thereby changing their metadata, but downstream crates were not recompiled. In the future this hash can be expanded to exclude contents of the AST like doc comments, but limitations in the compiler prevent this change from being made at this time. Closes #10207 --- mk/clean.mk | 1 + src/librustc/back/link.rs | 62 ++++++------- src/librustc/back/svh.rs | 112 +++++++++++++++++++++++ src/librustc/driver/driver.rs | 7 +- src/librustc/lib.rs | 38 +++----- src/librustc/metadata/common.rs | 5 +- src/librustc/metadata/creader.rs | 30 +++--- src/librustc/metadata/cstore.rs | 3 +- src/librustc/metadata/decoder.rs | 15 +-- src/librustc/metadata/encoder.rs | 9 +- src/librustc/metadata/loader.rs | 88 +++++++++++------- src/librustc/middle/trans/base.rs | 25 +++-- src/librustc/middle/trans/intrinsic.rs | 2 +- src/librustc/middle/ty.rs | 7 +- src/test/auxiliary/changing-crates-a1.rs | 13 +++ src/test/auxiliary/changing-crates-a2.rs | 14 +++ src/test/auxiliary/changing-crates-b.rs | 15 +++ src/test/compile-fail/bad-crate-id.rs | 14 +++ src/test/compile-fail/changing-crates.rs | 20 ++++ 19 files changed, 343 insertions(+), 137 deletions(-) create mode 100644 src/librustc/back/svh.rs create mode 100644 src/test/auxiliary/changing-crates-a1.rs create mode 100644 src/test/auxiliary/changing-crates-a2.rs create mode 100644 src/test/auxiliary/changing-crates-b.rs create mode 100644 src/test/compile-fail/bad-crate-id.rs create mode 100644 src/test/compile-fail/changing-crates.rs diff --git a/mk/clean.mk b/mk/clean.mk index 002db59ad3879..bc5961a998131 100644 --- a/mk/clean.mk +++ b/mk/clean.mk @@ -58,6 +58,7 @@ clean-generic-$(2)-$(1): -name '*.[odasS]' -o \ -name '*.so' -o \ -name '*.dylib' -o \ + -name '*.rlib' -o \ -name 'stamp.*' -o \ -name '*.lib' -o \ -name '*.dll' -o \ diff --git a/src/librustc/back/link.rs b/src/librustc/back/link.rs index 33d3a1c67f2e2..bacc98a01356a 100644 --- a/src/librustc/back/link.rs +++ b/src/librustc/back/link.rs @@ -8,9 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. - use back::archive::{Archive, METADATA_FILENAME}; use back::rpath; +use back::svh::Svh; use driver::driver::{CrateTranslation, OutputFilenames}; use driver::session::Session; use driver::session; @@ -499,28 +499,31 @@ pub mod write { * system linkers understand. */ -pub fn build_link_meta(attrs: &[ast::Attribute], - output: &OutputFilenames, - symbol_hasher: &mut Sha256) - -> LinkMeta { - // This calculates CMH as defined above - fn crate_hash(symbol_hasher: &mut Sha256, crateid: &CrateId) -> ~str { - symbol_hasher.reset(); - symbol_hasher.input_str(crateid.to_str()); - truncated_hash_result(symbol_hasher) - } - - let crateid = match attr::find_crateid(attrs) { +pub fn find_crate_id(attrs: &[ast::Attribute], + output: &OutputFilenames) -> CrateId { + match attr::find_crateid(attrs) { None => from_str(output.out_filestem).unwrap(), Some(s) => s, - }; + } +} - let hash = crate_hash(symbol_hasher, &crateid); +pub fn crate_id_hash(crate_id: &CrateId) -> ~str { + // This calculates CMH as defined above. Note that we don't use the path of + // the crate id in the hash because lookups are only done by (name/vers), + // not by path. + let mut s = Sha256::new(); + s.input_str(crate_id.short_name_with_version()); + truncated_hash_result(&mut s).slice_to(8).to_owned() +} - LinkMeta { - crateid: crateid, - crate_hash: hash, - } +pub fn build_link_meta(krate: &ast::Crate, + output: &OutputFilenames) -> LinkMeta { + let r = LinkMeta { + crateid: find_crate_id(krate.attrs, output), + crate_hash: Svh::calculate(krate), + }; + info!("{}", r); + return r; } fn truncated_hash_result(symbol_hasher: &mut Sha256) -> ~str { @@ -539,7 +542,7 @@ fn symbol_hash(tcx: ty::ctxt, symbol_hasher: &mut Sha256, symbol_hasher.reset(); symbol_hasher.input_str(link_meta.crateid.name); symbol_hasher.input_str("-"); - symbol_hasher.input_str(link_meta.crate_hash); + symbol_hasher.input_str(link_meta.crate_hash.as_str()); symbol_hasher.input_str("-"); symbol_hasher.input_str(encoder::encoded_ty(tcx, t)); let mut hash = truncated_hash_result(symbol_hasher); @@ -712,11 +715,8 @@ pub fn mangle_internal_name_by_path_and_seq(path: PathElems, flav: &str) -> ~str mangle(path.chain(Some(gensym_name(flav)).move_iter()), None, None) } -pub fn output_lib_filename(lm: &LinkMeta) -> ~str { - format!("{}-{}-{}", - lm.crateid.name, - lm.crate_hash.slice_chars(0, 8), - lm.crateid.version_or_default()) +pub fn output_lib_filename(id: &CrateId) -> ~str { + format!("{}-{}-{}", id.name, crate_id_hash(id), id.version_or_default()) } pub fn get_cc_prog(sess: Session) -> ~str { @@ -779,11 +779,11 @@ fn remove(sess: Session, path: &Path) { pub fn link_binary(sess: Session, trans: &CrateTranslation, outputs: &OutputFilenames, - lm: &LinkMeta) -> ~[Path] { + id: &CrateId) -> ~[Path] { let mut out_filenames = ~[]; let crate_types = sess.crate_types.borrow(); for &crate_type in crate_types.get().iter() { - let out_file = link_binary_output(sess, trans, crate_type, outputs, lm); + let out_file = link_binary_output(sess, trans, crate_type, outputs, id); out_filenames.push(out_file); } @@ -807,8 +807,8 @@ fn is_writeable(p: &Path) -> bool { } pub fn filename_for_input(sess: &Session, crate_type: session::CrateType, - lm: &LinkMeta, out_filename: &Path) -> Path { - let libname = output_lib_filename(lm); + id: &CrateId, out_filename: &Path) -> Path { + let libname = output_lib_filename(id); match crate_type { session::CrateTypeRlib => { out_filename.with_filename(format!("lib{}.rlib", libname)) @@ -834,13 +834,13 @@ fn link_binary_output(sess: Session, trans: &CrateTranslation, crate_type: session::CrateType, outputs: &OutputFilenames, - lm: &LinkMeta) -> Path { + id: &CrateId) -> Path { let obj_filename = outputs.temp_path(OutputTypeObject); let out_filename = match outputs.single_output_file { Some(ref file) => file.clone(), None => { let out_filename = outputs.path(OutputTypeExe); - filename_for_input(&sess, crate_type, lm, &out_filename) + filename_for_input(&sess, crate_type, id, &out_filename) } }; diff --git a/src/librustc/back/svh.rs b/src/librustc/back/svh.rs new file mode 100644 index 0000000000000..5f8a12b022a50 --- /dev/null +++ b/src/librustc/back/svh.rs @@ -0,0 +1,112 @@ +// Copyright 2012-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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//! Calculation and management of a Strict Version Hash for crates +//! +//! # Today's ABI problem +//! +//! In today's implementation of rustc, it is incredibly difficult to achieve +//! forward binary compatibility without resorting to C-like interfaces. Within +//! rust code itself, abi details such as symbol names suffer from a variety of +//! unrelated factors to code changing such as the "def id drift" problem. This +//! ends up yielding confusing error messages about metadata mismatches and +//! such. +//! +//! The core of this problem is when when an upstream dependency changes and +//! downstream dependants are not recompiled. This causes compile errors because +//! the upstream crate's metadata has changed but the downstream crates are +//! still referencing the older crate's metadata. +//! +//! This problem exists for many reasons, the primary of which is that rust does +//! not currently support forwards ABI compatibility (in place upgrades of a +//! crate). +//! +//! # SVH and how it alleviates the problem +//! +//! With all of this knowledge on hand, this module contains the implementation +//! of a notion of a "Strict Version Hash" for a crate. This is essentially a +//! hash of all contents of a crate which can somehow be exposed to downstream +//! crates. +//! +//! This hash is currently calculated by just hashing the AST, but this is +//! obviously wrong (doc changes should not result in an incompatible ABI). +//! Implementation-wise, this is required at this moment in time. +//! +//! By encoding this strict version hash into all crate's metadata, stale crates +//! can be detected immediately and error'd about by rustc itself. +//! +//! # Relevant links +//! +//! Original issue: https://github.com/mozilla/rust/issues/10207 + +use std::fmt; +use std::hash::Hash; +use std::hash::sip::SipState; +use std::iter::range_step; +use syntax::ast; + +#[deriving(Clone, Eq)] +pub struct Svh { + priv hash: ~str, +} + +impl Svh { + pub fn new(hash: &str) -> Svh { + assert!(hash.len() == 16); + Svh { hash: hash.to_owned() } + } + + pub fn as_str<'a>(&'a self) -> &'a str { + self.hash.as_slice() + } + + pub fn calculate(krate: &ast::Crate) -> Svh { + // FIXME: see above for why this is wrong, it shouldn't just hash the + // crate. Fixing this would require more in-depth analysis in + // this function about what portions of the crate are reachable + // in tandem with bug fixes throughout the rest of the compiler. + // + // Note that for now we actually exclude some top-level things + // from the crate like the CrateConfig/span. The CrateConfig + // contains command-line `--cfg` flags, so this means that the + // stage1/stage2 AST for libstd and such is different hash-wise + // when it's actually the exact same representation-wise. + // + // As a first stab at only hashing the relevant parts of the + // AST, this only hashes the module/attrs, not the CrateConfig + // field. + // + // FIXME: this should use SHA1, not SipHash. SipHash is not built to + // avoid collisions. + let mut state = SipState::new(); + krate.module.hash(&mut state); + krate.attrs.hash(&mut state); + + let hash = state.result(); + return Svh { + hash: range_step(0, 64, 4).map(|i| hex(hash >> i)).collect() + }; + + fn hex(b: u64) -> char { + let b = (b & 0xf) as u8; + let b = match b { + 0 .. 9 => '0' as u8 + b, + _ => 'a' as u8 + b - 10, + }; + b as char + } + } +} + +impl fmt::Show for Svh { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.pad(self.as_str()) + } +} diff --git a/src/librustc/driver/driver.rs b/src/librustc/driver/driver.rs index 4c552acc93692..d5ee736b6fb7f 100644 --- a/src/librustc/driver/driver.rs +++ b/src/librustc/driver/driver.rs @@ -433,7 +433,7 @@ pub fn phase_6_link_output(sess: Session, link::link_binary(sess, trans, outputs, - &trans.link)); + &trans.link.crateid)); } pub fn stop_after_phase_3(sess: Session) -> bool { @@ -472,8 +472,7 @@ fn write_out_deps(sess: Session, input: &Input, outputs: &OutputFilenames, krate: &ast::Crate) -> io::IoResult<()> { - let lm = link::build_link_meta(krate.attrs, outputs, - &mut ::util::sha2::Sha256::new()); + let id = link::find_crate_id(krate.attrs, outputs); let mut out_filenames = ~[]; for output_type in sess.opts.output_types.iter() { @@ -482,7 +481,7 @@ fn write_out_deps(sess: Session, link::OutputTypeExe => { let crate_types = sess.crate_types.borrow(); for output in crate_types.get().iter() { - let p = link::filename_for_input(&sess, *output, &lm, &file); + let p = link::filename_for_input(&sess, *output, &id, &file); out_filenames.push(p); } } diff --git a/src/librustc/lib.rs b/src/librustc/lib.rs index ff1a6bb7f7efb..2e647e5ca8295 100644 --- a/src/librustc/lib.rs +++ b/src/librustc/lib.rs @@ -55,7 +55,6 @@ use std::str; use std::task; use std::vec; use syntax::ast; -use syntax::attr; use syntax::diagnostic::Emitter; use syntax::diagnostic; use syntax::parse; @@ -104,16 +103,17 @@ pub mod front { } pub mod back { - pub mod archive; - pub mod link; pub mod abi; + pub mod archive; pub mod arm; + pub mod link; + pub mod lto; pub mod mips; - pub mod x86; - pub mod x86_64; pub mod rpath; + pub mod svh; pub mod target_strs; - pub mod lto; + pub mod x86; + pub mod x86_64; } pub mod metadata; @@ -312,28 +312,18 @@ pub fn run_compiler(args: &[~str]) { let attrs = parse_crate_attrs(sess, &input); let t_outputs = d::build_output_filenames(&input, &odir, &ofile, attrs, sess); - if crate_id || crate_name { - let crateid = match attr::find_crateid(attrs) { - Some(crateid) => crateid, - None => { - sess.fatal("No crate_id and --crate-id or \ - --crate-name requested") - } - }; - if crate_id { - println!("{}", crateid.to_str()); - } - if crate_name { - println!("{}", crateid.name); - } - } + let id = link::find_crate_id(attrs, &t_outputs); + if crate_id { + println!("{}", id.to_str()); + } + if crate_name { + println!("{}", id.name); + } if crate_file_name { - let lm = link::build_link_meta(attrs, &t_outputs, - &mut ::util::sha2::Sha256::new()); let crate_types = session::collect_crate_types(&sess, attrs); for &style in crate_types.iter() { - let fname = link::filename_for_input(&sess, style, &lm, + let fname = link::filename_for_input(&sess, style, &id, &t_outputs.with_extension("")); println!("{}", fname.filename_display()); } diff --git a/src/librustc/metadata/common.rs b/src/librustc/metadata/common.rs index 62f1dcedab458..7b7d526411c93 100644 --- a/src/librustc/metadata/common.rs +++ b/src/librustc/metadata/common.rs @@ -12,6 +12,7 @@ use std::cast; use syntax::crateid::CrateId; +use back::svh::Svh; // EBML enum definitions and utils shared by the encoder and decoder @@ -207,8 +208,8 @@ pub static tag_macro_registrar_fn: uint = 0x63; pub static tag_exported_macros: uint = 0x64; pub static tag_macro_def: uint = 0x65; -#[deriving(Clone)] +#[deriving(Clone, Show)] pub struct LinkMeta { crateid: CrateId, - crate_hash: ~str, + crate_hash: Svh, } diff --git a/src/librustc/metadata/creader.rs b/src/librustc/metadata/creader.rs index 1108917cdb1d5..165c1abdeedfa 100644 --- a/src/librustc/metadata/creader.rs +++ b/src/librustc/metadata/creader.rs @@ -12,6 +12,8 @@ //! Validates all used crates and extern libraries and loads their metadata +use back::link; +use back::svh::Svh; use driver::{driver, session}; use driver::session::Session; use metadata::csearch; @@ -78,7 +80,7 @@ impl<'a> visit::Visitor<()> for ReadCrateVisitor<'a> { struct cache_entry { cnum: ast::CrateNum, span: Span, - hash: ~str, + hash: Svh, crate_id: CrateId, } @@ -148,7 +150,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, "", + 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); } @@ -276,12 +278,13 @@ fn visit_item(e: &Env, i: &ast::Item) { } fn existing_match(e: &Env, crate_id: &CrateId, - hash: &str) -> Option { + hash: Option<&Svh>) -> Option { let crate_cache = e.crate_cache.borrow(); for c in crate_cache.get().iter() { - if crate_id.matches(&c.crate_id) && - (hash.is_empty() || hash == c.hash.as_slice()) { - return Some(c.cnum) + if !crate_id.matches(&c.crate_id) { continue } + match hash { + Some(hash) if *hash != c.hash => {} + Some(..) | None => return Some(c.cnum) } } None @@ -291,19 +294,22 @@ fn resolve_crate(e: &mut Env, root_ident: Option<&str>, ident: &str, crate_id: &CrateId, - hash: &str, + hash: Option<&Svh>, span: Span) -> ast::CrateNum { match existing_match(e, crate_id, hash) { None => { - let load_ctxt = loader::Context { + let id_hash = link::crate_id_hash(crate_id); + let mut load_ctxt = loader::Context { sess: e.sess, span: span, ident: ident, crate_id: crate_id, - hash: hash, + id_hash: id_hash, + hash: hash.map(|a| &*a), os: e.os, - intr: e.intr + intr: e.intr, + rejected_via_hash: false, }; let loader::Library { dylib, rlib, metadata @@ -375,7 +381,7 @@ fn resolve_crate_deps(e: &mut Env, let local_cnum = resolve_crate(e, root_ident, dep.crate_id.name.as_slice(), &dep.crate_id, - dep.hash, + Some(&dep.hash), span); cnum_map.insert(extrn_cnum, local_cnum); } @@ -406,7 +412,7 @@ impl CrateLoader for Loader { 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, - &info.crate_id, "", krate.span); + &info.crate_id, None, krate.span); let library = self.env.sess.cstore.get_used_crate_source(cnum).unwrap(); MacroCrate { lib: library.dylib, diff --git a/src/librustc/metadata/cstore.rs b/src/librustc/metadata/cstore.rs index 12461ddbe7154..baca85d50ae97 100644 --- a/src/librustc/metadata/cstore.rs +++ b/src/librustc/metadata/cstore.rs @@ -13,6 +13,7 @@ // The crate store - a central repo for information collected about external // crates and libraries +use back::svh::Svh; use metadata::decoder; use metadata::loader; @@ -92,7 +93,7 @@ impl CStore { *metas.get().get(&cnum) } - pub fn get_crate_hash(&self, cnum: ast::CrateNum) -> ~str { + pub fn get_crate_hash(&self, cnum: ast::CrateNum) -> Svh { let cdata = self.get_crate_data(cnum); decoder::get_crate_hash(cdata.data()) } diff --git a/src/librustc/metadata/decoder.rs b/src/librustc/metadata/decoder.rs index f365ddef94f54..42754aedba704 100644 --- a/src/librustc/metadata/decoder.rs +++ b/src/librustc/metadata/decoder.rs @@ -12,6 +12,7 @@ #[allow(non_camel_case_types)]; +use back::svh::Svh; use metadata::cstore::crate_metadata; use metadata::common::*; use metadata::csearch::StaticMethodInfo; @@ -1089,9 +1090,9 @@ fn get_attributes(md: ebml::Doc) -> ~[ast::Attribute] { return attrs; } -fn list_crate_attributes(md: ebml::Doc, hash: &str, +fn list_crate_attributes(md: ebml::Doc, hash: &Svh, out: &mut io::Writer) -> io::IoResult<()> { - try!(write!(out, "=Crate Attributes ({})=\n", hash)); + try!(write!(out, "=Crate Attributes ({})=\n", *hash)); let r = get_attributes(md); for attr in r.iter() { @@ -1109,7 +1110,7 @@ pub fn get_crate_attributes(data: &[u8]) -> ~[ast::Attribute] { pub struct CrateDep { cnum: ast::CrateNum, crate_id: CrateId, - hash: ~str, + hash: Svh, } pub fn get_crate_deps(data: &[u8]) -> ~[CrateDep] { @@ -1123,7 +1124,7 @@ pub fn get_crate_deps(data: &[u8]) -> ~[CrateDep] { } reader::tagged_docs(depsdoc, tag_crate_dep, |depdoc| { let crate_id = from_str(docstr(depdoc, tag_crate_dep_crateid)).unwrap(); - let hash = docstr(depdoc, tag_crate_dep_hash); + let hash = Svh::new(docstr(depdoc, tag_crate_dep_hash)); deps.push(CrateDep { cnum: crate_num, crate_id: crate_id, @@ -1144,10 +1145,10 @@ fn list_crate_deps(data: &[u8], out: &mut io::Writer) -> io::IoResult<()> { Ok(()) } -pub fn get_crate_hash(data: &[u8]) -> ~str { +pub fn get_crate_hash(data: &[u8]) -> Svh { let cratedoc = reader::Doc(data); let hashdoc = reader::get_doc(cratedoc, tag_crate_hash); - hashdoc.as_str_slice().to_str() + Svh::new(hashdoc.as_str_slice()) } pub fn get_crate_id(data: &[u8]) -> CrateId { @@ -1159,7 +1160,7 @@ pub fn get_crate_id(data: &[u8]) -> CrateId { pub fn list_crate_metadata(bytes: &[u8], out: &mut io::Writer) -> io::IoResult<()> { let hash = get_crate_hash(bytes); let md = reader::Doc(bytes); - try!(list_crate_attributes(md, hash, out)); + try!(list_crate_attributes(md, &hash, out)); list_crate_deps(bytes, out) } diff --git a/src/librustc/metadata/encoder.rs b/src/librustc/metadata/encoder.rs index 976c1ee92d3de..5bcc113ef946b 100644 --- a/src/librustc/metadata/encoder.rs +++ b/src/librustc/metadata/encoder.rs @@ -13,6 +13,7 @@ #[allow(unused_must_use)]; // everything is just a MemWriter, can't fail #[allow(non_camel_case_types)]; +use back::svh::Svh; use metadata::common::*; use metadata::cstore; use metadata::decoder; @@ -1733,14 +1734,14 @@ fn encode_crate_dep(ebml_w: &mut writer::Encoder, ebml_w.writer.write(dep.crate_id.to_str().as_bytes()); ebml_w.end_tag(); ebml_w.start_tag(tag_crate_dep_hash); - ebml_w.writer.write(dep.hash.as_bytes()); + ebml_w.writer.write(dep.hash.as_str().as_bytes()); ebml_w.end_tag(); ebml_w.end_tag(); } -fn encode_hash(ebml_w: &mut writer::Encoder, hash: &str) { +fn encode_hash(ebml_w: &mut writer::Encoder, hash: &Svh) { ebml_w.start_tag(tag_crate_hash); - ebml_w.writer.write(hash.as_bytes()); + ebml_w.writer.write(hash.as_str().as_bytes()); ebml_w.end_tag(); } @@ -1809,7 +1810,7 @@ fn encode_metadata_inner(wr: &mut MemWriter, parms: EncodeParams, krate: &Crate) let mut ebml_w = writer::Encoder(wr); encode_crate_id(&mut ebml_w, &ecx.link_meta.crateid); - encode_hash(&mut ebml_w, ecx.link_meta.crate_hash); + encode_hash(&mut ebml_w, &ecx.link_meta.crate_hash); let mut i = ebml_w.writer.tell().unwrap(); let crate_attrs = synthesize_crate_attrs(&ecx, krate); diff --git a/src/librustc/metadata/loader.rs b/src/librustc/metadata/loader.rs index faef2412e7802..9c61191ff9916 100644 --- a/src/librustc/metadata/loader.rs +++ b/src/librustc/metadata/loader.rs @@ -11,6 +11,7 @@ //! Finds crate binaries and loads their metadata use back::archive::{ArchiveRO, METADATA_FILENAME}; +use back::svh::Svh; use driver::session::Session; use lib::llvm::{False, llvm, ObjectFile, mk_section_iter}; use metadata::cstore::{MetadataBlob, MetadataVec, MetadataArchive}; @@ -21,7 +22,6 @@ use syntax::codemap::Span; use syntax::diagnostic::SpanHandler; use syntax::parse::token::IdentInterner; use syntax::crateid::CrateId; -use syntax::attr; use syntax::attr::AttrMetaMethods; use std::c_str::ToCStr; @@ -49,9 +49,11 @@ pub struct Context<'a> { span: Span, ident: &'a str, crate_id: &'a CrateId, - hash: &'a str, + id_hash: &'a str, + hash: Option<&'a Svh>, os: Os, - intr: @IdentInterner + intr: @IdentInterner, + rejected_via_hash: bool, } pub struct Library { @@ -79,23 +81,34 @@ fn realpath(p: &Path) -> Path { } impl<'a> Context<'a> { - pub fn load_library_crate(&self, root_ident: Option<&str>) -> Library { + pub fn load_library_crate(&mut self, root_ident: Option<&str>) -> Library { match self.find_library_crate() { Some(t) => t, None => { self.sess.abort_if_errors(); + let message = if self.rejected_via_hash { + format!("found possibly newer version of crate `{}`", + self.ident) + } else { + format!("can't find crate for `{}`", self.ident) + }; let message = match root_ident { - None => format!("can't find crate for `{}`", self.ident), - Some(c) => format!("can't find crate for `{}` which `{}` depends on", - self.ident, - c) + None => message, + Some(c) => format!("{} which `{}` depends on", message, c), }; - self.sess.span_fatal(self.span, message); + self.sess.span_err(self.span, message); + + if self.rejected_via_hash { + self.sess.span_note(self.span, "perhaps this crate needs \ + to be recompiled?"); + } + self.sess.abort_if_errors(); + unreachable!() } } } - fn find_library_crate(&self) -> Option { + fn find_library_crate(&mut self) -> Option { let filesearch = self.sess.filesearch; let (dyprefix, dysuffix) = self.dylibname(); @@ -212,13 +225,8 @@ impl<'a> Context<'a> { None => {} } let data = lib.metadata.as_slice(); - let attrs = decoder::get_crate_attributes(data); - match attr::find_crateid(attrs) { - None => {} - Some(crateid) => { - note_crateid_attr(self.sess.diagnostic(), &crateid); - } - } + let crate_id = decoder::get_crate_id(data); + note_crateid_attr(self.sess.diagnostic(), &crate_id); } None } @@ -240,18 +248,21 @@ impl<'a> Context<'a> { debug!("matching -- {}, middle: {}", file, middle); let mut parts = middle.splitn('-', 1); let hash = match parts.next() { Some(h) => h, None => return None }; - debug!("matching -- {}, hash: {}", file, hash); + debug!("matching -- {}, hash: {} (want {})", file, hash, self.id_hash); let vers = match parts.next() { Some(v) => v, None => return None }; - debug!("matching -- {}, vers: {}", file, vers); + debug!("matching -- {}, vers: {} (want {})", file, vers, + self.crate_id.version); match self.crate_id.version { Some(ref version) if version.as_slice() != vers => return None, - Some(..) | None => {} + Some(..) => {} // check the hash + + // hash is irrelevant, no version specified + None => return Some(hash.to_owned()) } - debug!("matching -- {}, vers ok (requested {})", file, - self.crate_id.version); + debug!("matching -- {}, vers ok", file); // hashes in filenames are prefixes of the "true hash" - if self.hash.is_empty() || self.hash.starts_with(hash) { - debug!("matching -- {}, hash ok (requested {})", file, self.hash); + if self.id_hash == hash.as_slice() { + debug!("matching -- {}, hash ok", file); Some(hash.to_owned()) } else { None @@ -270,7 +281,7 @@ impl<'a> Context<'a> { // FIXME(#10786): for an optimization, we only read one of the library's // metadata sections. In theory we should read both, but // reading dylib metadata is quite slow. - fn extract_one(&self, m: HashSet, flavor: &str, + fn extract_one(&mut self, m: HashSet, flavor: &str, slot: &mut Option) -> Option { if m.len() == 0 { return None } if m.len() > 1 { @@ -290,7 +301,7 @@ impl<'a> Context<'a> { info!("{} reading meatadata from: {}", flavor, lib.display()); match get_metadata_section(self.os, &lib) { Some(blob) => { - if crate_matches(blob.as_slice(), self.crate_id, self.hash){ + if self.crate_matches(blob.as_slice()) { *slot = Some(blob); } else { info!("metadata mismatch"); @@ -306,6 +317,22 @@ impl<'a> Context<'a> { return Some(lib); } + fn crate_matches(&mut self, crate_data: &[u8]) -> bool { + let other_id = decoder::get_crate_id(crate_data); + if !self.crate_id.matches(&other_id) { return false } + match self.hash { + None => true, + Some(hash) => { + if *hash != decoder::get_crate_hash(crate_data) { + self.rejected_via_hash = true; + false + } else { + true + } + } + } + } + // Returns the corresponding (prefix, suffix) that files need to have for // dynamic libraries fn dylibname(&self) -> (&'static str, &'static str) { @@ -323,15 +350,6 @@ pub fn note_crateid_attr(diag: @SpanHandler, crateid: &CrateId) { diag.handler().note(format!("crate_id: {}", crateid.to_str())); } -fn crate_matches(crate_data: &[u8], crate_id: &CrateId, hash: &str) -> bool { - let other_id = decoder::get_crate_id(crate_data); - if !crate_id.matches(&other_id) { return false } - if hash != "" && hash != decoder::get_crate_hash(crate_data).as_slice() { - return false - } - return true; -} - impl ArchiveMetadata { fn new(ar: ArchiveRO) -> Option { let data: &'static [u8] = { diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 553dbe2ae6e36..683246f3333be 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -2453,7 +2453,8 @@ pub fn decl_crate_map(sess: session::Session, mapmeta: LinkMeta, let sym_name = if is_top { ~"_rust_crate_map_toplevel" } else { - symname("_rust_crate_map_" + mapmeta.crateid.name, mapmeta.crate_hash, + symname("_rust_crate_map_" + mapmeta.crateid.name, + mapmeta.crate_hash.as_str(), mapmeta.crateid.version_or_default()) }; @@ -2487,7 +2488,7 @@ pub fn fill_crate_map(ccx: @CrateContext, map: ValueRef) { while cstore.have_crate_data(i) { let cdata = cstore.get_crate_data(i); let nm = symname(format!("_rust_crate_map_{}", cdata.name), - cstore.get_crate_hash(i), + cstore.get_crate_hash(i).as_str(), cstore.get_crate_id(i).version_or_default()); let cr = nm.with_c_str(|buf| { unsafe { @@ -2609,9 +2610,7 @@ pub fn trans_crate(sess: session::Session, } } - let mut symbol_hasher = Sha256::new(); - let link_meta = link::build_link_meta(krate.attrs, output, - &mut symbol_hasher); + let link_meta = link::build_link_meta(&krate, output); // Append ".rs" to crate name as LLVM module identifier. // @@ -2621,16 +2620,16 @@ pub fn trans_crate(sess: session::Session, // crashes if the module identifer is same as other symbols // such as a function name in the module. // 1. http://llvm.org/bugs/show_bug.cgi?id=11479 - let llmod_id = link_meta.crateid.name.clone() + ".rs"; + let llmod_id = link_meta.crateid.name + ".rs"; let ccx = @CrateContext::new(sess, - llmod_id, - analysis.ty_cx, - analysis.exp_map2, - analysis.maps, - symbol_hasher, - link_meta, - analysis.reachable); + llmod_id, + analysis.ty_cx, + analysis.exp_map2, + analysis.maps, + Sha256::new(), + link_meta, + analysis.reachable); { let _icx = push_ctxt("text"); trans_mod(ccx, &krate.module); diff --git a/src/librustc/middle/trans/intrinsic.rs b/src/librustc/middle/trans/intrinsic.rs index 4abc114fef667..8350b24c451c9 100644 --- a/src/librustc/middle/trans/intrinsic.rs +++ b/src/librustc/middle/trans/intrinsic.rs @@ -329,7 +329,7 @@ pub fn trans_intrinsic(ccx: @CrateContext, let hash = ty::hash_crate_independent( ccx.tcx, substs.tys[0], - ccx.link_meta.crate_hash.clone()); + &ccx.link_meta.crate_hash); // NB: This needs to be kept in lockstep with the TypeId struct in // libstd/unstable/intrinsics.rs let val = C_named_struct(type_of::type_of(ccx, output_type), [C_u64(hash)]); diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 6b26e268057af..2ca19f0b61d82 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -10,6 +10,7 @@ #[allow(non_camel_case_types)]; +use back::svh::Svh; use driver::session; use metadata::csearch; use metadata; @@ -4882,7 +4883,7 @@ pub fn trait_method_of_method(tcx: ctxt, /// Creates a hash of the type `t` which will be the same no matter what crate /// context it's calculated within. This is used by the `type_id` intrinsic. -pub fn hash_crate_independent(tcx: ctxt, t: t, local_hash: ~str) -> u64 { +pub fn hash_crate_independent(tcx: ctxt, t: t, svh: &Svh) -> u64 { let mut state = sip::SipState::new(); macro_rules! byte( ($b:expr) => { ($b as u8).hash(&mut state) } ); macro_rules! hash( ($e:expr) => { $e.hash(&mut state) } ); @@ -4913,11 +4914,11 @@ pub fn hash_crate_independent(tcx: ctxt, t: t, local_hash: ~str) -> u64 { }; let did = |state: &mut sip::SipState, did: DefId| { let h = if ast_util::is_local(did) { - local_hash.clone() + svh.clone() } else { tcx.sess.cstore.get_crate_hash(did.krate) }; - h.as_bytes().hash(state); + h.as_str().hash(state); did.node.hash(state); }; let mt = |state: &mut sip::SipState, mt: mt| { diff --git a/src/test/auxiliary/changing-crates-a1.rs b/src/test/auxiliary/changing-crates-a1.rs new file mode 100644 index 0000000000000..c0bdbf81772ac --- /dev/null +++ b/src/test/auxiliary/changing-crates-a1.rs @@ -0,0 +1,13 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[crate_id = "a"]; + +pub fn foo() {} diff --git a/src/test/auxiliary/changing-crates-a2.rs b/src/test/auxiliary/changing-crates-a2.rs new file mode 100644 index 0000000000000..cc123c0f65ddf --- /dev/null +++ b/src/test/auxiliary/changing-crates-a2.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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[crate_id = "a"]; + +pub fn foo() { println!("hello!"); } + diff --git a/src/test/auxiliary/changing-crates-b.rs b/src/test/auxiliary/changing-crates-b.rs new file mode 100644 index 0000000000000..9b80583bb84a0 --- /dev/null +++ b/src/test/auxiliary/changing-crates-b.rs @@ -0,0 +1,15 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[crate_id = "b"]; + +extern crate a; + +pub fn foo() { a::foo::(); } diff --git a/src/test/compile-fail/bad-crate-id.rs b/src/test/compile-fail/bad-crate-id.rs new file mode 100644 index 0000000000000..43956752cd9b3 --- /dev/null +++ b/src/test/compile-fail/bad-crate-id.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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +extern crate foo = ""; //~ ERROR: malformed crate id +extern crate bar = "#a"; //~ ERROR: malformed crate id + +fn main() {} diff --git a/src/test/compile-fail/changing-crates.rs b/src/test/compile-fail/changing-crates.rs new file mode 100644 index 0000000000000..ae3ef76066710 --- /dev/null +++ b/src/test/compile-fail/changing-crates.rs @@ -0,0 +1,20 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// note that these aux-build directives must be in this order +// aux-build:changing-crates-a1.rs +// aux-build:changing-crates-b.rs +// aux-build:changing-crates-a2.rs + +extern crate a; +extern crate b; //~ ERROR: found possibly newer version of crate `a` which `b` depends on +//~^ NOTE: perhaps this crate needs to be recompiled + +fn main() {} From 017c5044895a3f708dee46d64dd3d67dac61145e Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 27 Feb 2014 17:07:27 -0800 Subject: [PATCH 3/3] syntax: Expand format!() deterministically Previously, format!("{a}{b}", a=foo(), b=bar()) has foo() and bar() run in a nondeterminisc order. This is clearly a non-desirable property, so this commit uses iteration over a list instead of iteration over a hash map to provide deterministic code generation of these format arguments. --- src/libsyntax/ext/deriving/show.rs | 3 ++- src/libsyntax/ext/format.rs | 36 ++++++++++++++++++++---------- src/test/run-pass/ifmt.rs | 16 +++++++++++++ 3 files changed, 42 insertions(+), 13 deletions(-) diff --git a/src/libsyntax/ext/deriving/show.rs b/src/libsyntax/ext/deriving/show.rs index 5286720b9fc08..4b9925c8d9f3e 100644 --- a/src/libsyntax/ext/deriving/show.rs +++ b/src/libsyntax/ext/deriving/show.rs @@ -135,5 +135,6 @@ fn show_substructure(cx: &mut ExtCtxt, span: Span, // phew, not our responsibility any more! format::expand_preparsed_format_args(cx, span, format_closure, - format_string, exprs, HashMap::new()) + format_string, exprs, ~[], + HashMap::new()) } diff --git a/src/libsyntax/ext/format.rs b/src/libsyntax/ext/format.rs index 1b73d42c79aa4..2642ee00458c1 100644 --- a/src/libsyntax/ext/format.rs +++ b/src/libsyntax/ext/format.rs @@ -43,9 +43,13 @@ struct Context<'a> { // them. args: ~[@ast::Expr], arg_types: ~[Option], - // Parsed named expressions and the types that we've found for them so far + // Parsed named expressions and the types that we've found for them so far. + // Note that we keep a side-array of the ordering of the named arguments + // found to be sure that we can translate them in the same order that they + // were declared in. names: HashMap<~str, @ast::Expr>, name_types: HashMap<~str, ArgumentType>, + name_ordering: ~[~str], // Collection of the compiled `rt::Piece` structures pieces: ~[@ast::Expr], @@ -63,12 +67,15 @@ struct Context<'a> { /// /// If parsing succeeds, the second return value is: /// -/// Some((fmtstr, unnamed arguments, named arguments)) -fn parse_args(ecx: &mut ExtCtxt, sp: Span, - tts: &[ast::TokenTree]) -> (@ast::Expr, Option<(@ast::Expr, ~[@ast::Expr], - HashMap<~str, @ast::Expr>)>) { +/// Some((fmtstr, unnamed arguments, ordering of named arguments, +/// named arguments)) +fn parse_args(ecx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree]) + -> (@ast::Expr, Option<(@ast::Expr, ~[@ast::Expr], ~[~str], + HashMap<~str, @ast::Expr>)>) +{ let mut args = ~[]; let mut names = HashMap::<~str, @ast::Expr>::new(); + let mut order = ~[]; let mut p = rsparse::new_parser_from_tts(ecx.parse_sess(), ecx.cfg(), @@ -125,12 +132,13 @@ fn parse_args(ecx: &mut ExtCtxt, sp: Span, continue } } + order.push(name.to_str()); names.insert(name.to_str(), e); } else { args.push(p.parse_expr()); } } - return (extra, Some((fmtstr, args, names))); + return (extra, Some((fmtstr, args, order, names))); } impl<'a> Context<'a> { @@ -661,10 +669,11 @@ impl<'a> Context<'a> { locals.push(self.format_arg(e.span, Exact(i), self.ecx.expr_ident(e.span, name))); } - for (name, &e) in self.names.iter() { - if !self.name_types.contains_key(name) { - continue - } + for name in self.name_ordering.iter() { + let e = match self.names.find(name) { + Some(&e) if self.name_types.contains_key(name) => e, + Some(..) | None => continue + }; let lname = self.ecx.ident_of(format!("__arg{}", *name)); pats.push(self.ecx.pat_ident(e.span, lname)); @@ -810,8 +819,9 @@ pub fn expand_args(ecx: &mut ExtCtxt, sp: Span, tts: &[ast::TokenTree]) -> base::MacResult { match parse_args(ecx, sp, tts) { - (extra, Some((efmt, args, names))) => { - MRExpr(expand_preparsed_format_args(ecx, sp, extra, efmt, args, names)) + (extra, Some((efmt, args, order, names))) => { + MRExpr(expand_preparsed_format_args(ecx, sp, extra, efmt, args, + order, names)) } (_, None) => MRExpr(ecx.expr_uint(sp, 2)) } @@ -823,6 +833,7 @@ pub fn expand_args(ecx: &mut ExtCtxt, sp: Span, pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt, sp: Span, extra: @ast::Expr, efmt: @ast::Expr, args: ~[@ast::Expr], + name_ordering: ~[~str], names: HashMap<~str, @ast::Expr>) -> @ast::Expr { let arg_types = vec::from_fn(args.len(), |_| None); let mut cx = Context { @@ -832,6 +843,7 @@ pub fn expand_preparsed_format_args(ecx: &mut ExtCtxt, sp: Span, names: names, name_positions: HashMap::new(), name_types: HashMap::new(), + name_ordering: name_ordering, nest_level: 0, next_arg: 0, pieces: ~[], diff --git a/src/test/run-pass/ifmt.rs b/src/test/run-pass/ifmt.rs index ca798a77a4fd1..564f7b4342639 100644 --- a/src/test/run-pass/ifmt.rs +++ b/src/test/run-pass/ifmt.rs @@ -139,6 +139,7 @@ pub fn main() { test_write(); test_print(); + test_order(); // make sure that format! doesn't move out of local variables let a = ~3; @@ -202,3 +203,18 @@ fn test_format_args() { let s = format_args!(fmt::format, "hello {}", "world"); t!(s, "hello world"); } + +fn test_order() { + // Make sure format!() arguments are always evaluated in a left-to-right + // ordering + fn foo() -> int { + static mut FOO: int = 0; + unsafe { + FOO += 1; + FOO + } + } + assert_eq!(format!("{} {} {a} {b} {} {c}", + foo(), foo(), foo(), a=foo(), b=foo(), c=foo()), + ~"1 2 4 5 3 6"); +}