Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions compiler/rustc_const_eval/src/interpret/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {

// Compute callee information.
// FIXME: for variadic support, do we have to somehow determine callee's extra_args?
let callee_fn_abi = self.fn_abi_of_instance(instance, ty::List::empty())?;
let callee_fn_abi =
self.fn_abi_of_instance_no_deduced_attrs(instance, ty::List::empty())?;

if callee_fn_abi.c_variadic || caller_fn_abi.c_variadic {
throw_unsup_format!("calling a c-variadic function is not supported");
Expand Down Expand Up @@ -839,7 +840,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
enter_trace_span!(M, resolve::resolve_drop_in_place, ty = ?place.layout.ty);
ty::Instance::resolve_drop_in_place(*self.tcx, place.layout.ty)
};
let fn_abi = self.fn_abi_of_instance(instance, ty::List::empty())?;
let fn_abi = self.fn_abi_of_instance_no_deduced_attrs(instance, ty::List::empty())?;

let arg = self.mplace_to_ref(&place)?;
let ret = MPlaceTy::fake_alloc_zst(self.layout_of(self.tcx.types.unit)?);
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,16 +153,16 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
}

/// This inherent method takes priority over the trait method with the same name in FnAbiOf,
/// and allows wrapping the actual [FnAbiOf::fn_abi_of_instance] with a tracing span.
/// See [FnAbiOf::fn_abi_of_instance] for the original documentation.
/// and allows wrapping the actual [FnAbiOf::fn_abi_of_instance_no_deduced_attrs] with a tracing span.
/// See [FnAbiOf::fn_abi_of_instance_no_deduced_attrs] for the original documentation.
#[inline(always)]
pub fn fn_abi_of_instance(
pub fn fn_abi_of_instance_no_deduced_attrs(
&self,
instance: ty::Instance<'tcx>,
extra_args: &'tcx ty::List<Ty<'tcx>>,
) -> <Self as FnAbiOfHelpers<'tcx>>::FnAbiOfResult {
let _trace = enter_trace_span!(M, layouting::fn_abi_of_instance, ?instance, ?extra_args);
FnAbiOf::fn_abi_of_instance(self, instance, extra_args)
FnAbiOf::fn_abi_of_instance_no_deduced_attrs(self, instance, extra_args)
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
let instance = self.resolve(def_id, args)?;
(
FnVal::Instance(instance),
self.fn_abi_of_instance(instance, extra_args)?,
self.fn_abi_of_instance_no_deduced_attrs(instance, extra_args)?,
instance.def.requires_caller_location(*self.tcx),
)
}
Expand Down
29 changes: 25 additions & 4 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1811,11 +1811,32 @@ rustc_queries! {
desc { "computing call ABI of `{}` function pointers", key.value.0 }
}

/// Compute a `FnAbi` suitable for declaring/defining an `fn` instance, and for
/// direct calls to an `fn`.
/// Compute a `FnAbi` suitable for declaring/defining an `fn` instance, and for direct calls*
/// to an `fn`. Indirectly-passed parameters in the returned ABI might not include all possible
/// codegen optimization attributes (such as `ReadOnly` or `CapturesNone`), as deducing these
/// requires inspection of function bodies that can lead to cycles when performed during typeck.
/// Post typeck, you should prefer the optimized ABI returned by `fn_abi_of_instance`.
Comment on lines +1815 to +1818
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// to an `fn`. Indirectly-passed parameters in the returned ABI might not include all possible
/// codegen optimization attributes (such as `ReadOnly` or `CapturesNone`), as deducing these
/// requires inspection of function bodies that can lead to cycles when performed during typeck.
/// Post typeck, you should prefer the optimized ABI returned by `fn_abi_of_instance`.
/// to an `fn`. This query does *not* look at the function body to deduce further
/// attributes for function arguments as doing so can lead to cycles during typeck.
/// Post typeck, you should prefer the optimized ABI returned by `fn_abi_of_instance`.

Let's not list what exactly gets put into "deduced attrs" as that will inevitably expand in the future.

