-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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.
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
Expand is_uninhabited #36449
Expand is_uninhabited #36449
Changes from 8 commits
69bb5fa
d648251
7514051
f1bdd4f
5b20c6a
a6cc398
d756f61
2afec4d
2121118
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,7 @@ use ty::subst::{Subst, Substs}; | |
use ty::walk::TypeWalker; | ||
use util::common::MemoizationMap; | ||
use util::nodemap::NodeSet; | ||
use util::nodemap::FxHashMap; | ||
use util::nodemap::{FxHashMap, FxHashSet}; | ||
|
||
use serialize::{self, Encodable, Encoder}; | ||
use std::borrow::Cow; | ||
|
@@ -1389,6 +1389,22 @@ impl<'tcx> serialize::UseSpecializedEncodable for AdtDef<'tcx> { | |
|
||
impl<'tcx> serialize::UseSpecializedDecodable for AdtDef<'tcx> {} | ||
|
||
impl<'a, 'gcx, 'tcx> AdtDefData<'tcx, 'static> { | ||
#[inline] | ||
pub fn is_uninhabited_recurse(&'tcx self, | ||
visited: &mut FxHashSet<(DefId, &'tcx Substs<'tcx>)>, | ||
block: Option<NodeId>, | ||
cx: TyCtxt<'a, 'gcx, 'tcx>, | ||
substs: &'tcx Substs<'tcx>) -> bool { | ||
if !visited.insert((self.did, substs)) { | ||
return false; | ||
}; | ||
self.variants.iter().all(|v| { | ||
v.is_uninhabited_recurse(visited, block, cx, substs, self.is_union()) | ||
}) | ||
} | ||
} | ||
|
||
#[derive(Copy, Clone, Debug, Eq, PartialEq)] | ||
pub enum AdtKind { Struct, Union, Enum } | ||
|
||
|
@@ -1531,11 +1547,6 @@ impl<'a, 'gcx, 'tcx, 'container> AdtDefData<'gcx, 'container> { | |
self.variants.iter().flat_map(VariantDefData::fields_iter) | ||
} | ||
|
||
#[inline] | ||
pub fn is_empty(&self) -> bool { | ||
self.variants.is_empty() | ||
} | ||
|
||
#[inline] | ||
pub fn is_univariant(&self) -> bool { | ||
self.variants.len() == 1 | ||
|
@@ -1795,6 +1806,22 @@ impl<'tcx, 'container> VariantDefData<'tcx, 'container> { | |
} | ||
} | ||
|
||
impl<'a, 'gcx, 'tcx> VariantDefData<'tcx, 'static> { | ||
#[inline] | ||
pub fn is_uninhabited_recurse(&'tcx self, | ||
visited: &mut FxHashSet<(DefId, &'tcx Substs<'tcx>)>, | ||
block: Option<NodeId>, | ||
cx: TyCtxt<'a, 'gcx, 'tcx>, | ||
substs: &'tcx Substs<'tcx>, | ||
is_union: bool) -> bool { | ||
if is_union { | ||
self.fields.iter().all(|f| f.is_uninhabited_recurse(visited, block, cx, substs)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assumes an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eddyb |
||
} else { | ||
self.fields.iter().any(|f| f.is_uninhabited_recurse(visited, block, cx, substs)) | ||
} | ||
} | ||
} | ||
|
||
impl<'a, 'gcx, 'tcx, 'container> FieldDefData<'tcx, 'container> { | ||
pub fn new(did: DefId, | ||
name: Name, | ||
|
@@ -1820,6 +1847,18 @@ impl<'a, 'gcx, 'tcx, 'container> FieldDefData<'tcx, 'container> { | |
} | ||
} | ||
|
||
impl<'a, 'gcx, 'tcx> FieldDefData<'tcx, 'static> { | ||
#[inline] | ||
pub fn is_uninhabited_recurse(&'tcx self, | ||
visited: &mut FxHashSet<(DefId, &'tcx Substs<'tcx>)>, | ||
block: Option<NodeId>, | ||
tcx: TyCtxt<'a, 'gcx, 'tcx>, | ||
substs: &'tcx Substs<'tcx>) -> bool { | ||
block.map_or(true, |b| self.vis.is_accessible_from(b, &tcx.map)) && | ||
self.ty(tcx, substs).is_uninhabited_recurse(visited, block, tcx) | ||
} | ||
} | ||
|
||
/// Records the substitutions used to translate the polytype for an | ||
/// item into the monotype of an item reference. | ||
#[derive(Clone, RustcEncodable, RustcDecodable)] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not have any change in
libcore
if possible.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the problem with this change? I think this is what @nikomatsakis meant by redefining
Void
. We need to make some change here or else that module breaks.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it break practically, or conceptually?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First conceptually, then practically when tests fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What tests? I don't want to land this if it changes semantics of code that already compiles (as opposed to just letting more code compiles).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IOW, this is a good/necessary change long-term, but it shouldn't be needed here - if it is, something is wrong with the assumption we're making which is the basis for allowing this PR through.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... I just removed that change and now all tests pass. I definitely added it out of necessity though, something must have changed in the intervening two months.
I'll do another commit to remove it.