Skip to content

Commit a68b1c5

Browse files
borsbors
committed
Auto merge of #32293 - nikomatsakis:incr-comp-def-path-munging, r=eddyb
Revamp symbol names for impls (and make them deterministic, etc) This builds on @michaelwoerister's epic PR #31539 (note that his PR never landed, so I just incorporated it into this one). The main change here is that we remove the "name" from `DefPathData` for impls, since that name is synthetic and not sufficiently predictable for incr comp. However, just doing that would cause bad symbol names since those are based on the `DefPath`. Therefore, I introduce a new mechanism for getting symbol names (and also paths for user display) called `item_path`. This is kind of simplistic for now (based on strings) but I expect to expand it later to support richer types, hopefully generating C++-mangled names that gdb etc can understand. Along the way I cleaned up how we track the path that leads to an extern crate. There is still some cleanup left undone here. Notably, I didn't remove the impl names altogether -- that would probably make sense. I also didn't try to remove the `item_symbols` vector. Mostly I want to unblock my other incr. comp. work. =) r? @eddyb cc @eddyb @alexcrichton @michaelwoerister
2 parents 8ba2ea5 + c2f1073 commit a68b1c5

File tree

100 files changed

+1959
-661
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

100 files changed

+1959
-661
lines changed

‎mk/tests.mk‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ $(3)/stage$(1)/test/$(4)test-$(2)$$(X_$(2)): \
383383
@$$(call E, rustc: $$@)
384384
$(Q)CFG_LLVM_LINKAGE_FILE=$$(LLVM_LINKAGE_PATH_$(2)) \
385385
$$(subst @,,$$(STAGE$(1)_T_$(2)_H_$(3))) -o $$@ $$< --test \
386-
-L "$$(RT_OUTPUT_DIR_$(2))" \
386+
-Cmetadata="test-crate" -L "$$(RT_OUTPUT_DIR_$(2))" \
387387
$$(LLVM_LIBDIR_RUSTFLAGS_$(2)) \
388388
$$(RUSTFLAGS_$(4))
389389

‎src/compiletest/common.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,10 @@ impl fmt::Display for Mode {
6969
#[derive(Clone)]
7070
pub struct Config {
7171
// The library paths required for running the compiler
72-
pub compile_lib_path: String,
72+
pub compile_lib_path: PathBuf,
7373

7474
// The library paths required for running compiled programs
75-
pub run_lib_path: String,
75+
pub run_lib_path: PathBuf,
7676

7777
// The rustc executable
7878
pub rustc_path: PathBuf,

‎src/compiletest/compiletest.rs‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,17 @@ pub fn parse_config(args: Vec<String> ) -> Config {
117117
}
118118
}
119119

120+
fn make_absolute(path: PathBuf) -> PathBuf {
121+
if path.is_relative() {
122+
env::current_dir().unwrap().join(path)
123+
} else {
124+
path
125+
}
126+
}
127+
120128
Config {
121-
compile_lib_path: matches.opt_str("compile-lib-path").unwrap(),
122-
run_lib_path: matches.opt_str("run-lib-path").unwrap(),
129+
compile_lib_path: make_absolute(opt_path(matches, "compile-lib-path")),
130+
run_lib_path: make_absolute(opt_path(matches, "run-lib-path")),
123131
rustc_path: opt_path(matches, "rustc-path"),
124132
rustdoc_path: opt_path(matches, "rustdoc-path"),
125133
python: matches.opt_str("python").unwrap(),

‎src/compiletest/runtest.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ fn run_pretty_test_revision(config: &Config,
316316
testpaths,
317317
pretty_type.to_owned()),
318318
props.exec_env.clone(),
319-
&config.compile_lib_path,
319+
config.compile_lib_path.to_str().unwrap(),
320320
Some(aux_dir.to_str().unwrap()),
321321
Some(src))
322322
}
@@ -635,7 +635,7 @@ fn run_debuginfo_gdb_test(config: &Config, props: &TestProps, testpaths: &TestPa
635635
testpaths,
636636
proc_args,
637637
environment,
638-
&config.run_lib_path,
638+
config.run_lib_path.to_str().unwrap(),
639639
None,
640640
None);
641641
}
@@ -1315,7 +1315,7 @@ fn exec_compiled_test(config: &Config, props: &TestProps,
13151315
testpaths,
13161316
make_run_args(config, props, testpaths),
13171317
env,
1318-
&config.run_lib_path,
1318+
config.run_lib_path.to_str().unwrap(),
13191319
Some(aux_dir.to_str().unwrap()),
13201320
None)
13211321
}
@@ -1387,7 +1387,7 @@ fn compose_and_run_compiler(config: &Config, props: &TestProps,
13871387
&aux_testpaths,
13881388
aux_args,
13891389
Vec::new(),
1390-
&config.compile_lib_path,
1390+
config.compile_lib_path.to_str().unwrap(),
13911391
Some(aux_dir.to_str().unwrap()),
13921392
None);
13931393
if !auxres.status.success() {
@@ -1410,7 +1410,7 @@ fn compose_and_run_compiler(config: &Config, props: &TestProps,
14101410
testpaths,
14111411
args,
14121412
props.rustc_env.clone(),
1413-
&config.compile_lib_path,
1413+
config.compile_lib_path.to_str().unwrap(),
14141414
Some(aux_dir.to_str().unwrap()),
14151415
input)
14161416
}

