Skip to content

Commit

Permalink
Unrolled build for #135426
Browse files Browse the repository at this point in the history
Rollup merge of #135426 - compiler-errors:no-resolve-assoc-ty, r=lcnr

Assert that `Instance::try_resolve` is only used on body-like things

`Instance::resolve` is not set up to resolve items that are not body-like things. The logic in `resolve_associated_item` very much encodes this assumption:

https://github.com/rust-lang/rust/blob/e7ad3ae331bf2716389c10e01612e201a7f98c8d/compiler/rustc_ty_utils/src/instance.rs#L96-L386

However, some diagnostics were using `Instance::resolve` on an associated type, and it was simply a lucky coicidence that nothing went wrong.

This PR adds an assertion to make sure we won't do this again in the future, and fixes two callsites:
1. `call_kind` which returns a `CallKind` enum to categorize what a call in MIR comes from, and was using `Instance::resolve` to point at the associated type `Deref::Target` for a specific self ty.
2. `MirBorrowckCtxt::explain_deref_coercion`, which was doing the same thing.

The logic was replaced with `specialization_graph::assoc_def`, which is the proper way of fetching the right `AssocItem` for a given impl.

r? `@lcnr` or re-roll :)
  • Loading branch information
rust-timer authored Jan 14, 2025
2 parents 2ae9916 + 9bf9f5d commit cbba621
Show file tree
Hide file tree
Showing 10 changed files with 116 additions and 97 deletions.
59 changes: 25 additions & 34 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use rustc_middle::bug;
use rustc_middle::hir::nested_filter::OnlyBodies;
use rustc_middle::mir::tcx::PlaceTy;
use rustc_middle::mir::{
self, AggregateKind, BindingForm, BorrowKind, CallSource, ClearCrossCrate, ConstraintCategory,
self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, ConstraintCategory,
FakeBorrowKind, FakeReadCause, LocalDecl, LocalInfo, LocalKind, Location, MutBorrowKind,
Operand, Place, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, Terminator,
TerminatorKind, VarBindingForm, VarDebugInfoContents,
Expand All @@ -30,13 +30,13 @@ use rustc_middle::ty::{
self, PredicateKind, Ty, TyCtxt, TypeSuperVisitable, TypeVisitor, Upcast,
suggest_constraining_type_params,
};
use rustc_middle::util::CallKind;
use rustc_mir_dataflow::move_paths::{InitKind, MoveOutIndex, MovePathIndex};
use rustc_span::def_id::{DefId, LocalDefId};
use rustc_span::hygiene::DesugaringKind;
use rustc_span::{BytePos, Ident, Span, Symbol, kw, sym};
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
use rustc_trait_selection::error_reporting::traits::FindExprBySpan;
use rustc_trait_selection::error_reporting::traits::call_kind::CallKind;
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;
use rustc_trait_selection::traits::{Obligation, ObligationCause, ObligationCtxt};
Expand All @@ -46,7 +46,7 @@ use super::explain_borrow::{BorrowExplanation, LaterUseKind};
use super::{DescribePlaceOpt, RegionName, RegionNameSource, UseSpans};
use crate::borrow_set::{BorrowData, TwoPhaseActivation};
use crate::diagnostics::conflict_errors::StorageDeadOrDrop::LocalStorageDead;
use crate::diagnostics::{CapturedMessageOpt, Instance, find_all_local_uses};
use crate::diagnostics::{CapturedMessageOpt, call_kind, find_all_local_uses};
use crate::prefixes::IsPrefixOf;
use crate::{InitializationRequiringAction, MirBorrowckCtxt, WriteKind, borrowck_errors};

Expand Down Expand Up @@ -305,7 +305,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
}

if let UseSpans::FnSelfUse {
kind: CallKind::DerefCoercion { deref_target, deref_target_ty, .. },
kind: CallKind::DerefCoercion { deref_target_span, deref_target_ty, .. },
..
} = use_spans
{
Expand All @@ -315,8 +315,10 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
));

