Auto merge of rust-lang#125495 - lcnr:canonicalizer-bound-var-lookup, r=<try>

canonicalizer: add lookup table

blocking this on being able to run perf, may be necessary for deeply nested types.

r? `@compiler-errors`

Author: bors

compiler/rustc_interface/src/tests.rs

@@ -804,7 +804,7 @@ fn test_unstable_options_tracking_hash() {
     tracked!(mir_opt_level, Some(4));
     tracked!(move_size_limit, Some(4096));
     tracked!(mutable_noalias, false);
-    tracked!(next_solver, Some(NextSolverConfig { coherence: true, globally: false }));
+    tracked!(next_solver, NextSolverConfig { coherence: true, globally: true });
     tracked!(no_generate_arange_section, true);
     tracked!(no_jump_tables, true);
     tracked!(no_link, true);

compiler/rustc_middle/src/ty/context.rs

@@ -3127,11 +3127,11 @@ impl<'tcx> TyCtxt<'tcx> {
     }
 
     pub fn next_trait_solver_globally(self) -> bool {
-        self.sess.opts.unstable_opts.next_solver.map_or(false, |c| c.globally)
+        self.sess.opts.unstable_opts.next_solver.globally
     }
 
     pub fn next_trait_solver_in_coherence(self) -> bool {
-        self.sess.opts.unstable_opts.next_solver.map_or(false, |c| c.coherence)
+        self.sess.opts.unstable_opts.next_solver.coherence
     }
 
     pub fn is_impl_trait_in_trait(self, def_id: DefId) -> bool {

compiler/rustc_next_trait_solver/src/canonicalizer.rs

@@ -1,5 +1,6 @@
 use std::cmp::Ordering;
 
+use rustc_type_ir::data_structures::HashMap;
 use rustc_type_ir::fold::{TypeFoldable, TypeFolder, TypeSuperFoldable};
 use rustc_type_ir::inherent::*;
 use rustc_type_ir::visit::TypeVisitableExt;
@@ -44,6 +45,8 @@ pub struct Canonicalizer<'a, D: SolverDelegate<Interner = I>, I: Interner> {
     canonicalize_mode: CanonicalizeMode,
 
     variables: &'a mut Vec<I::GenericArg>,
+    variable_lookup_table: HashMap<I::GenericArg, usize>,
+
     primitive_var_infos: Vec<CanonicalVarInfo<I>>,
     binder_index: ty::DebruijnIndex,
 }
@@ -60,6 +63,7 @@ impl<'a, D: SolverDelegate<Interner = I>, I: Interner> Canonicalizer<'a, D, I> {
             canonicalize_mode,
 
             variables,
+            variable_lookup_table: Default::default(),
             primitive_var_infos: Vec::new(),
             binder_index: ty::INNERMOST,
         };
@@ -75,6 +79,37 @@ impl<'a, D: SolverDelegate<Interner = I>, I: Interner> Canonicalizer<'a, D, I> {
         Canonical { defining_opaque_types, max_universe, variables, value }
     }
 
+    fn get_or_insert_bound_var(
+        &mut self,
+        arg: impl Into<I::GenericArg>,
+        canonical_var_info: CanonicalVarInfo<I>,
+    ) -> ty::BoundVar {
+        // FIXME: 16 is made up and arbitrary. We should look at some
+        // perf data here.
+        let arg = arg.into();
+        let idx = if self.variables.len() > 16 {
+            if self.variable_lookup_table.is_empty() {
+                self.variable_lookup_table.extend(self.variables.iter().copied().zip(0..));
+            }
+
+            *self.variable_lookup_table.entry(arg).or_insert_with(|| {
+                let var = self.variables.len();
+                self.variables.push(arg);
+                self.primitive_var_infos.push(canonical_var_info);
+                var
+            })
+        } else {
+            self.variables.iter().position(|&v| v == arg).unwrap_or_else(|| {
+                let var = self.variables.len();
+                self.variables.push(arg);
+                self.primitive_var_infos.push(canonical_var_info);
+                var
+            })
+        };
+
+        ty::BoundVar::from(idx)
+    }
+
     fn finalize(self) -> (ty::UniverseIndex, I::CanonicalVars) {
         let mut var_infos = self.primitive_var_infos;
         // See the rustc-dev-guide section about how we deal with universes
@@ -124,8 +159,8 @@ impl<'a, D: SolverDelegate<Interner = I>, I: Interner> Canonicalizer<'a, D, I> {
         // - var_infos: [E0, U1, E2, U1, E1, E6, U6], curr_compressed_uv: 2, next_orig_uv: 6
         // - var_infos: [E0, U1, E1, U1, E1, E3, U3], curr_compressed_uv: 2, next_orig_uv: -
         //
-        // This algorithm runs in `O(n²)` where `n` is the number of different universe
-        // indices in the input. This should be fine as `n` is expected to be small.
+        // This algorithm runs in `O(mn)` where `n` is the number of different universes and
+        // `m` the number of variables. This should be fine as both are expected to be small.
         let mut curr_compressed_uv = ty::UniverseIndex::ROOT;
         let mut existential_in_new_uv = None;
         let mut next_orig_uv = Some(ty::UniverseIndex::ROOT);
@@ -279,20 +314,20 @@ impl<D: SolverDelegate<Interner = I>, I: Interner> TypeFolder<I> for Canonicaliz
             }
         };
 
-        let existing_bound_var = match self.canonicalize_mode {
-            CanonicalizeMode::Input => None,
+        let var = match self.canonicalize_mode {
+            CanonicalizeMode::Input => {
+                // It's fine to not add `r` to the lookup table, as we will
+                // never lookup regions when canonicalizing inputs.
+                let var = ty::BoundVar::from(self.variables.len());
+                self.variables.push(r.into());
+                self.primitive_var_infos.push(CanonicalVarInfo { kind });
+                var
+            }
             CanonicalizeMode::Response { .. } => {
-                self.variables.iter().position(|&v| v == r.into()).map(ty::BoundVar::from)
+                self.get_or_insert_bound_var(r, CanonicalVarInfo { kind })
             }
         };
 
-        let var = existing_bound_var.unwrap_or_else(|| {
-            let var = ty::BoundVar::from(self.variables.len());
-            self.variables.push(r.into());
-            self.primitive_var_infos.push(CanonicalVarInfo { kind });
-            var
-        });
-
         Region::new_anon_bound(self.cx(), self.binder_index, var)
     }
 
@@ -373,14 +408,7 @@ impl<D: SolverDelegate<Interner = I>, I: Interner> TypeFolder<I> for Canonicaliz
             | ty::Error(_) => return t.super_fold_with(self),
         };
 
