Auto merge of #31539 - michaelwoerister:stable-symbols, r=nikomatsakis

WIP: Implement stable symbol-name generation algorithm.

This PR changes the way symbol names are generated by the compiler. The new algorithm reflects the current state of the discussion over at rust-lang/rfcs#689.

Once it is done, it will also fix issue #30330. I want to add a test case for that before closing it though.

I also want to do some performance tests. The new algorithm does a little more work than the previous one due to various reasons, and it might make sense to adapt it in a way that allows it to be implemented more efficiently.

@nikomatsakis: It would be nice if there was a way of finding out if a `DefPath` refers to something in the current crate or in an external one. The information is already there, it's just not accessible at the moment. I'll probably propose some minor changes there, together with some facilities to allow for accessing `DefPaths` without allocating a `Vec` for them.

**TODO**
 - ~~Actually "crate qualify" symbols, as promised in the docs.~~
 - ~~Add a test case showing that symbol names are deterministic~~.
 - Maybe add a test case showing that symbol names are stable against small code changes.

~~One thing that might be interesting to the @rust-lang/compiler team:
I've used SipHash exclusively now for generating symbol hashes. Previously it was only used for monomorphizations and the rest of the code used a truncated version on SHA256. Is there any benefit to sticking to SHA? I don't really see one since we only used 64 bits of the digest anyway, but maybe I'm missing something?~~ ==> Just switched things back to SHA-2 for now.

Author: bors

mk/tests.mk

@@ -383,7 +383,7 @@ $(3)/stage$(1)/test/$(4)test-$(2)$$(X_$(2)): \
 	@$$(call E, rustc: $$@)
 	$(Q)CFG_LLVM_LINKAGE_FILE=$$(LLVM_LINKAGE_PATH_$(2)) \
 	    $$(subst @,,$$(STAGE$(1)_T_$(2)_H_$(3))) -o $$@ $$< --test \
-		-L "$$(RT_OUTPUT_DIR_$(2))" \
+		-Cmetadata="test-crate" -L "$$(RT_OUTPUT_DIR_$(2))" \
 		$$(LLVM_LIBDIR_RUSTFLAGS_$(2)) \
 		$$(RUSTFLAGS_$(4))
 

src/compiletest/common.rs

@@ -69,10 +69,10 @@ impl fmt::Display for Mode {
 #[derive(Clone)]
 pub struct Config {
     // The library paths required for running the compiler
-    pub compile_lib_path: String,
+    pub compile_lib_path: PathBuf,
 
     // The library paths required for running compiled programs
-    pub run_lib_path: String,
+    pub run_lib_path: PathBuf,
 
     // The rustc executable
     pub rustc_path: PathBuf,

src/compiletest/compiletest.rs

@@ -116,9 +116,23 @@ pub fn parse_config(args: Vec<String> ) -> Config {
         }
     }
 
+    fn make_absolute(path: PathBuf) -> PathBuf {
+        if path.is_relative() {
+            env::current_dir().unwrap().join(path)
+        } else {
+            path
+        }
+    }
+
+    let filter = if !matches.free.is_empty() {
+        Some(matches.free[0].clone())
+    } else {
+        None
+    };
+
     Config {
-        compile_lib_path: matches.opt_str("compile-lib-path").unwrap(),
-        run_lib_path: matches.opt_str("run-lib-path").unwrap(),
+        compile_lib_path: make_absolute(opt_path(matches, "compile-lib-path")),
+        run_lib_path: make_absolute(opt_path(matches, "run-lib-path")),
         rustc_path: opt_path(matches, "rustc-path"),
         rustdoc_path: opt_path(matches, "rustdoc-path"),
         python: matches.opt_str("python").unwrap(),

src/compiletest/runtest.rs

@@ -316,7 +316,7 @@ fn run_pretty_test_revision(config: &Config,
                                      testpaths,
                                      pretty_type.to_owned()),
                         props.exec_env.clone(),
-                        &config.compile_lib_path,
+                        config.compile_lib_path.to_str().unwrap(),
                         Some(aux_dir.to_str().unwrap()),
                         Some(src))
     }