‎src/librbml/lib.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,15 +165,15 @@ impl<'doc> Doc<'doc> {
165165
}
166166
}
167167

168-
pub fn get<'a>(&'a self, tag: usize) -> Doc<'a> {
168+
pub fn get(&self, tag: usize) -> Doc<'doc> {
169169
reader::get_doc(*self, tag)
170170
}
171171

172172
pub fn is_empty(&self) -> bool {
173173
self.start == self.end
174174
}
175175

176-
pub fn as_str_slice<'a>(&'a self) -> &'a str {
176+
pub fn as_str_slice(&self) -> &'doc str {
177177
str::from_utf8(&self.data[self.start..self.end]).unwrap()
178178
}
179179

‎src/librustc/front/map/collector.rs‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use super::MapEntry::*;
1414
use rustc_front::hir::*;
1515
use rustc_front::util;
1616
use rustc_front::intravisit::{self, Visitor};
17-
use middle::def_id::{CRATE_DEF_INDEX, DefIndex};
17+
use middle::def_id::{CRATE_DEF_INDEX, DefId, DefIndex};
1818
use std::iter::repeat;
1919
use syntax::ast::{NodeId, CRATE_NODE_ID, DUMMY_NODE_ID};
2020
use syntax::codemap::Span;
@@ -50,6 +50,7 @@ impl<'ast> NodeCollector<'ast> {
5050
parent: &'ast InlinedParent,
5151
parent_node: NodeId,
5252
parent_def_path: DefPath,
53+
parent_def_id: DefId,
5354
map: Vec<MapEntry<'ast>>,
5455
definitions: Definitions)
5556
-> NodeCollector<'ast> {
@@ -60,8 +61,14 @@ impl<'ast> NodeCollector<'ast> {
6061
definitions: definitions,
6162
};
6263

64+
assert_eq!(parent_def_path.krate, parent_def_id.krate);
65+
let root_path = Box::new(InlinedRootPath {
66+
data: parent_def_path.data,
67+
def_id: parent_def_id,
68+
});
69+
6370
collector.insert_entry(parent_node, RootInlinedParent(parent));
64-
collector.create_def(parent_node, DefPathData::InlinedRoot(parent_def_path));
71+
collector.create_def(parent_node, DefPathData::InlinedRoot(root_path));
6572

6673
collector
6774
}
@@ -126,11 +133,16 @@ impl<'ast> Visitor<'ast> for NodeCollector<'ast> {
126133
// Pick the def data. This need not be unique, but the more
127134
// information we encapsulate into
128135
let def_data = match i.node {
129-
ItemDefaultImpl(..) | ItemImpl(..) => DefPathData::Impl(i.name),
130-
ItemEnum(..) | ItemStruct(..) | ItemTrait(..) => DefPathData::Type(i.name),
131-
ItemExternCrate(..) | ItemMod(..) => DefPathData::Mod(i.name),
132-
ItemStatic(..) | ItemConst(..) | ItemFn(..) => DefPathData::Value(i.name),
133-
_ => DefPathData::Misc,
136+
ItemDefaultImpl(..) | ItemImpl(..) =>
137+
DefPathData::Impl,
138+
ItemEnum(..) | ItemStruct(..) | ItemTrait(..) |
139+
ItemExternCrate(..) | ItemMod(..) | ItemForeignMod(..) |
140+
ItemTy(..) =>
141+
DefPathData::TypeNs(i.name),
142+
ItemStatic(..) | ItemConst(..) | ItemFn(..) =>
143+
DefPathData::ValueNs(i.name),
144+
ItemUse(..) =>
145+
DefPathData::Misc,
134146
};
135147