-        let var = ty::BoundVar::from(
-            self.variables.iter().position(|&v| v == t.into()).unwrap_or_else(|| {
-                let var = self.variables.len();
-                self.variables.push(t.into());
-                self.primitive_var_infos.push(CanonicalVarInfo { kind });
-                var
-            }),
-        );
+        let var = self.get_or_insert_bound_var(t, CanonicalVarInfo { kind });
 
         Ty::new_anon_bound(self.cx(), self.binder_index, var)
     }
@@ -421,14 +449,7 @@ impl<D: SolverDelegate<Interner = I>, I: Interner> TypeFolder<I> for Canonicaliz
             | ty::ConstKind::Expr(_) => return c.super_fold_with(self),
         };
 
-        let var = ty::BoundVar::from(
-            self.variables.iter().position(|&v| v == c.into()).unwrap_or_else(|| {
-                let var = self.variables.len();
-                self.variables.push(c.into());
-                self.primitive_var_infos.push(CanonicalVarInfo { kind });
-                var
-            }),
-        );
+        let var = self.get_or_insert_bound_var(c, CanonicalVarInfo { kind });
 
         Const::new_anon_bound(self.cx(), self.binder_index, var)
     }

compiler/rustc_session/src/config.rs

@@ -823,6 +823,11 @@ pub struct NextSolverConfig {
     /// This is only `true` if `coherence` is also enabled.
     pub globally: bool,
 }