///
/// NB: the ABI returned by this query must not differ from that returned by
/// `fn_abi_of_instance` in any other way.
///
/// * that includes virtual calls, which are represented by "direct calls" to an
/// `InstanceKind::Virtual` instance (of `<dyn Trait as Trait>::fn`).
query fn_abi_of_instance_no_deduced_attrs(
key: ty::PseudoCanonicalInput<'tcx, (ty::Instance<'tcx>, &'tcx ty::List<Ty<'tcx>>)>
) -> Result<&'tcx rustc_target::callconv::FnAbi<'tcx, Ty<'tcx>>, &'tcx ty::layout::FnAbiError<'tcx>> {
desc { "computing unadjusted call ABI of `{}`", key.value.0 }
}

/// Compute a `FnAbi` suitable for declaring/defining an `fn` instance, and for direct calls*
/// to an `fn`. Indirectly-passed parameters in the returned ABI will include applicable
/// codegen optimization attributes, including `ReadOnly` and `CapturesNone` -- deduction of
/// which requires inspection of function bodies that can lead to cycles when performed during
/// typeck. During typeck, you should therefore use instead the unoptimized ABI returned by
/// `fn_abi_of_instance_no_deduced_attrs`.
Comment on lines +1831 to +1836
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Compute a `FnAbi` suitable for declaring/defining an `fn` instance, and for direct calls*
/// to an `fn`. Indirectly-passed parameters in the returned ABI will include applicable
/// codegen optimization attributes, including `ReadOnly` and `CapturesNone` -- deduction of
/// which requires inspection of function bodies that can lead to cycles when performed during
/// typeck. During typeck, you should therefore use instead the unoptimized ABI returned by
/// `fn_abi_of_instance_no_deduced_attrs`.
/// Compute a `FnAbi` suitable for declaring/defining an `fn` instance, and for direct calls*
/// to an `fn`. This query looks at the function body to deduce further attributes, which
/// can lead to cycles when performed during typeck. During typeck, you should therefore
/// instead use `fn_abi_of_instance_no_deduced_attrs`.

///
/// NB: that includes virtual calls, which are represented by "direct calls"
/// to an `InstanceKind::Virtual` instance (of `<dyn Trait as Trait>::fn`).
/// * that includes virtual calls, which are represented by "direct calls" to an
/// `InstanceKind::Virtual` instance (of `<dyn Trait as Trait>::fn`).
query fn_abi_of_instance(
key: ty::PseudoCanonicalInput<'tcx, (ty::Instance<'tcx>, &'tcx ty::List<Ty<'tcx>>)>
) -> Result<&'tcx rustc_target::callconv::FnAbi<'tcx, Ty<'tcx>>, &'tcx ty::layout::FnAbiError<'tcx>> {
Expand Down
53 changes: 49 additions & 4 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1360,11 +1360,56 @@ pub trait FnAbiOf<'tcx>: FnAbiOfHelpers<'tcx> {
)
}

/// Compute a `FnAbi` suitable for declaring/defining an `fn` instance, and for
/// direct calls to an `fn`.
/// Compute a `FnAbi` suitable for declaring/defining an `fn` instance, and for direct calls*
/// to an `fn`. Indirectly-passed parameters in the returned ABI might not include all possible
/// codegen optimization attributes (such as `ReadOnly` or `CapturesNone`), as deducing these
/// requires inspection of function bodies that can lead to cycles when performed during typeck.
/// Post typeck, you should prefer the optimized ABI returned by `fn_abi_of_instance`.
///
/// NB: that includes virtual calls, which are represented by "direct calls"
/// to an `InstanceKind::Virtual` instance (of `<dyn Trait as Trait>::fn`).
/// NB: the ABI returned by this query must not differ from that returned by
/// `fn_abi_of_instance` in any other way.
///
/// * that includes virtual calls, which are represented by "direct calls" to an
/// `InstanceKind::Virtual` instance (of `<dyn Trait as Trait>::fn`).
#[inline]
#[tracing::instrument(level = "debug", skip(self))]
fn fn_abi_of_instance_no_deduced_attrs(
&self,
instance: ty::Instance<'tcx>,
extra_args: &'tcx ty::List<Ty<'tcx>>,
) -> Self::FnAbiOfResult {
// FIXME(eddyb) get a better `span` here.
let span = self.layout_tcx_at_span();
let tcx = self.tcx().at(span);

MaybeResult::from(
tcx.fn_abi_of_instance_no_deduced_attrs(
self.typing_env().as_query_input((instance, extra_args)),
)
.map_err(|err| {
// HACK(eddyb) at least for definitions of/calls to `Instance`s,
// we can get some kind of span even if one wasn't provided.
// However, we don't do this early in order to avoid calling
// `def_span` unconditionally (which may have a perf penalty).
let span = if !span.is_dummy() { span } else { tcx.def_span(instance.def_id()) };
self.handle_fn_abi_err(
*err,
span,
FnAbiRequest::OfInstance { instance, extra_args },
)
}),
)
}

