Skip to content
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

pat_analysis: Don't rely on contiguous VariantIds outside of rustc #120039

Merged
merged 1 commit into from
Jan 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4355,6 +4355,7 @@ name = "rustc_pattern_analysis"
version = "0.0.0"
dependencies = [
"derivative",
"rustc-hash",
"rustc_apfloat",
"rustc_arena",
"rustc_data_structures",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_pattern_analysis/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ edition = "2021"
[dependencies]
# tidy-alphabetical-start
derivative = "2.2.0"
rustc-hash = "1.1.0"
rustc_apfloat = "0.2.0"
rustc_arena = { path = "../rustc_arena", optional = true }
rustc_data_structures = { path = "../rustc_data_structures", optional = true }
Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_pattern_analysis/src/constructor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,13 @@ use std::iter::once;
use smallvec::SmallVec;

use rustc_apfloat::ieee::{DoubleS, IeeeFloat, SingleS};
use rustc_index::bit_set::{BitSet, GrowableBitSet};
use rustc_index::IndexVec;
use rustc_index::bit_set::GrowableBitSet;

use self::Constructor::*;
use self::MaybeInfiniteInt::*;
use self::SliceKind::*;

use crate::index;
use crate::usefulness::PlaceCtxt;
use crate::TypeCx;

Expand Down Expand Up @@ -804,7 +804,10 @@ pub enum ConstructorSet<Cx: TypeCx> {
Struct { empty: bool },
/// This type has the following list of constructors. If `variants` is empty and
/// `non_exhaustive` is false, don't use this; use `NoConstructors` instead.
Variants { variants: IndexVec<Cx::VariantIdx, VariantVisibility>, non_exhaustive: bool },
Variants {
variants: index::IdxContainer<Cx::VariantIdx, VariantVisibility>,
non_exhaustive: bool,
},
/// The type is `&T`.
Ref,
/// The type is a union.
Expand Down Expand Up @@ -904,7 +907,7 @@ impl<Cx: TypeCx> ConstructorSet<Cx> {
}
}
ConstructorSet::Variants { variants, non_exhaustive } => {
let mut seen_set: BitSet<_> = BitSet::new_empty(variants.len());
let mut seen_set = index::IdxSet::new_empty(variants.len());
for idx in seen.iter().map(|c| c.as_variant().unwrap()) {
seen_set.insert(idx);
}
Expand Down
42 changes: 40 additions & 2 deletions compiler/rustc_pattern_analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,45 @@ rustc_fluent_macro::fluent_messages! { "../messages.ftl" }

use std::fmt;

use rustc_index::Idx;
#[cfg(feature = "rustc")]
pub mod index {
// Faster version when the indices of variants are `0..variants.len()`.
pub use rustc_index::bit_set::BitSet as IdxSet;
pub use rustc_index::Idx;
pub use rustc_index::IndexVec as IdxContainer;
}
#[cfg(not(feature = "rustc"))]
pub mod index {
// Slower version when the indices of variants are something else.
pub trait Idx: Copy + PartialEq + Eq + std::hash::Hash {}
impl<T: Copy + PartialEq + Eq + std::hash::Hash> Idx for T {}

#[derive(Debug)]
pub struct IdxContainer<K, V>(pub rustc_hash::FxHashMap<K, V>);
Copy link
Member

Choose a reason for hiding this comment

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

Can we use the rustc_data_structures types here instead and avoid the extra crate dep? (It's still the same under the hood, but makes it easier to tweak globally).

Copy link
Member

Choose a reason for hiding this comment

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

Can we use the rustc_data_structures types here instead [...]

rustc_data_structures is not compatible with rust-analyzer due to its use of nightly features. I guess @Nadrieril could add a feature = "nightly" to that crate, but it seems unnecessary.

Are IDs elsewhere still contiguous in the common case, even if not starting at zero?

I don't believe so. As Nadrieril said above, they're global interning IDs, so they're probably really sparse.

Copy link
Member Author

@Nadrieril Nadrieril Jan 17, 2024

Choose a reason for hiding this comment

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

No because rust_analyzer wants to avoid the rustc_data_structures dependency (it's really big and has lots of unstable features)

EDIT: raced by errs

impl<K: Idx, V> IdxContainer<K, V> {
pub fn len(&self) -> usize {
self.0.len()
}
pub fn iter_enumerated(&self) -> impl Iterator<Item = (K, &V)> {
self.0.iter().map(|(k, v)| (*k, v))
}
}

#[derive(Debug)]
pub struct IdxSet<T>(pub rustc_hash::FxHashSet<T>);
impl<T: Idx> IdxSet<T> {
pub fn new_empty(_len: usize) -> Self {
Self(Default::default())
}
pub fn contains(&self, elem: T) -> bool {
self.0.contains(&elem)
}
pub fn insert(&mut self, elem: T) {
self.0.insert(elem);
}
}
}

#[cfg(feature = "rustc")]
use rustc_middle::ty::Ty;
#[cfg(feature = "rustc")]
Expand Down Expand Up @@ -50,7 +88,7 @@ pub trait TypeCx: Sized + fmt::Debug {
/// Errors that can abort analysis.
type Error: fmt::Debug;
/// The index of an enum variant.
type VariantIdx: Clone + Idx;
type VariantIdx: Clone + index::Idx + fmt::Debug;
/// A string literal
type StrLit: Clone + PartialEq + fmt::Debug;
/// Extra data to store in a match arm.
Expand Down