review

Author: lcnr

compiler/rustc_codegen_llvm/src/debuginfo/mod.rs

@@ -499,7 +499,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
                         ty::Adt(def, ..) if !def.is_box() => {
                             // Again, only create type information if full debuginfo is enabled
                             if cx.sess().opts.debuginfo == DebugInfo::Full
-                                && !impl_self_ty.needs_subst(cx.tcx)
+                                && !impl_self_ty.definitely_needs_subst(cx.tcx)
                             {
                                 Some(type_metadata(cx, impl_self_ty, rustc_span::DUMMY_SP))
                             } else {

compiler/rustc_infer/src/infer/canonical/canonicalizer.rs

@@ -470,7 +470,7 @@ impl<'cx, 'tcx> Canonicalizer<'cx, 'tcx> {
     {
         let needs_canonical_flags = if canonicalize_region_mode.any() {
             TypeFlags::NEEDS_INFER |
-            TypeFlags::HAS_POTENTIAL_FREE_REGIONS | // `HAS_RE_PLACEHOLDER` implies `HAS_xxx_FREE_REGIONS`
+            TypeFlags::HAS_POTENTIAL_FREE_REGIONS | // `HAS_RE_PLACEHOLDER` implies `HAS_POTENTIAL_FREE_REGIONS`
             TypeFlags::HAS_TY_PLACEHOLDER |
             TypeFlags::HAS_CT_PLACEHOLDER
         } else {

compiler/rustc_lint/src/noop_method_call.rs

@@ -62,7 +62,7 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
             _ => return,
         };
         let substs = cx.typeck_results().node_substs(expr.hir_id);
-        if substs.needs_subst(cx.tcx) {
+        if substs.definitely_needs_subst(cx.tcx) {
             // We can't resolve on types that require monomorphization, so we don't handle them if
             // we need to perfom substitution.
             return;

compiler/rustc_middle/src/mir/mod.rs

@@ -285,7 +285,7 @@ impl<'tcx> Body<'tcx> {
             predecessor_cache: PredecessorCache::new(),
             is_cyclic: GraphIsCyclicCache::new(),
         };
-        body.is_polymorphic = body.has_param_types_or_consts(tcx);
+        body.is_polymorphic = body.definitely_has_param_types_or_consts(tcx);
         body
     }
 

compiler/rustc_middle/src/ty/flags.rs

@@ -311,9 +311,20 @@ impl FlagComputation {
     }
 
     fn add_unevaluated_const<P>(&mut self, ct: ty::Unevaluated<'tcx, P>) {
+        // The generic arguments of unevaluated consts are a bit special,
+        // see the `rustc-dev-guide` for more information.
+        //
+        // FIXME(@lcnr): Actually add a link here.
         if let Some(substs) = ct.substs_ {
+            // If they are available, we treat them as ordinary generic arguments.
             self.add_substs(substs);
         } else {
+            // Otherwise, we add `HAS_UNKNOWN_DEFAULT_CONST_SUBSTS` to signify
+            // that our const may potentially refer to generic parameters.
+            //
+            // Note that depending on which generic parameters are actually
+            // used in this constant, we may not actually refer to any generic
+            // parameters at all.
             self.add_flags(TypeFlags::STILL_FURTHER_SPECIALIZABLE);
             self.add_flags(TypeFlags::HAS_UNKNOWN_DEFAULT_CONST_SUBSTS);
         }

compiler/rustc_middle/src/ty/fold.rs

@@ -92,14 +92,14 @@ pub trait TypeFoldable<'tcx>: fmt::Debug + Clone {
     fn references_error(&self) -> bool {
         self.has_type_flags(TypeFlags::HAS_ERROR)
     }
-    fn has_potential_param_types_or_consts(&self) -> bool {
+    fn potentially_has_param_types_or_consts(&self) -> bool {
         self.has_type_flags(
             TypeFlags::HAS_KNOWN_TY_PARAM
                 | TypeFlags::HAS_KNOWN_CT_PARAM
                 | TypeFlags::HAS_UNKNOWN_DEFAULT_CONST_SUBSTS,
         )
     }
-    fn has_param_types_or_consts(&self, tcx: TyCtxt<'tcx>) -> bool {
+    fn definitely_has_param_types_or_consts(&self, tcx: TyCtxt<'tcx>) -> bool {
         self.definitely_has_type_flags(
             tcx,
             TypeFlags::HAS_KNOWN_TY_PARAM | TypeFlags::HAS_KNOWN_CT_PARAM,
@@ -129,7 +129,7 @@ pub trait TypeFoldable<'tcx>: fmt::Debug + Clone {
             TypeFlags::KNOWN_NEEDS_SUBST | TypeFlags::HAS_UNKNOWN_DEFAULT_CONST_SUBSTS,
         )
     }
-    fn needs_subst(&self, tcx: TyCtxt<'tcx>) -> bool {
+    fn definitely_needs_subst(&self, tcx: TyCtxt<'tcx>) -> bool {
         self.definitely_has_type_flags(tcx, TypeFlags::KNOWN_NEEDS_SUBST)
     }
     /// "Free" regions in this context means that it has any region
@@ -227,10 +227,13 @@ pub trait TypeVisitor<'tcx>: Sized {
     /// Supplies the `tcx` for an unevaluated anonymous constant in case its default substs
     /// are not yet supplied.
     ///
-    /// Visitors which do not look into these substs may return `None` here, in which case
-    /// `super_visit_with` completely skips the default substs. Incorrectly returning
-    /// `None` can very quickly lead to ICE or other critical bugs, so be careful and
-    /// try to return an actual `tcx` if at all possible.
+    /// Returning `None` for this method is only recommended if the `TypeVisitor`
+    /// does not care about default anon const substs, as it ignores generic parameters,
+    /// and fetching the default substs would cause a query cycle.
+    ///
+    /// For visitors which return `None` we completely skip the default substs in `ty::Unevaluated::super_visit_with`.
+    /// This means that incorrectly returning `None` can very quickly lead to ICE or other critical bugs, so be careful and
+    /// try to return an actual `tcx` if possible.
     fn tcx_for_anon_const_substs(&self) -> Option<TyCtxt<'tcx>>;
 
     fn visit_binder<T: TypeFoldable<'tcx>>(

compiler/rustc_middle/src/ty/layout.rs

@@ -1727,7 +1727,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
         // Ignore layouts that are done with non-empty environments or
         // non-monomorphic layouts, as the user only wants to see the stuff
         // resulting from the final codegen session.
-        if layout.ty.has_param_types_or_consts(self.tcx)
+        if layout.ty.definitely_has_param_types_or_consts(self.tcx)
             || !self.param_env.caller_bounds().is_empty()
         {
             return;
@@ -1896,7 +1896,7 @@ impl<'tcx> SizeSkeleton<'tcx> {
                 let tail = tcx.struct_tail_erasing_lifetimes(pointee, param_env);
                 match tail.kind() {
                     ty::Param(_) | ty::Projection(_) => {
-                        debug_assert!(tail.has_param_types_or_consts(tcx));
+                        debug_assert!(tail.definitely_has_param_types_or_consts(tcx));
                         Ok(SizeSkeleton::Pointer { non_zero, tail: tcx.erase_regions(tail) })
                     }
                     _ => bug!(

compiler/rustc_middle/src/ty/print/pretty.rs

@@ -1193,7 +1193,7 @@ pub trait PrettyPrinter<'tcx>:
 
             // Aggregates, printed as array/tuple/struct/variant construction syntax.
             //
-            // NB: the `has_param_types_or_consts` check ensures that we can use
+            // NB: the `potentially_has_param_types_or_consts` check ensures that we can use
             // the `destructure_const` query with an empty `ty::ParamEnv` without
             // introducing ICEs (e.g. via `layout_of`) from missing bounds.
             // E.g. `transmute([0usize; 2]): (u8, *mut T)` needs to know `T: Sized`
@@ -1202,7 +1202,7 @@ pub trait PrettyPrinter<'tcx>:
             // FIXME(eddyb) for `--emit=mir`/`-Z dump-mir`, we should provide the
             // correct `ty::ParamEnv` to allow printing *all* constant values.
             (_, ty::Array(..) | ty::Tuple(..) | ty::Adt(..))
-                if !ty.has_potential_param_types_or_consts() =>
+                if !ty.potentially_has_param_types_or_consts() =>
             {
                 let contents = self.tcx().destructure_const(
                     ty::ParamEnv::reveal_all()

compiler/rustc_mir/src/interpret/util.rs

@@ -43,7 +43,7 @@ where
                         let is_used = unused_params.contains(index).map_or(true, |unused| !unused);
                         // Only recurse when generic parameters in fns, closures and generators
                         // are used and require substitution.
-                        match (is_used, subst.needs_subst(self.tcx)) {
+                        match (is_used, subst.definitely_needs_subst(self.tcx)) {
                             // Just in case there are closures or generators within this subst,
                             // recurse.
                             (true, true) => return subst.super_visit_with(self),

compiler/rustc_mir/src/monomorphize/polymorphize.rs

@@ -288,7 +288,7 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for MarkUsedGenericParams<'a, 'tcx> {
     }
     #[instrument(skip(self))]
     fn visit_const(&mut self, c: &'tcx Const<'tcx>) -> ControlFlow<Self::BreakTy> {
-        if !c.has_potential_param_types_or_consts() {
+        if !c.potentially_has_param_types_or_consts() {
             return ControlFlow::CONTINUE;
         }
 
@@ -321,7 +321,7 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for MarkUsedGenericParams<'a, 'tcx> {
 
     #[instrument(skip(self))]
     fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
-        if !ty.has_potential_param_types_or_consts() {
+        if !ty.potentially_has_param_types_or_consts() {
             return ControlFlow::CONTINUE;
         }
 
@@ -363,7 +363,7 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for HasUsedGenericParams<'a, 'tcx> {
 
     #[instrument(skip(self))]
     fn visit_const(&mut self, c: &'tcx Const<'tcx>) -> ControlFlow<Self::BreakTy> {
-        if !c.has_potential_param_types_or_consts() {
+        if !c.potentially_has_param_types_or_consts() {
             return ControlFlow::CONTINUE;
         }
 
@@ -381,7 +381,7 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for HasUsedGenericParams<'a, 'tcx> {
 
     #[instrument(skip(self))]
     fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
-        if !ty.has_potential_param_types_or_consts() {
+        if !ty.potentially_has_param_types_or_consts() {
             return ControlFlow::CONTINUE;
         }
 

compiler/rustc_mir/src/transform/const_prop.rs

@@ -469,7 +469,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
     /// Returns the value, if any, of evaluating `c`.
     fn eval_constant(&mut self, c: &Constant<'tcx>, source_info: SourceInfo) -> Option<OpTy<'tcx>> {
         // FIXME we need to revisit this for #67176
-        if c.needs_subst(self.tcx) {
+        if c.definitely_needs_subst(self.tcx) {
             return None;
         }
 
@@ -489,9 +489,9 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
                             }) => true,
                             // Out of backwards compatibility we cannot report hard errors in unused
                             // generic functions using associated constants of the generic parameters.
-                            _ => c.literal.needs_subst(*tcx),
+                            _ => c.literal.definitely_needs_subst(*tcx),
                         },
-                        ConstantKind::Val(_, ty) => ty.needs_subst(*tcx),
+                        ConstantKind::Val(_, ty) => ty.definitely_needs_subst(*tcx),
                     };
                     if lint_only {
                         // Out of backwards compatibility we cannot report hard errors in unused
@@ -721,7 +721,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
         }
 
         // FIXME we need to revisit this for #67176
-        if rvalue.needs_subst(self.tcx) {
+        if rvalue.definitely_needs_subst(self.tcx) {
             return None;
         }
 

compiler/rustc_mir/src/transform/inline/cycle.rs

@@ -89,7 +89,7 @@ crate fn mir_callgraph_reachable(
                     // FIXME: A not fully substituted drop shim can cause ICEs if one attempts to
                     // have its MIR built. Likely oli-obk just screwed up the `ParamEnv`s, so this
                     // needs some more analysis.
-                    if callee.needs_subst(tcx) {
+                    if callee.definitely_needs_subst(tcx) {
                         continue;
                     }
                 }

compiler/rustc_symbol_mangling/src/legacy.rs

@@ -107,7 +107,7 @@ fn get_symbol_hash<'tcx>(
         tcx.def_path_hash(def_id).hash_stable(&mut hcx, &mut hasher);
 
         // Include the main item-type. Note that, in this case, the
-        // assertions about `needs_subst` may not hold, but this item-type
+        // assertions about `definitely_needs_subst` may not hold, but this item-type
         // ought to be the same for every reference anyway.
         assert!(!item_type.has_erasable_regions(tcx));
         hcx.while_hashing_spans(false, |hcx| {

compiler/rustc_symbol_mangling/src/v0.rs

@@ -277,7 +277,8 @@ impl Printer<'tcx> for &mut SymbolMangler<'tcx> {
 
         // Encode impl generic params if the substitutions contain parameters (implying
         // polymorphization is enabled) and this isn't an inherent impl.
-        if impl_trait_ref.is_some() && substs.iter().any(|a| a.has_param_types_or_consts(self.tcx))
+        if impl_trait_ref.is_some()
+            && substs.iter().any(|a| a.definitely_has_param_types_or_consts(self.tcx))
         {
             self = self.path_generic_args(
                 |this| {

compiler/rustc_trait_selection/src/traits/coherence.rs

@@ -391,7 +391,7 @@ fn orphan_check_trait_ref<'tcx>(
 ) -> Result<(), OrphanCheckErr<'tcx>> {
     debug!("orphan_check_trait_ref(trait_ref={:?}, in_crate={:?})", trait_ref, in_crate);
 
-    if trait_ref.needs_infer() && trait_ref.needs_subst(tcx) {
+    if trait_ref.needs_infer() && trait_ref.definitely_needs_subst(tcx) {
         bug!(
             "can't orphan check a trait ref with both params and inference variables {:?}",
             trait_ref

compiler/rustc_trait_selection/src/traits/const_evaluatable.rs

@@ -85,7 +85,7 @@ pub fn is_const_evaluatable<'cx, 'tcx>(
                         let leaf = leaf.subst(tcx, ct.substs);
                         if leaf.has_infer_types_or_consts() {
                             failure_kind = FailureKind::MentionsInfer;
-                        } else if leaf.has_param_types_or_consts(tcx) {
+                        } else if leaf.definitely_has_param_types_or_consts(tcx) {
                             failure_kind = cmp::min(failure_kind, FailureKind::MentionsParam);
                         }
 
@@ -95,7 +95,7 @@ pub fn is_const_evaluatable<'cx, 'tcx>(
                         let ty = ty.subst(tcx, ct.substs);
                         if ty.has_infer_types_or_consts() {
                             failure_kind = FailureKind::MentionsInfer;
-                        } else if ty.has_param_types_or_consts(tcx) {
+                        } else if ty.definitely_has_param_types_or_consts(tcx) {
                             failure_kind = cmp::min(failure_kind, FailureKind::MentionsParam);
                         }
 
@@ -151,7 +151,7 @@ pub fn is_const_evaluatable<'cx, 'tcx>(
     // See #74595 for more details about this.
     let concrete = infcx.const_eval_resolve(param_env, uv.expand(), Some(span));
 
-    if concrete.is_ok() && uv.substs(infcx.tcx).has_param_types_or_consts(infcx.tcx) {
+    if concrete.is_ok() && uv.substs(infcx.tcx).definitely_has_param_types_or_consts(infcx.tcx) {
         match infcx.tcx.def_kind(uv.def.did) {
             DefKind::AnonConst => {
                 let mir_body = infcx.tcx.mir_for_ctfe_opt_const_arg(uv.def);

compiler/rustc_trait_selection/src/traits/mod.rs

@@ -450,7 +450,7 @@ fn subst_and_check_impossible_predicates<'tcx>(
     debug!("subst_and_check_impossible_predicates(key={:?})", key);
 
     let mut predicates = tcx.predicates_of(key.0).instantiate(tcx, key.1).predicates;
-    predicates.retain(|predicate| !predicate.needs_subst(tcx));
+    predicates.retain(|predicate| !predicate.definitely_needs_subst(tcx));
     let result = impossible_predicates(tcx, predicates);
 
     debug!("subst_and_check_impossible_predicates(key={:?}) = {:?}", key, result);

compiler/rustc_trait_selection/src/traits/select/mod.rs

@@ -696,7 +696,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 .param_env
                 .caller_bounds()
                 .iter()
-                .all(|bound| bound.needs_subst(self.tcx()))
+                .all(|bound| bound.definitely_needs_subst(self.tcx()))
         {
             // If a param env has no global bounds, global obligations do not
             // depend on its particular value in order to work, so we can clear

compiler/rustc_typeck/src/astconv/mod.rs

@@ -2204,7 +2204,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
                 self.prohibit_generics(path.segments);
                 // Try to evaluate any array length constants.
                 let normalized_ty = self.normalize_ty(span, tcx.at(span).type_of(def_id));
-                if forbid_generic && normalized_ty.needs_subst(tcx) {
+                if forbid_generic && normalized_ty.definitely_needs_subst(tcx) {
                     let mut err = tcx.sess.struct_span_err(
                         path.span,
                         "generic `Self` types are currently not permitted in anonymous constants",

compiler/rustc_typeck/src/check/wfcheck.rs

@@ -746,7 +746,7 @@ fn check_where_clauses<'tcx, 'fcx>(
                     // Ignore dependent defaults -- that is, where the default of one type
                     // parameter includes another (e.g., `<T, U = T>`). In those cases, we can't
                     // be sure if it will error or not as user might always specify the other.
-                    if !ty.needs_subst(tcx) {
+                    if !ty.definitely_needs_subst(tcx) {
                         fcx.register_wf_obligation(
                             ty.into(),
                             tcx.def_span(param.def_id),
@@ -762,7 +762,7 @@ fn check_where_clauses<'tcx, 'fcx>(
                     // for `struct Foo<const N: usize, const M: usize = { 1 - 2 }>`
                     // we should eagerly error.
                     let default_ct = tcx.const_param_default(param.def_id);
-                    if !default_ct.needs_subst(tcx) {
+                    if !default_ct.definitely_needs_subst(tcx) {
                         fcx.register_wf_obligation(
                             default_ct.into(),
                             tcx.def_span(param.def_id),
@@ -796,7 +796,7 @@ fn check_where_clauses<'tcx, 'fcx>(
                 if is_our_default(param) {
                     let default_ty = tcx.type_of(param.def_id);
                     // ... and it's not a dependent default, ...
-                    if !default_ty.needs_subst(tcx) {
+                    if !default_ty.definitely_needs_subst(tcx) {
                         // ... then substitute it with the default.
                         return default_ty.into();
                     }
@@ -809,7 +809,7 @@ fn check_where_clauses<'tcx, 'fcx>(
                 if is_our_default(param) {
                     let default_ct = tcx.const_param_default(param.def_id);
                     // ... and it's not a dependent default, ...
-                    if !default_ct.needs_subst(tcx) {
+                    if !default_ct.definitely_needs_subst(tcx) {
                         // ... then substitute it with the default.
                         return default_ct.into();
                     }
@@ -858,7 +858,7 @@ fn check_where_clauses<'tcx, 'fcx>(
             let substituted_pred = pred.subst(tcx, substs);
             // Don't check non-defaulted params, dependent defaults (including lifetimes)
             // or preds with multiple params.
-            if substituted_pred.has_param_types_or_consts(tcx)
+            if substituted_pred.definitely_has_param_types_or_consts(tcx)
                 || param_count.params.len() > 1
                 || has_region
             {

compiler/rustc_typeck/src/collect/type_of.rs

@@ -277,14 +277,21 @@ fn get_path_containing_arg_in_pat<'hir>(
 pub(super) fn default_anon_const_substs(tcx: TyCtxt<'_>, def_id: DefId) -> SubstsRef<'_> {
     let generics = tcx.generics_of(def_id);
     if let Some(parent) = generics.parent {
+        // This is the reason we bother with having optional anon const substs.
+        //
+        // In the future the substs of an anon const will depend on its parents predicates
+        // at which point eagerly looking at them will cause a query cycle.
+        //
+        // So for now this is only an assurance that this approach won't cause cycle errors in
+        // the future.
         let _cycle_check = tcx.predicates_of(parent);
     }
 
     let substs = InternalSubsts::identity_for_item(tcx, def_id);
     // We only expect substs with the following type flags as default substs.
     //
     // Getting this wrong can lead to ICE and unsoundness, so we assert it here.
-    for arg in substs.iter().flat_map(|s| s.walk(tcx)) {
+    for arg in substs.iter() {
         let allowed_flags = ty::TypeFlags::MAY_NEED_DEFAULT_CONST_SUBSTS
             | ty::TypeFlags::STILL_FURTHER_SPECIALIZABLE;
         assert!(!arg.has_type_flags(!allowed_flags));