/// Compute a `FnAbi` suitable for declaring/defining an `fn` instance, and for direct calls*
/// to an `fn`. Indirectly-passed parameters in the returned ABI will include applicable
/// codegen optimization attributes, including `ReadOnly` and `CapturesNone` -- deduction of
/// which requires inspection of function bodies that can lead to cycles when performed during
/// typeck. During typeck, you should therefore use instead the unoptimized ABI returned by
/// `fn_abi_of_instance_no_deduced_attrs`.
///
/// * that includes virtual calls, which are represented by "direct calls" to an
/// `InstanceKind::Virtual` instance (of `<dyn Trait as Trait>::fn`).
#[inline]
#[tracing::instrument(level = "debug", skip(self))]
fn fn_abi_of_instance(
Expand Down
154 changes: 100 additions & 54 deletions compiler/rustc_ty_utils/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@ use rustc_target::callconv::{
use tracing::debug;

pub(crate) fn provide(providers: &mut Providers) {
*providers = Providers { fn_abi_of_fn_ptr, fn_abi_of_instance, ..*providers };
*providers = Providers {
fn_abi_of_fn_ptr,
fn_abi_of_instance_no_deduced_attrs,
fn_abi_of_instance,
..*providers
};
}

// NOTE(eddyb) this is private to avoid using it from outside of
Expand Down Expand Up @@ -250,25 +255,71 @@ fn fn_abi_of_fn_ptr<'tcx>(
let ty::PseudoCanonicalInput { typing_env, value: (sig, extra_args) } = query;
fn_abi_new_uncached(
&LayoutCx::new(tcx, typing_env),
tcx.instantiate_bound_regions_with_erased(sig),
extra_args,
tcx.normalize_erasing_regions(typing_env, tcx.instantiate_bound_regions_with_erased(sig)),
None,
None,
false,
extra_args,
)
}

fn fn_abi_of_instance<'tcx>(
fn instance_params<'tcx>(
tcx: TyCtxt<'tcx>,
query: ty::PseudoCanonicalInput<'tcx, (ty::Instance<'tcx>, &'tcx ty::List<Ty<'tcx>>)>,
) -> Result<&'tcx FnAbi<'tcx, Ty<'tcx>>, &'tcx FnAbiError<'tcx>> {
) -> (LayoutCx<'tcx>, ty::FnSig<'tcx>, Option<DefId>, Option<Ty<'tcx>>, bool, &'tcx [Ty<'tcx>]) {
let ty::PseudoCanonicalInput { typing_env, value: (instance, extra_args) } = query;
let cx = LayoutCx::new(tcx, typing_env);
let sig =
tcx.normalize_erasing_regions(typing_env, fn_sig_for_fn_abi(tcx, instance, typing_env));
let is_virtual_call = matches!(instance.def, ty::InstanceKind::Virtual(..));
let is_tls_shim_call = matches!(instance.def, ty::InstanceKind::ThreadLocalShim(_));
(
cx,
sig,
(!is_virtual_call && !is_tls_shim_call).then(|| instance.def_id()),
instance.def.requires_caller_location(tcx).then(|| tcx.caller_location_ty()),
is_virtual_call,
extra_args,
)
}

fn fn_abi_of_instance_no_deduced_attrs<'tcx>(
tcx: TyCtxt<'tcx>,
query: ty::PseudoCanonicalInput<'tcx, (ty::Instance<'tcx>, &'tcx ty::List<Ty<'tcx>>)>,
) -> Result<&'tcx FnAbi<'tcx, Ty<'tcx>>, &'tcx FnAbiError<'tcx>> {
let (cx, sig, determined_fn_def_id, caller_location, is_virtual_call, extra_args) =
instance_params(tcx, query);
fn_abi_new_uncached(
&LayoutCx::new(tcx, typing_env),
fn_sig_for_fn_abi(tcx, instance, typing_env),
&cx,
sig,
caller_location,
determined_fn_def_id,
is_virtual_call,
extra_args,
Some(instance),
)
}