@@ -635,7 +635,7 @@ fn run_debuginfo_gdb_test(config: &Config, props: &TestProps, testpaths: &TestPa
                                                   testpaths,
                                                   proc_args,
                                                   environment,
-                                                  &config.run_lib_path,
+                                                  config.run_lib_path.to_str().unwrap(),
                                                   None,
                                                   None);
         }
@@ -1292,7 +1292,7 @@ fn exec_compiled_test(config: &Config, props: &TestProps,
                             testpaths,
                             make_run_args(config, props, testpaths),
                             env,
-                            &config.run_lib_path,
+                            config.run_lib_path.to_str().unwrap(),
                             Some(aux_dir.to_str().unwrap()),
                             None)
         }
@@ -1364,7 +1364,7 @@ fn compose_and_run_compiler(config: &Config, props: &TestProps,
                                      &aux_testpaths,
                                      aux_args,
                                      Vec::new(),
-                                     &config.compile_lib_path,
+                                     config.compile_lib_path.to_str().unwrap(),
                                      Some(aux_dir.to_str().unwrap()),
                                      None);
         if !auxres.status.success() {
@@ -1387,7 +1387,7 @@ fn compose_and_run_compiler(config: &Config, props: &TestProps,
                     testpaths,
                     args,
                     Vec::new(),
-                    &config.compile_lib_path,
+                    config.compile_lib_path.to_str().unwrap(),
                     Some(aux_dir.to_str().unwrap()),
                     input)
 }

src/librbml/lib.rs

@@ -165,15 +165,15 @@ impl<'doc> Doc<'doc> {
         }
     }
 
-    pub fn get<'a>(&'a self, tag: usize) -> Doc<'a> {
+    pub fn get(&self, tag: usize) -> Doc<'doc> {
         reader::get_doc(*self, tag)
     }
 
     pub fn is_empty(&self) -> bool {
         self.start == self.end
     }
 
-    pub fn as_str_slice<'a>(&'a self) -> &'a str {
+    pub fn as_str_slice(&self) -> &'doc str {
         str::from_utf8(&self.data[self.start..self.end]).unwrap()
     }
 

src/librustc/middle/cstore.rs

@@ -42,6 +42,7 @@ use syntax::ast_util::{IdVisitingOperation};
 use syntax::attr;
 use syntax::codemap::Span;
 use syntax::ptr::P;
+use syntax::parse::token::InternedString;
 use rustc_back::target::Target;
 use rustc_front::hir;
 use rustc_front::intravisit::Visitor;
@@ -203,8 +204,13 @@ pub trait CrateStore<'tcx> : Any {
     fn is_explicitly_linked(&self, cnum: ast::CrateNum) -> bool;
     fn is_allocator(&self, cnum: ast::CrateNum) -> bool;
     fn crate_attrs(&self, cnum: ast::CrateNum) -> Vec<ast::Attribute>;
-    fn crate_name(&self, cnum: ast::CrateNum) -> String;
+    /// The name of the crate as it is referred to in source code of the current
+    /// crate.
+    fn crate_name(&self, cnum: ast::CrateNum) -> InternedString;
+    /// The name of the crate as it is stored in the crate's metadata.
+    fn original_crate_name(&self, cnum: ast::CrateNum) -> InternedString;
     fn crate_hash(&self, cnum: ast::CrateNum) -> Svh;
+    fn crate_disambiguator(&self, cnum: ast::CrateNum) -> InternedString;
     fn crate_struct_field_attrs(&self, cnum: ast::CrateNum)
                                 -> FnvHashMap<DefId, Vec<ast::Attribute>>;
     fn plugin_registrar_fn(&self, cnum: ast::CrateNum) -> Option<DefId>;
@@ -236,7 +242,11 @@ pub trait CrateStore<'tcx> : Any {
     // utility functions
     fn metadata_filename(&self) -> &str;
     fn metadata_section_name(&self, target: &Target) -> &str;
