Skip to content

Commit

Permalink
watch out for krate numbers being reassigned
Browse files Browse the repository at this point in the history
The biggest problem, actually, is krate numbers being removed entirely,
which can lead to array-index-out-of-bounds errors.

cc rust-lang#35123 -- not a complete fix, since really we ought to "map" the old
crate numbers to the new ones, not just detect changes.
  • Loading branch information
nikomatsakis committed Aug 2, 2016
1 parent c56eb4b commit b4929d1
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 5 deletions.
78 changes: 73 additions & 5 deletions src/librustc_incremental/persist/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@
use rustc::dep_graph::DepNode;
use rustc::hir::map::DefPath;
use rustc::hir::def_id::DefId;
use rustc::middle::cstore::LOCAL_CRATE;
use rustc::ty::TyCtxt;
use rustc::util::nodemap::DefIdMap;
use std::fmt::{self, Debug};
use std::iter::once;
use syntax::ast;

/// Index into the DefIdDirectory
#[derive(Copy, Clone, Debug, PartialOrd, Ord, Hash, PartialEq, Eq,
Expand All @@ -31,17 +34,66 @@ pub struct DefPathIndex {
pub struct DefIdDirectory {
// N.B. don't use Removable here because these def-ids are loaded
// directly without remapping, so loading them should not fail.
paths: Vec<DefPath>
paths: Vec<DefPath>,

// For each crate, saves the crate-name/disambiguator so that
// later we can match crate-numbers up again.
krates: Vec<KrateInfo>,
}

#[derive(Debug, RustcEncodable, RustcDecodable)]
pub struct KrateInfo {
krate: ast::CrateNum,
name: String,
disambiguator: String,
}

impl DefIdDirectory {
pub fn new() -> DefIdDirectory {
DefIdDirectory { paths: vec![] }
pub fn new(krates: Vec<KrateInfo>) -> DefIdDirectory {
DefIdDirectory { paths: vec![], krates: krates }
}

pub fn krate_still_valid(&self,
tcx: TyCtxt,
max_current_crate: ast::CrateNum,
krate: ast::CrateNum) -> bool {
// Check that the crate-number still matches. For now, if it
// doesn't, just return None. We could do better, such as
// finding the new number.

if krate > max_current_crate {
false
} else {
let old_info = &self.krates[krate as usize];
assert_eq!(old_info.krate, krate);
let old_name: &str = &old_info.name;
let old_disambiguator: &str = &old_info.disambiguator;
let new_name: &str = &tcx.crate_name(krate);
let new_disambiguator: &str = &tcx.crate_disambiguator(krate);
old_name == new_name && old_disambiguator == new_disambiguator
}
}

pub fn retrace(&self, tcx: TyCtxt) -> RetracedDefIdDirectory {
let max_current_crate =
tcx.sess.cstore.crates()
.into_iter()
.max()
.unwrap_or(LOCAL_CRATE);

let ids = self.paths.iter()
.map(|path| tcx.retrace_path(path))
.map(|path| {
if self.krate_still_valid(tcx, max_current_crate, path.krate) {
tcx.retrace_path(path)
} else {
debug!("crate {} changed from {:?} to {:?}/{:?}",
path.krate,
self.krates[path.krate as usize],
tcx.crate_name(path.krate),
tcx.crate_disambiguator(path.krate));
None
}
})
.collect();
RetracedDefIdDirectory { ids: ids }
}
Expand Down Expand Up @@ -70,10 +122,26 @@ pub struct DefIdDirectoryBuilder<'a,'tcx:'a> {

impl<'a,'tcx> DefIdDirectoryBuilder<'a,'tcx> {
pub fn new(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> DefIdDirectoryBuilder<'a, 'tcx> {
let mut krates: Vec<_> =
once(LOCAL_CRATE)
.chain(tcx.sess.cstore.crates())
.map(|krate| {
KrateInfo {
krate: krate,
name: tcx.crate_name(krate).to_string(),
disambiguator: tcx.crate_disambiguator(krate).to_string()
}
})
.collect();

// the result of crates() is not in order, so sort list of
// crates so that we can just index it later
krates.sort_by_key(|k| k.krate);

DefIdDirectoryBuilder {
tcx: tcx,
hash: DefIdMap(),
directory: DefIdDirectory::new()
directory: DefIdDirectory::new(krates),
}
}

Expand Down
14 changes: 14 additions & 0 deletions src/test/incremental/krate_reassign_34991/auxiliary/a.rs
Original file line number Diff line number Diff line change
@@ -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_type="rlib"]

pub type X = u32;

30 changes: 30 additions & 0 deletions src/test/incremental/krate_reassign_34991/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// 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.

// aux-build:a.rs
// revisions:rpass1 rpass2

#![feature(rustc_attrs)]

#[cfg(rpass1)]
extern crate a;

#[cfg(rpass1)]
pub fn use_X() -> u32 {
let x: a::X = 22;
x as u32
}

#[cfg(rpass2)]
pub fn use_X() -> u32 {
22
}

pub fn main() { }

0 comments on commit b4929d1

Please sign in to comment.