Skip to content

Remove ty_to_def_id #52381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 15, 2018
Merged

Remove ty_to_def_id #52381

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a59cf4e
Remove `ty_to_def_id`
oli-obk Jul 14, 2018
1caa593
Expose a self-referential object
oli-obk Jul 14, 2018
ecab96f
Ubsan this newly discovered dead code
oli-obk Jul 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions src/librustc/middle/dead.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,13 +469,9 @@ impl<'a, 'tcx> DeadVisitor<'a, 'tcx> {

fn should_warn_about_field(&mut self, field: &hir::StructField) -> bool {
let field_type = self.tcx.type_of(self.tcx.hir.local_def_id(field.id));
let is_marker_field = match field_type.ty_to_def_id() {
Some(def_id) => self.tcx.lang_items().items().iter().any(|item| *item == Some(def_id)),
_ => false
};
!field.is_positional()
&& !self.symbol_is_live(field.id, None)
&& !is_marker_field
&& !field_type.is_phantom_data()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, are the only two lang item types, PhantomData and Box?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh. I don't see why Box should be special. Depending on the lang_itemnes of a type seems rather fragile.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it shouldn't be, but once upon a time there were many lang items, one for each of "{co,contra,in}variant {lifetime,type}" (so 6 at least), so maybe that's where it's leftover from.
I'm just amused only two types are left and the Box case actually hides some warnings!

&& !has_allow_dead_code_or_lang_attr(self.tcx, field.id, &field.attrs)
}

Expand Down
10 changes: 8 additions & 2 deletions src/librustc/traits/error_reporting.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
flags.push((name, Some(value)));
}

if let Some(true) = self_ty.ty_to_def_id().map(|def_id| def_id.is_local()) {
if let Some(true) = self_ty.ty_adt_def().map(|def| def.did.is_local()) {
flags.push(("crate_local".to_string(), None));
}

Expand Down Expand Up @@ -775,7 +775,13 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
let found_trait_ty = found_trait_ref.self_ty();

let found_did = found_trait_ty.ty_to_def_id();
let found_did = match found_trait_ty.sty {
ty::TyClosure(did, _) |
ty::TyForeign(did) |
ty::TyFnDef(did, _) => Some(did),
ty::TyAdt(def, _) => Some(def.did),
_ => None,
};
let found_span = found_did.and_then(|did| {
self.tcx.hir.span_if_local(did)
}).map(|sp| self.tcx.sess.codemap().def_span(sp)); // the sp could be an fn def
Expand Down
11 changes: 0 additions & 11 deletions src/librustc/ty/sty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1767,17 +1767,6 @@ impl<'a, 'gcx, 'tcx> TyS<'tcx> {
}
}

pub fn ty_to_def_id(&self) -> Option<DefId> {
match self.sty {
TyDynamic(ref tt, ..) => tt.principal().map(|p| p.def_id()),
TyAdt(def, _) => Some(def.did),
TyForeign(did) => Some(did),
TyClosure(id, _) => Some(id),
TyFnDef(id, _) => Some(id),
_ => None,
}
}

pub fn ty_adt_def(&self) -> Option<&'tcx AdtDef> {
match self.sty {
TyAdt(adt, _) => Some(adt),
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_codegen_llvm/back/write.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,9 @@ impl CodegenContext {
}

struct DiagnosticHandlers<'a> {
#[allow(dead_code)]
// This value is not actually dead, llcx has pointers to it and needs these pointers to be alive
// until Drop is executed on this object
inner: Box<(&'a CodegenContext, &'a Handler)>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the heap allocation itself? This is really confusing, maybe a raw pointer should be kept instead, with a manual Drop impl that uses Box::from_raw.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also maybe the comment should be phrased to be about the callback having a copy of the pointer or something. It's not really "self-referential" insofar as much as it is scoping a resource for the duration C++ needs it.

llcx: ContextRef,
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_lint/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,8 +591,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDebugImplementations {
if self.impling_types.is_none() {
let mut impls = NodeSet();
cx.tcx.for_each_impl(debug, |d| {
if let Some(ty_def) = cx.tcx.type_of(d).ty_to_def_id() {
if let Some(node_id) = cx.tcx.hir.as_local_node_id(ty_def) {
if let Some(ty_def) = cx.tcx.type_of(d).ty_adt_def() {
if let Some(node_id) = cx.tcx.hir.as_local_node_id(ty_def.did) {
impls.insert(node_id);
}
}
Expand Down