-    fn encode_type(&self, tcx: &TyCtxt<'tcx>, ty: Ty<'tcx>) -> Vec<u8>;
+    fn encode_type(&self,
+                   tcx: &TyCtxt<'tcx>,
+                   ty: Ty<'tcx>,
+                   def_id_to_string: fn(&TyCtxt<'tcx>, DefId) -> String)
+                   -> Vec<u8>;
     fn used_crates(&self, prefer: LinkagePreference) -> Vec<(ast::CrateNum, Option<PathBuf>)>;
     fn used_crate_source(&self, cnum: ast::CrateNum) -> CrateSource;
     fn extern_mod_stmt_cnum(&self, emod_id: ast::NodeId) -> Option<ast::CrateNum>;
@@ -378,8 +388,12 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore {
     fn is_allocator(&self, cnum: ast::CrateNum) -> bool { unimplemented!() }
     fn crate_attrs(&self, cnum: ast::CrateNum) -> Vec<ast::Attribute>
         { unimplemented!() }
-    fn crate_name(&self, cnum: ast::CrateNum) -> String { unimplemented!() }
+    fn crate_name(&self, cnum: ast::CrateNum) -> InternedString { unimplemented!() }
+    fn original_crate_name(&self, cnum: ast::CrateNum) -> InternedString {
+        unimplemented!()
+    }
     fn crate_hash(&self, cnum: ast::CrateNum) -> Svh { unimplemented!() }
+    fn crate_disambiguator(&self, cnum: ast::CrateNum) -> InternedString { unimplemented!() }
     fn crate_struct_field_attrs(&self, cnum: ast::CrateNum)
                                 -> FnvHashMap<DefId, Vec<ast::Attribute>>
         { unimplemented!() }
@@ -419,8 +433,13 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore {
     // utility functions
     fn metadata_filename(&self) -> &str { unimplemented!() }
     fn metadata_section_name(&self, target: &Target) -> &str { unimplemented!() }
-    fn encode_type(&self, tcx: &TyCtxt<'tcx>, ty: Ty<'tcx>) -> Vec<u8>
-        { unimplemented!() }
+    fn encode_type(&self,
+                   tcx: &TyCtxt<'tcx>,
+                   ty: Ty<'tcx>,
+                   def_id_to_string: fn(&TyCtxt<'tcx>, DefId) -> String)
+                   -> Vec<u8> {
+        unimplemented!()
+    }
     fn used_crates(&self, prefer: LinkagePreference) -> Vec<(ast::CrateNum, Option<PathBuf>)>
         { vec![] }
     fn used_crate_source(&self, cnum: ast::CrateNum) -> CrateSource { unimplemented!() }

src/librustc/middle/ty/context.rs

@@ -43,7 +43,7 @@ use std::hash::{Hash, Hasher};
 use std::rc::Rc;
 use syntax::ast::{self, Name, NodeId};
 use syntax::attr;
-use syntax::parse::token::special_idents;
+use syntax::parse::token::{self, special_idents};
 
 use rustc_front::hir;
 
@@ -415,6 +415,10 @@ pub struct TyCtxt<'tcx> {
     /// fragmented data to the set of unfragmented pieces that
     /// constitute it.
     pub fragment_infos: RefCell<DefIdMap<Vec<ty::FragmentInfo>>>,
+
+    /// The definite name of the current crate after taking into account
+    /// attributes, commandline parameters, etc.
+    pub crate_name: token::InternedString,
 }
 
 impl<'tcx> TyCtxt<'tcx> {
@@ -511,6 +515,7 @@ impl<'tcx> TyCtxt<'tcx> {
                                  region_maps: RegionMaps,
                                  lang_items: middle::lang_items::LanguageItems,
                                  stability: stability::Index<'tcx>,
+                                 crate_name: &str,
                                  f: F) -> R
                                  where F: FnOnce(&TyCtxt<'tcx>) -> R
     {
@@ -570,7 +575,8 @@ impl<'tcx> TyCtxt<'tcx> {
             const_qualif_map: RefCell::new(NodeMap()),
             custom_coerce_unsized_kinds: RefCell::new(DefIdMap()),
             cast_kinds: RefCell::new(NodeMap()),
-            fragment_infos: RefCell::new(DefIdMap())
+            fragment_infos: RefCell::new(DefIdMap()),
+            crate_name: token::intern_and_get_ident(crate_name),
        }, f)
     }
 }

src/librustc/session/mod.rs

@@ -64,7 +64,12 @@ pub struct Session {
     pub plugin_attributes: RefCell<Vec<(String, AttributeType)>>,
     pub crate_types: RefCell<Vec<config::CrateType>>,
     pub dependency_formats: RefCell<dependency_format::Dependencies>,
-    pub crate_metadata: RefCell<Vec<String>>,
+    // The crate_disambiguator is constructed out of all the `-C metadata`
+    // arguments passed to the compiler. Its value together with the crate-name
+    // forms a unique global identifier for the crate. It is used to allow
+    // multiple crates with the same name to coexist. See the
+    // trans::back::symbol_names module for more information.
+    pub crate_disambiguator: RefCell<String>,
     pub features: RefCell<feature_gate::Features>,
 
     /// The maximum recursion limit for potentially infinitely recursive
@@ -481,7 +486,7 @@ pub fn build_session_(sopts: config::Options,
         plugin_attributes: RefCell::new(Vec::new()),
         crate_types: RefCell::new(Vec::new()),
         dependency_formats: RefCell::new(FnvHashMap()),
-        crate_metadata: RefCell::new(Vec::new()),
+        crate_disambiguator: RefCell::new(String::new()),
         features: RefCell::new(feature_gate::Features::new()),
         recursion_limit: Cell::new(64),
         next_node_id: Cell::new(1),

src/librustc_back/svh.rs

@@ -66,7 +66,7 @@ impl Svh {
         &self.hash
     }
 
-    pub fn calculate(metadata: &Vec<String>, krate: &hir::Crate) -> Svh {
+    pub fn calculate(crate_disambiguator: &str, krate: &hir::Crate) -> Svh {
         // FIXME (#14132): This is better than it used to be, but it still not
         // ideal. We now attempt to hash only the relevant portions of the
         // Crate AST as well as the top-level crate attributes. (However,
@@ -78,9 +78,9 @@ impl Svh {
         //        avoid collisions.
         let mut state = SipHasher::new();
 
-        for data in metadata {
-            data.hash(&mut state);
-        }
+        "crate_disambiguator".hash(&mut state);
+        crate_disambiguator.len().hash(&mut state);
+        crate_disambiguator.hash(&mut state);
 
         {
             let mut visit = svh_visitor::make(&mut state, krate);

src/librustc_driver/driver.rs

@@ -22,6 +22,7 @@ use rustc::middle::privacy::AccessLevels;
 use rustc::middle::ty::TyCtxt;
 use rustc::util::common::time;
 use rustc::util::nodemap::NodeSet;
+use rustc_back::sha2::{Sha256, Digest};
 use rustc_borrowck as borrowck;
 use rustc_resolve as resolve;
 use rustc_metadata::macro_import;
@@ -500,7 +501,7 @@ pub fn phase_2_configure_and_expand(sess: &Session,
     }));
 
     *sess.crate_types.borrow_mut() = collect_crate_types(sess, &krate.attrs);
-    *sess.crate_metadata.borrow_mut() = collect_crate_metadata(sess, &krate.attrs);
+    *sess.crate_disambiguator.borrow_mut() = compute_crate_disambiguator(sess);
 
     time(time_passes, "recursion limit", || {
         middle::recursion_limit::update_recursion_limit(sess, &krate);
@@ -525,11 +526,15 @@ pub fn phase_2_configure_and_expand(sess: &Session,
 
     let macros = time(time_passes,
                       "macro loading",
-                      || macro_import::read_macro_defs(sess, &cstore, &krate));
+                      || macro_import::read_macro_defs(sess, &cstore, &krate, crate_name));
 
     let mut addl_plugins = Some(addl_plugins);
     let registrars = time(time_passes, "plugin loading", || {
-        plugin::load::load_plugins(sess, &cstore, &krate, addl_plugins.take().unwrap())
+        plugin::load::load_plugins(sess,
+                                   &cstore,
+                                   &krate,
+                                   crate_name,
+                                   addl_plugins.take().unwrap())
     });
 
     let mut registry = Registry::new(sess, &krate);
@@ -754,7 +759,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
 
     time(time_passes,
          "external crate/lib resolution",
-         || LocalCrateReader::new(sess, cstore, &hir_map).read_crates());
+         || LocalCrateReader::new(sess, cstore, &hir_map, name).read_crates());
 
     let lang_items = try!(time(time_passes, "language item collection", || {
         sess.track_errors(|| {
@@ -817,6 +822,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
                                region_map,
                                lang_items,
                                index,
+                               name,
                                |tcx| {
         // passes are timed inside typeck
         try_with_f!(typeck::check_crate(tcx, trait_map), (tcx, None, analysis));
@@ -1121,8 +1127,34 @@ pub fn collect_crate_types(session: &Session, attrs: &[ast::Attribute]) -> Vec<c
         .collect()
 }
 
-pub fn collect_crate_metadata(session: &Session, _attrs: &[ast::Attribute]) -> Vec<String> {
-    session.opts.cg.metadata.clone()
+pub fn compute_crate_disambiguator(session: &Session) -> String {
+    let mut hasher = Sha256::new();
+
+    let mut metadata = session.opts.cg.metadata.clone();
+    // We don't want the crate_disambiguator to dependent on the order
+    // -C metadata arguments, so sort them:
+    metadata.sort();
+    // Every distinct -C metadata value is only incorporated once:
+    metadata.dedup();
+
+    hasher.input_str("metadata");
+    for s in &metadata {
+        // Also incorporate the length of a metadata string, so that we generate
+        // different values for `-Cmetadata=ab -Cmetadata=c` and
+        // `-Cmetadata=a -Cmetadata=bc`
+        hasher.input_str(&format!("{}", s.len())[..]);
+        hasher.input_str(&s[..]);
+    }
+
+    let mut hash = hasher.result_str();
+
+    // If this is an executable, add a special suffix, so that we don't get
+    // symbol conflicts when linking against a library of the same name.
+    if session.crate_types.borrow().contains(&config::CrateTypeExecutable) {
+       hash.push_str("-exe");
+    }
+
+    hash
 }
 
 pub fn build_output_filenames(input: &Input,

src/librustc_driver/lib.rs

@@ -567,8 +567,6 @@ impl RustcDefaultCalls {
                         continue;
                     }
                     let crate_types = driver::collect_crate_types(sess, attrs);
-                    let metadata = driver::collect_crate_metadata(sess, attrs);
-                    *sess.crate_metadata.borrow_mut() = metadata;
                     for &style in &crate_types {
                         let fname = link::filename_for_input(sess, style, &id, &t_outputs);
                         println!("{}",

src/librustc_driver/test.rs

@@ -146,6 +146,7 @@ fn test_env<F>(source_string: &str,
                                region_map,
                                lang_items,
                                index,
+                               "test_crate",
                                |tcx| {
                                    let infcx = infer::new_infer_ctxt(tcx,
                                                                      &tcx.tables,

src/librustc_metadata/common.rs

@@ -73,6 +73,7 @@ pub const tag_crate_dep: usize = 0x35;
 
 pub const tag_crate_hash: usize = 0x103; // top-level only
 pub const tag_crate_crate_name: usize = 0x104; // top-level only
+pub const tag_crate_disambiguator: usize = 0x113; // top-level only
 
 pub const tag_crate_dep_crate_name: usize = 0x36;
 pub const tag_crate_dep_hash: usize = 0x37;