fn fn_abi_of_instance<'tcx>(
tcx: TyCtxt<'tcx>,
query: ty::PseudoCanonicalInput<'tcx, (ty::Instance<'tcx>, &'tcx ty::List<Ty<'tcx>>)>,
) -> Result<&'tcx FnAbi<'tcx, Ty<'tcx>>, &'tcx FnAbiError<'tcx>> {
tcx.fn_abi_of_instance_no_deduced_attrs(query).map(|fn_abi| {
let (cx, sig, determined_fn_def_id, ..) = instance_params(tcx, query);
determined_fn_def_id.map_or(fn_abi, |determined_fn_def_id| {
fn_abi_adjust_for_deduced_attrs(
&cx,
fn_abi,
sig.abi,
// If this is a virtual call, we cannot pass the `fn_def_id`, as it might call other
// functions from vtable. And for a tls shim, passing the `fn_def_id` would refer to
// the underlying static. Internally, `deduced_param_attrs` attempts to infer attributes
// by visit the function body.
determined_fn_def_id,
)
})
})
}

// Handle safe Rust thin and wide pointers.
fn arg_attrs_for_rust_scalar<'tcx>(
cx: LayoutCx<'tcx>,
Expand Down Expand Up @@ -479,27 +530,19 @@ fn fn_abi_sanity_check<'tcx>(
fn_arg_sanity_check(cx, fn_abi, spec_abi, &fn_abi.ret);
}

#[tracing::instrument(level = "debug", skip(cx, instance))]
#[tracing::instrument(
level = "debug",
skip(cx, caller_location, determined_fn_def_id, is_virtual_call)
)]
fn fn_abi_new_uncached<'tcx>(
cx: &LayoutCx<'tcx>,
sig: ty::FnSig<'tcx>,
caller_location: Option<Ty<'tcx>>,
determined_fn_def_id: Option<DefId>,
is_virtual_call: bool,
extra_args: &[Ty<'tcx>],
instance: Option<ty::Instance<'tcx>>,
) -> Result<&'tcx FnAbi<'tcx, Ty<'tcx>>, &'tcx FnAbiError<'tcx>> {
let tcx = cx.tcx();
let (caller_location, determined_fn_def_id, is_virtual_call) = if let Some(instance) = instance
{
let is_virtual_call = matches!(instance.def, ty::InstanceKind::Virtual(..));
let is_tls_shim_call = matches!(instance.def, ty::InstanceKind::ThreadLocalShim(_));
(
instance.def.requires_caller_location(tcx).then(|| tcx.caller_location_ty()),
if is_virtual_call || is_tls_shim_call { None } else { Some(instance.def_id()) },
is_virtual_call,
)
} else {
(None, None, false)
};
let sig = tcx.normalize_erasing_regions(cx.typing_env, sig);