136148
self.insert_def(i.id, NodeItem(i), def_data);
@@ -195,7 +207,7 @@ impl<'ast> Visitor<'ast> for NodeCollector<'ast> {
195207
fn visit_foreign_item(&mut self, foreign_item: &'ast ForeignItem) {
196208
self.insert_def(foreign_item.id,
197209
NodeForeignItem(foreign_item),
198-
DefPathData::Value(foreign_item.name));
210+
DefPathData::ValueNs(foreign_item.name));
199211

200212
let parent_node = self.parent_node;
201213
self.parent_node = foreign_item.id;
@@ -215,8 +227,8 @@ impl<'ast> Visitor<'ast> for NodeCollector<'ast> {
215227

216228
fn visit_trait_item(&mut self, ti: &'ast TraitItem) {
217229
let def_data = match ti.node {
218-
MethodTraitItem(..) | ConstTraitItem(..) => DefPathData::Value(ti.name),
219-
TypeTraitItem(..) => DefPathData::Type(ti.name),
230+
MethodTraitItem(..) | ConstTraitItem(..) => DefPathData::ValueNs(ti.name),
231+
TypeTraitItem(..) => DefPathData::TypeNs(ti.name),
220232
};
221233

222234
self.insert(ti.id, NodeTraitItem(ti));
@@ -239,8 +251,8 @@ impl<'ast> Visitor<'ast> for NodeCollector<'ast> {
239251

240252
fn visit_impl_item(&mut self, ii: &'ast ImplItem) {
241253
let def_data = match ii.node {
242-
ImplItemKind::Method(..) | ImplItemKind::Const(..) => DefPathData::Value(ii.name),
243-
ImplItemKind::Type(..) => DefPathData::Type(ii.name),
254+
ImplItemKind::Method(..) | ImplItemKind::Const(..) => DefPathData::ValueNs(ii.name),
255+
ImplItemKind::Type(..) => DefPathData::TypeNs(ii.name),
244256
};
245257

246258
self.insert_def(ii.id, NodeImplItem(ii), def_data);

‎src/librustc/front/map/definitions.rs‎

Lines changed: 84 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -59,23 +59,94 @@ pub struct DefData {
5959
pub node_id: ast::NodeId,
6060
}
6161

62-
pub type DefPath = Vec<DisambiguatedDefPathData>;
62+
#[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
63+
pub struct DefPath {
64+
/// the path leading from the crate root to the item
65+
pub data: Vec<DisambiguatedDefPathData>,
66+
67+
/// what krate root is this path relative to?
68+
pub krate: ast::CrateNum,
69+
}
70+
71+
impl DefPath {
72+
pub fn is_local(&self) -> bool {
73+
self.krate == LOCAL_CRATE
74+
}
75+
76+
pub fn make<FN>(start_krate: ast::CrateNum,
77+
start_index: DefIndex,
78+
mut get_key: FN) -> DefPath
79+
where FN: FnMut(DefIndex) -> DefKey
80+
{
81+
let mut krate = start_krate;
82+
let mut data = vec![];
83+
let mut index = Some(start_index);
84+
loop {
85+
let p = index.unwrap();
86+
let key = get_key(p);
87+
match key.disambiguated_data.data {
88+
DefPathData::CrateRoot => {
89+
assert!(key.parent.is_none());
90+
break;
91+
}
92+
DefPathData::InlinedRoot(ref p) => {
93+
assert!(key.parent.is_none());
94+
assert!(!p.def_id.is_local());
95+
data.extend(p.data.iter().cloned().rev());
96+
krate = p.def_id.krate;
97+
break;
98+
}
99+
_ => {
100+
data.push(key.disambiguated_data);
101+
index = key.parent;
102+
}
103+
}
104+
}
105+
data.reverse();
106+
DefPath { data: data, krate: krate }
107+
}
108+
}
109+
110+
/// Root of an inlined item. We track the `DefPath` of the item within
111+
/// the original crate but also its def-id. This is kind of an
112+
/// augmented version of a `DefPath` that includes a `DefId`. This is
113+
/// all sort of ugly but the hope is that inlined items will be going
114+
/// away soon anyway.
115+
///
116+
/// Some of the constraints that led to the current approach:
117+
///
118+
/// - I don't want to have a `DefId` in the main `DefPath` because
119+
/// that gets serialized for incr. comp., and when reloaded the
120+
/// `DefId` is no longer valid. I'd rather maintain the invariant
121+
/// that every `DefId` is valid, and a potentially outdated `DefId` is
122+
/// represented as a `DefPath`.
123+
/// - (We don't serialize def-paths from inlined items, so it's ok to have one here.)
124+
/// - We need to be able to extract the def-id from inline items to
125+
/// make the symbol name. In theory we could retrace it from the
126+
/// data, but the metadata doesn't have the required indices, and I
127+
/// don't want to write the code to create one just for this.
128+
/// - It may be that we don't actually need `data` at all. We'll have
129+
/// to see about that.
130+
#[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
131+
pub struct InlinedRootPath {
132+
pub data: Vec<DisambiguatedDefPathData>,
133+
pub def_id: DefId,
134+
}
63135

64136
#[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
65137
pub enum DefPathData {
66138
// Root: these should only be used for the root nodes, because
67139
// they are treated specially by the `def_path` function.
68140
CrateRoot,
69-
InlinedRoot(DefPath),
141+
InlinedRoot(Box<InlinedRootPath>),
70142

71143
// Catch-all for random DefId things like DUMMY_NODE_ID
72144
Misc,
73145

74146
// Different kinds of items and item-like things:
75-
Impl(ast::Name),
76-
Type(ast::Name),
77-
Mod(ast::Name),
78-
Value(ast::Name),
147+
Impl,
148+
TypeNs(ast::Name), // something in the type NS
149+
ValueNs(ast::Name), // something in the value NS
79150
MacroDef(ast::Name),
80151
ClosureExpr,
81152

@@ -87,10 +158,6 @@ pub enum DefPathData {
87158
StructCtor, // implicit ctor for a tuple-like struct
88159
Initializer, // initializer for a const
89160
Binding(ast::Name), // pattern binding
90-
91-
// An external crate that does not have an `extern crate` in this
92-
// crate.
93-
DetachedCrate(ast::Name),
94161
}
95162

96163
impl Definitions {
@@ -116,7 +183,7 @@ impl Definitions {
116183
/// will be the path of the item in the external crate (but the
117184
/// path will begin with the path to the external crate).
118185
pub fn def_path(&self, index: DefIndex) -> DefPath {
119-
make_def_path(index, |p| self.def_key(p))
186+
DefPath::make(LOCAL_CRATE, index, |p| self.def_key(p))
120187
}
121188

122189
pub fn opt_def_index(&self, node: ast::NodeId) -> Option<DefIndex> {
@@ -175,20 +242,21 @@ impl DefPathData {
175242
pub fn as_interned_str(&self) -> InternedString {
176243
use self::DefPathData::*;
177244
match *self {
178-
Impl(name) |
179-
Type(name) |
180-
Mod(name) |
181-
Value(name) |
245+
TypeNs(name) |
246+
ValueNs(name) |
182247
MacroDef(name) |
183248
TypeParam(name) |
184249
LifetimeDef(name) |
185250
EnumVariant(name) |
186-
DetachedCrate(name) |
187251
Binding(name) |
188252
Field(name) => {
189253
name.as_str()
190254
}
191255

256+
Impl => {
257+
InternedString::new("{{impl}}")
258+
}
259+
192260
// note that this does not show up in user printouts
193261
CrateRoot => {
194262
InternedString::new("{{root}}")
@@ -222,29 +290,3 @@ impl DefPathData {
222290
}
223291
}
224292

225-
pub fn make_def_path<FN>(start_index: DefIndex, mut get_key: FN) -> DefPath
226-
where FN: FnMut(DefIndex) -> DefKey
227-
{
228-
let mut result = vec![];
229-
let mut index = Some(start_index);
230-
while let Some(p) = index {
231-
let key = get_key(p);
232-
match key.disambiguated_data.data {
233-
DefPathData::CrateRoot => {
234-
assert!(key.parent.is_none());
235-
break;
236-
}
237-
DefPathData::InlinedRoot(ref p) => {
238-
assert!(key.parent.is_none());
239-
result.extend(p.iter().cloned().rev());
240-
break;
241-
}
242-
_ => {
243-
result.push(key.disambiguated_data);
244-
index = key.parent;
245-
}
246-
}
247-
}
248-
result.reverse();
249-
result
250-
}

0 commit comments

Comments
 (0)