// Check first whether the source is accessible (issue #87060)
if self.infcx.tcx.sess.source_map().is_span_accessible(deref_target) {
err.span_note(deref_target, "deref defined here");
if let Some(deref_target_span) = deref_target_span
&& self.infcx.tcx.sess.source_map().is_span_accessible(deref_target_span)
{
err.span_note(deref_target_span, "deref defined here");
}
}

Expand Down Expand Up @@ -3765,38 +3767,27 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {

fn explain_deref_coercion(&mut self, loan: &BorrowData<'tcx>, err: &mut Diag<'_>) {
let tcx = self.infcx.tcx;
if let (
Some(Terminator {
kind: TerminatorKind::Call { call_source: CallSource::OverloadedOperator, .. },
..
}),
Some((method_did, method_args)),
) = (
&self.body[loan.reserve_location.block].terminator,
rustc_middle::util::find_self_call(
if let Some(Terminator { kind: TerminatorKind::Call { call_source, fn_span, .. }, .. }) =
&self.body[loan.reserve_location.block].terminator
&& let Some((method_did, method_args)) = rustc_middle::util::find_self_call(
tcx,
self.body,
loan.assigned_place.local,
loan.reserve_location.block,
),
) {
if tcx.is_diagnostic_item(sym::deref_method, method_did) {
let deref_target =
tcx.get_diagnostic_item(sym::deref_target).and_then(|deref_target| {
Instance::try_resolve(
tcx,
self.infcx.typing_env(self.infcx.param_env),
deref_target,
method_args,
)
.transpose()
});
if let Some(Ok(instance)) = deref_target {
let deref_target_ty =
instance.ty(tcx, self.infcx.typing_env(self.infcx.param_env));
err.note(format!("borrow occurs due to deref coercion to `{deref_target_ty}`"));
err.span_note(tcx.def_span(instance.def_id()), "deref defined here");
}
)
&& let CallKind::DerefCoercion { deref_target_span, deref_target_ty, .. } = call_kind(
self.infcx.tcx,
self.infcx.typing_env(self.infcx.param_env),
method_did,
method_args,
*fn_span,
call_source.from_hir_call(),
Some(self.infcx.tcx.fn_arg_names(method_did)[0]),
)
{
err.note(format!("borrow occurs due to deref coercion to `{deref_target_ty}`"));
if let Some(deref_target_span) = deref_target_span {
err.span_note(deref_target_span, "deref defined here");
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ use rustc_middle::mir::{
};
use rustc_middle::ty::adjustment::PointerCoercion;
use rustc_middle::ty::{self, RegionVid, Ty, TyCtxt};
use rustc_middle::util::CallKind;
use rustc_span::{DesugaringKind, Span, kw, sym};
use rustc_trait_selection::error_reporting::traits::FindExprBySpan;
use rustc_trait_selection::error_reporting::traits::call_kind::CallKind;
use tracing::{debug, instrument};

use super::{RegionName, UseSpans, find_use};
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ use rustc_middle::mir::{
StatementKind, Terminator, TerminatorKind,
};
use rustc_middle::ty::print::Print;
use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
use rustc_middle::util::{CallDesugaringKind, call_kind};
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_mir_dataflow::move_paths::{InitLocation, LookupResult, MoveOutIndex};
use rustc_span::def_id::LocalDefId;
use rustc_span::source_map::Spanned;
use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span, Symbol, sym};
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
use rustc_trait_selection::error_reporting::traits::call_kind::{CallDesugaringKind, call_kind};
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::{
FulfillmentErrorCode, type_known_to_meet_bound_modulo_regions,
Expand Down Expand Up @@ -63,7 +63,7 @@ pub(crate) use mutability_errors::AccessKind;
pub(crate) use outlives_suggestion::OutlivesSuggestionBuilder;
pub(crate) use region_errors::{ErrorConstraintInfo, RegionErrorKind, RegionErrors};
pub(crate) use region_name::{RegionName, RegionNameSource};
pub(crate) use rustc_middle::util::CallKind;
pub(crate) use rustc_trait_selection::error_reporting::traits::call_kind::CallKind;

pub(super) struct DescribePlaceOpt {
including_downcast: bool,
Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_const_eval/src/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ use rustc_middle::ty::{
self, Closure, FnDef, FnPtr, GenericArgKind, GenericArgsRef, Param, TraitRef, Ty,
suggest_constraining_type_param,
};
use rustc_middle::util::{CallDesugaringKind, CallKind, call_kind};
use rustc_session::parse::add_feature_diagnostics;
use rustc_span::{BytePos, Pos, Span, Symbol, sym};
use rustc_trait_selection::error_reporting::traits::call_kind::{
CallDesugaringKind, CallKind, call_kind,
};
use rustc_trait_selection::traits::SelectionContext;
use tracing::debug;

Expand Down Expand Up @@ -324,10 +326,12 @@ fn build_error_for_const_call<'tcx>(
note_trait_if_possible(&mut err, self_ty, trait_id);
err
}
CallKind::DerefCoercion { deref_target, deref_target_ty, self_ty } => {
CallKind::DerefCoercion { deref_target_span, deref_target_ty, self_ty } => {
// Check first whether the source is accessible (issue #87060)
let target = if tcx.sess.source_map().is_span_accessible(deref_target) {
Some(deref_target)
let target = if let Some(deref_target_span) = deref_target_span
&& tcx.sess.source_map().is_span_accessible(deref_target_span)
{
Some(deref_target_span)
} else {
None
};
Expand Down
22 changes: 20 additions & 2 deletions compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::path::PathBuf;
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::ErrorGuaranteed;
use rustc_hir as hir;
use rustc_hir::def::Namespace;
use rustc_hir::def::{CtorKind, DefKind, Namespace};
use rustc_hir::def_id::{CrateNum, DefId};
use rustc_hir::lang_items::LangItem;
use rustc_index::bit_set::FiniteBitSet;
Expand Down Expand Up @@ -498,7 +498,8 @@ impl<'tcx> Instance<'tcx> {

/// Resolves a `(def_id, args)` pair to an (optional) instance -- most commonly,
/// this is used to find the precise code that will run for a trait method invocation,
/// if known.
/// if known. This should only be used for functions and consts. If you want to
/// resolve an associated type, use [`TyCtxt::try_normalize_erasing_regions`].
///
/// Returns `Ok(None)` if we cannot resolve `Instance` to a specific instance.
/// For example, in a context like this,
Expand Down Expand Up @@ -527,6 +528,23 @@ impl<'tcx> Instance<'tcx> {
def_id: DefId,
args: GenericArgsRef<'tcx>,
) -> Result<Option<Instance<'tcx>>, ErrorGuaranteed> {
assert_matches!(
tcx.def_kind(def_id),
DefKind::Fn
| DefKind::AssocFn
| DefKind::Const
| DefKind::AssocConst
| DefKind::AnonConst
| DefKind::InlineConst
| DefKind::Static { .. }
| DefKind::Ctor(_, CtorKind::Fn)
| DefKind::Closure
| DefKind::SyntheticCoroutineBody,
"`Instance::try_resolve` should only be used to resolve instances of \
functions, statics, and consts; to resolve associated types, use \
`try_normalize_erasing_regions`."
);

// Rust code can easily create exponentially-long types using only a
// polynomial recursion depth. Even with the default recursion
// depth, you can easily get cases that take >2^60 steps to run,
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_middle/src/util/mod.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
pub mod bug;
pub mod call_kind;
pub mod common;
pub mod find_self_call;

pub use call_kind::{CallDesugaringKind, CallKind, call_kind};
pub use find_self_call::find_self_call;

#[derive(Default, Copy, Clone)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
//! as well as errors when attempting to call a non-const function in a const
//! context.
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
use rustc_hir::{LangItem, lang_items};
use rustc_middle::ty::{AssocItemContainer, GenericArgsRef, Instance, Ty, TyCtxt, TypingEnv};
use rustc_span::{DesugaringKind, Ident, Span, sym};
use tracing::debug;

use crate::ty::{AssocItemContainer, GenericArgsRef, Instance, Ty, TyCtxt, TypingEnv};
use crate::traits::specialization_graph;

#[derive(Clone, Copy, PartialEq, Eq, Debug)]
pub enum CallDesugaringKind {
Expand Down Expand Up @@ -55,7 +57,7 @@ pub enum CallKind<'tcx> {
DerefCoercion {
/// The `Span` of the `Target` associated type
/// in the `Deref` impl we are using.
deref_target: Span,
deref_target_span: Option<Span>,
/// The type `T::Deref` we are dereferencing to
deref_target_ty: Ty<'tcx>,
self_ty: Ty<'tcx>,
Expand Down Expand Up @@ -89,61 +91,65 @@ pub fn call_kind<'tcx>(
None
};

let is_deref = !from_hir_call && tcx.is_diagnostic_item(sym::deref_method, method_did);

// Check for a 'special' use of 'self' -
// an FnOnce call, an operator (e.g. `<<`), or a
// deref coercion.
let kind = if let Some(trait_id) = fn_call {
Some(CallKind::FnCall { fn_trait_id: trait_id, self_ty: method_args.type_at(0) })
if let Some(trait_id) = fn_call {
return CallKind::FnCall { fn_trait_id: trait_id, self_ty: method_args.type_at(0) };
} else if let Some(trait_id) = operator {
Some(CallKind::Operator { self_arg, trait_id, self_ty: method_args.type_at(0) })
} else if is_deref {
let deref_target = tcx.get_diagnostic_item(sym::deref_target).and_then(|deref_target| {
Instance::try_resolve(tcx, typing_env, deref_target, method_args).transpose()
});
if let Some(Ok(instance)) = deref_target {
let deref_target_ty = instance.ty(tcx, typing_env);
Some(CallKind::DerefCoercion {
deref_target: tcx.def_span(instance.def_id()),
deref_target_ty,
self_ty: method_args.type_at(0),
})
return CallKind::Operator { self_arg, trait_id, self_ty: method_args.type_at(0) };
} else if !from_hir_call && tcx.is_diagnostic_item(sym::deref_method, method_did) {
let deref_target_def_id =
tcx.get_diagnostic_item(sym::deref_target).expect("deref method but no deref target");
let deref_target_ty = tcx.normalize_erasing_regions(
typing_env,
Ty::new_projection(tcx, deref_target_def_id, method_args),
);
let deref_target_span = if let Ok(Some(instance)) =
Instance::try_resolve(tcx, typing_env, method_did, method_args)
&& let instance_parent_def_id = tcx.parent(instance.def_id())
&& matches!(tcx.def_kind(instance_parent_def_id), DefKind::Impl { .. })
&& let Ok(instance) =
specialization_graph::assoc_def(tcx, instance_parent_def_id, deref_target_def_id)
&& instance.is_final()
{
Some(tcx.def_span(instance.item.def_id))
} else {
None
};
return CallKind::DerefCoercion {
deref_target_ty,
deref_target_span,
self_ty: method_args.type_at(0),
};
}

// This isn't a 'special' use of `self`
debug!(?method_did, ?fn_call_span);
let desugaring = if tcx.is_lang_item(method_did, LangItem::IntoIterIntoIter)
&& fn_call_span.desugaring_kind() == Some(DesugaringKind::ForLoop)
{
Some((CallDesugaringKind::ForLoopIntoIter, method_args.type_at(0)))
} else if tcx.is_lang_item(method_did, LangItem::IteratorNext)
&& fn_call_span.desugaring_kind() == Some(DesugaringKind::ForLoop)
{
Some((CallDesugaringKind::ForLoopNext, method_args.type_at(0)))
} else if fn_call_span.desugaring_kind() == Some(DesugaringKind::QuestionMark) {
if tcx.is_lang_item(method_did, LangItem::TryTraitBranch) {
Some((CallDesugaringKind::QuestionBranch, method_args.type_at(0)))
} else if tcx.is_lang_item(method_did, LangItem::TryTraitFromResidual) {
Some((CallDesugaringKind::QuestionFromResidual, method_args.type_at(0)))
} else {
None
}
} else if tcx.is_lang_item(method_did, LangItem::TryTraitFromOutput)
&& fn_call_span.desugaring_kind() == Some(DesugaringKind::TryBlock)
{
Some((CallDesugaringKind::TryBlockFromOutput, method_args.type_at(0)))
} else if fn_call_span.is_desugaring(DesugaringKind::Await) {
Some((CallDesugaringKind::Await, method_args.type_at(0)))
} else {
None
};

kind.unwrap_or_else(|| {
// This isn't a 'special' use of `self`
debug!(?method_did, ?fn_call_span);
let desugaring = if tcx.is_lang_item(method_did, LangItem::IntoIterIntoIter)
&& fn_call_span.desugaring_kind() == Some(DesugaringKind::ForLoop)
{
Some((CallDesugaringKind::ForLoopIntoIter, method_args.type_at(0)))
} else if tcx.is_lang_item(method_did, LangItem::IteratorNext)
&& fn_call_span.desugaring_kind() == Some(DesugaringKind::ForLoop)
{
Some((CallDesugaringKind::ForLoopNext, method_args.type_at(0)))
} else if fn_call_span.desugaring_kind() == Some(DesugaringKind::QuestionMark) {
if tcx.is_lang_item(method_did, LangItem::TryTraitBranch) {
Some((CallDesugaringKind::QuestionBranch, method_args.type_at(0)))
} else if tcx.is_lang_item(method_did, LangItem::TryTraitFromResidual) {
Some((CallDesugaringKind::QuestionFromResidual, method_args.type_at(0)))
} else {
None
}
} else if tcx.is_lang_item(method_did, LangItem::TryTraitFromOutput)
&& fn_call_span.desugaring_kind() == Some(DesugaringKind::TryBlock)
{
Some((CallDesugaringKind::TryBlockFromOutput, method_args.type_at(0)))
} else if fn_call_span.is_desugaring(DesugaringKind::Await) {
Some((CallDesugaringKind::Await, method_args.type_at(0)))
} else {
None
};
CallKind::Normal { self_arg, desugaring, method_did, method_args }
})
CallKind::Normal { self_arg, desugaring, method_did, method_args }
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
pub mod ambiguity;
pub mod call_kind;
mod fulfillment_errors;
pub mod on_unimplemented;
mod overflow;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/self/arbitrary-self-from-method-substs-ice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ impl Foo {
//~^ ERROR invalid generic `self` parameter type
//~| ERROR destructor of `R` cannot be evaluated at compile-time
self.0
//~^ ERROR cannot call conditionally-const method `<R as Deref>::deref` in constant function
//~^ ERROR cannot perform conditionally-const deref coercion on `R` in constant functions
}
}

Expand Down
3 changes: 2 additions & 1 deletion tests/ui/self/arbitrary-self-from-method-substs-ice.stderr
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
error[E0658]: cannot call conditionally-const method `<R as Deref>::deref` in constant functions
error[E0658]: cannot perform conditionally-const deref coercion on `R` in constant functions
--> $DIR/arbitrary-self-from-method-substs-ice.rs:13:9
|
LL | self.0
| ^^^^^^
|
= note: attempting to deref into `Foo`
= note: calls in constant functions are limited to constant functions, tuple structs and tuple variants
= note: see issue #67792 <https://github.com/rust-lang/rust/issues/67792> for more information
= help: add `#![feature(const_trait_impl)]` to the crate attributes to enable
Expand Down

0 comments on commit cbba621

Please sign in to comment.