let abi_map = AbiMap::from_target(&tcx.sess.target);
let conv = abi_map.canonize_abi(sig.abi, sig.c_variadic).unwrap();
Expand Down Expand Up @@ -575,16 +618,7 @@ fn fn_abi_new_uncached<'tcx>(
sig.abi,
),
};
fn_abi_adjust_for_abi(
cx,
&mut fn_abi,
sig.abi,
// If this is a virtual call, we cannot pass the `fn_def_id`, as it might call other
// functions from vtable. And for a tls shim, passing the `fn_def_id` would refer to
// the underlying static. Internally, `deduced_param_attrs` attempts to infer attributes
// by visit the function body.
determined_fn_def_id,
);
fn_abi_adjust_for_abi(cx, &mut fn_abi, sig.abi);
debug!("fn_abi_new_uncached = {:?}", fn_abi);
fn_abi_sanity_check(cx, &fn_abi, sig.abi);
Ok(tcx.arena.alloc(fn_abi))
Expand All @@ -595,7 +629,6 @@ fn fn_abi_adjust_for_abi<'tcx>(
cx: &LayoutCx<'tcx>,
fn_abi: &mut FnAbi<'tcx, Ty<'tcx>>,
abi: ExternAbi,
fn_def_id: Option<DefId>,
) {
if abi == ExternAbi::Unadjusted {
// The "unadjusted" ABI passes aggregates in "direct" mode. That's fragile but needed for
Expand All @@ -616,30 +649,43 @@ fn fn_abi_adjust_for_abi<'tcx>(
for arg in fn_abi.args.iter_mut() {
unadjust(arg);
}
return;
} else if abi.is_rustic_abi() {
fn_abi.adjust_for_rust_abi(cx);
} else {
fn_abi.adjust_for_foreign_abi(cx, abi);
}
}

#[tracing::instrument(level = "trace", skip(cx))]
fn fn_abi_adjust_for_deduced_attrs<'tcx>(
cx: &LayoutCx<'tcx>,
fn_abi: &'tcx FnAbi<'tcx, Ty<'tcx>>,
abi: ExternAbi,
fn_def_id: DefId,
) -> &'tcx FnAbi<'tcx, Ty<'tcx>> {
let tcx = cx.tcx();

if abi.is_rustic_abi() {
fn_abi.adjust_for_rust_abi(cx);
// Look up the deduced parameter attributes for this function, if we have its def ID and
// we're optimizing in non-incremental mode. We'll tag its parameters with those attributes
// as appropriate.
let deduced =
if tcx.sess.opts.optimize != OptLevel::No && tcx.sess.opts.incremental.is_none() {
fn_def_id.map(|fn_def_id| tcx.deduced_param_attrs(fn_def_id)).unwrap_or_default()
} else {
&[]
};
if !deduced.is_empty() {
apply_deduced_attributes(cx, deduced, 0, &mut fn_abi.ret);
for (arg_idx, arg) in fn_abi.args.iter_mut().enumerate() {
apply_deduced_attributes(cx, deduced, arg_idx + 1, arg);
}
}
// Look up the deduced parameter attributes for this function, if we have its def ID and
// we're optimizing in non-incremental mode. We'll tag its parameters with those attributes
// as appropriate.
let deduced = if abi.is_rustic_abi()
&& tcx.sess.opts.optimize != OptLevel::No
&& tcx.sess.opts.incremental.is_none()
{
tcx.deduced_param_attrs(fn_def_id)
} else {
fn_abi.adjust_for_foreign_abi(cx, abi);
&[]
};
if deduced.is_empty() {
fn_abi
} else {
let mut fn_abi = fn_abi.clone();
apply_deduced_attributes(cx, deduced, 0, &mut fn_abi.ret);
for (arg_idx, arg) in fn_abi.args.iter_mut().enumerate() {
apply_deduced_attributes(cx, deduced, arg_idx + 1, arg);
}
debug!("fn_abi_adjust_for_deduced_attrs = {:?}", fn_abi);
fn_abi_sanity_check(cx, &fn_abi, abi);
tcx.arena.alloc(fn_abi)
}
}

Expand Down
18 changes: 18 additions & 0 deletions tests/ui/coroutine/avoid-query-cycle-in-ctfe.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//! Items whose type depends on CTFE (such as the async closure/coroutine beneath, whose type
//! depends upon evaluating `do_nothing`) should not cause a query cycle owing to the deduction of
//! the function's parameter attributes, which are only required for codegen and not for CTFE.
//!
//! Regression test for https://github.com/rust-lang/rust/issues/151748
//@ compile-flags: -O
//@ edition: 2018
//@ check-pass

fn main() {
let _ = async || {
let COMPLEX_CONSTANT = ();
};
}

const fn do_nothing() {}

const COMPLEX_CONSTANT: () = do_nothing();
Loading