+impl Default for NextSolverConfig {
+    fn default() -> Self {
+        NextSolverConfig { coherence: true, globally: false }
+    }
+}
 
 #[derive(Clone)]
 pub enum Input {

compiler/rustc_session/src/options.rs

@@ -402,7 +402,7 @@ mod desc {
     pub const parse_unpretty: &str = "`string` or `string=string`";
     pub const parse_treat_err_as_bug: &str = "either no value or a non-negative number";
     pub const parse_next_solver_config: &str =
-        "a comma separated list of solver configurations: `globally` (default), and `coherence`";
+        "either `globally` (when used without an argument), `coherence` (default) or `no`";
     pub const parse_lto: &str =
         "either a boolean (`yes`, `no`, `on`, `off`, etc), `thin`, `fat`, or omitted";
     pub const parse_linker_plugin_lto: &str =
@@ -1088,27 +1088,16 @@ mod parse {
         }
     }
 
-    pub(crate) fn parse_next_solver_config(
-        slot: &mut Option<NextSolverConfig>,
-        v: Option<&str>,
-    ) -> bool {
+    pub(crate) fn parse_next_solver_config(slot: &mut NextSolverConfig, v: Option<&str>) -> bool {
         if let Some(config) = v {
-            let mut coherence = false;
-            let mut globally = true;
-            for c in config.split(',') {
-                match c {
-                    "globally" => globally = true,
-                    "coherence" => {
-                        globally = false;
-                        coherence = true;
-                    }
-                    _ => return false,
-                }
-            }
-
-            *slot = Some(NextSolverConfig { coherence: coherence || globally, globally });
+            *slot = match config {
+                "no" => NextSolverConfig { coherence: false, globally: false },
+                "coherence" => NextSolverConfig { coherence: true, globally: false },
+                "globally" => NextSolverConfig { coherence: true, globally: true },
+                _ => return false,
+            };
         } else {
-            *slot = Some(NextSolverConfig { coherence: true, globally: true });
+            *slot = NextSolverConfig { coherence: true, globally: true };
         }
 
         true
@@ -1842,7 +1831,7 @@ options! {
         "the size at which the `large_assignments` lint starts to be emitted"),
     mutable_noalias: bool = (true, parse_bool, [TRACKED],
         "emit noalias metadata for mutable references (default: yes)"),
-    next_solver: Option<NextSolverConfig> = (None, parse_next_solver_config, [TRACKED],
+    next_solver: NextSolverConfig = (NextSolverConfig::default(), parse_next_solver_config, [TRACKED],
         "enable and configure the next generation trait solver used by rustc"),
     nll_facts: bool = (false, parse_bool, [UNTRACKED],
         "dump facts from NLL analysis into side files (default: no)"),

compiler/rustc_trait_selection/src/traits/engine.rs

@@ -35,10 +35,8 @@ where
         if infcx.next_trait_solver() {
             Box::new(NextFulfillmentCtxt::new(infcx))
         } else {
-            let new_solver_globally =
-                infcx.tcx.sess.opts.unstable_opts.next_solver.map_or(false, |c| c.globally);
             assert!(
-                !new_solver_globally,
+                !infcx.tcx.next_trait_solver_globally(),
                 "using old solver even though new solver is enabled globally"
             );
             Box::new(FulfillmentContext::new(infcx))

tests/crashes/118987.rs

@@ -1,20 +1,20 @@
-error[E0119]: conflicting implementations of trait `IntoCow<'_, _>` for type `Cow<'_, _>`
+error[E0119]: conflicting implementations of trait `IntoCow<'_, _>` for type `<_ as ToOwned>::Owned`
   --> $DIR/associated-types-coherence-failure.rs:21:1
    |
 LL | impl<'a, B: ?Sized> IntoCow<'a, B> for <B as ToOwned>::Owned where B: ToOwned {
    | ----------------------------------------------------------------------------- first implementation here
 ...
 LL | impl<'a, B: ?Sized> IntoCow<'a, B> for Cow<'a, B> where B: ToOwned {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `Cow<'_, _>`
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `<_ as ToOwned>::Owned`
 
-error[E0119]: conflicting implementations of trait `IntoCow<'_, _>` for type `&_`
+error[E0119]: conflicting implementations of trait `IntoCow<'_, _>` for type `<_ as ToOwned>::Owned`
   --> $DIR/associated-types-coherence-failure.rs:28:1
    |
 LL | impl<'a, B: ?Sized> IntoCow<'a, B> for <B as ToOwned>::Owned where B: ToOwned {
    | ----------------------------------------------------------------------------- first implementation here
 ...
 LL | impl<'a, B: ?Sized> IntoCow<'a, B> for &'a B where B: ToOwned {
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `&_`
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `<_ as ToOwned>::Owned`
 
 error: aborting due to 2 previous errors
 

tests/ui/associated-types/associated-types-coherence-failure.stderr

@@ -5,6 +5,8 @@ LL | impl<'a, T: MyPredicate<'a>> MyTrait<'a> for T {}
    | ---------------------------------------------- first implementation here
 LL | impl<'a, T> MyTrait<'a> for &'a T {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `&_`
+   |
+   = note: downstream crates may implement trait `MyPredicate<'_>` for type `&_`
 
 error: aborting due to 1 previous error