-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
support c-variadic functions in rustc_const_eval
#150601
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
base: main
Are you sure you want to change the base?
Changes from all commits
7cfa4d7
f237efc
1651931
ab1918f
36759a2
ce45670
2ca4269
5d20e76
25e96ba
c0ce0b0
9e57c73
67aabb2
bdc0935
83db12a
902fd15
ab52d77
33fcb97
1bcf89f
a564f24
76b68b1
14fa6a8
1c86e0b
023a038
666adc8
71af2d1
8ad00dc
0e7c74e
0b084d7
e6acf30
ba2ba3a
7a02aa1
e8cda1f
db1bee2
e4bc4f9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ use std::borrow::Cow; | |
| use either::{Left, Right}; | ||
| use rustc_abi::{self as abi, ExternAbi, FieldIdx, Integer, VariantIdx}; | ||
| use rustc_data_structures::assert_matches; | ||
| use rustc_hir::def::DefKind; | ||
| use rustc_hir::def_id::DefId; | ||
| use rustc_middle::ty::layout::{IntegerExt, TyAndLayout}; | ||
| use rustc_middle::ty::{self, AdtDef, Instance, Ty, VariantDef}; | ||
|
|
@@ -17,7 +18,7 @@ use tracing::{info, instrument, trace}; | |
| use super::{ | ||
| CtfeProvenance, FnVal, ImmTy, InterpCx, InterpResult, MPlaceTy, Machine, OpTy, PlaceTy, | ||
| Projectable, Provenance, ReturnAction, ReturnContinuation, Scalar, StackPopInfo, interp_ok, | ||
| throw_ub, throw_ub_custom, throw_unsup_format, | ||
| throw_ub, throw_ub_custom, | ||
| }; | ||
| use crate::interpret::EnteredTraceSpan; | ||
| use crate::{enter_trace_span, fluent_generated as fluent}; | ||
|
|
@@ -349,13 +350,27 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { | |
| ) -> InterpResult<'tcx> { | ||
| let _trace = enter_trace_span!(M, step::init_stack_frame, %instance, tracing_separate_thread = Empty); | ||
|
|
||
| // 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())?; | ||
| // The check for the DefKind is so that we don't requiest the fn_sig of a closure. | ||
| // Otherwise, we hit: | ||
| // | ||
| // DefId(1:180 ~ std[269c]::rt::lang_start_internal::{closure#0}) does not have a "fn_sig" | ||
| let (fixed_count, callee_c_variadic_args) = | ||
| if matches!(self.tcx.def_kind(instance.def_id()), DefKind::Fn) | ||
| && let callee_fn_sig = self.tcx.fn_sig(instance.def_id()).skip_binder() | ||
| && callee_fn_sig.c_variadic() | ||
| { | ||
| // A mismatch in caller and callee fixed_count will error below. | ||
| let fixed_count = callee_fn_sig.inputs().skip_binder().len(); | ||
| let extra_tys = caller_fn_abi.args[caller_fn_abi.fixed_count as usize..] | ||
| .iter() | ||
| .map(|arg_abi| arg_abi.layout.ty); | ||
|
|
||
| (fixed_count, self.tcx.mk_type_list_from_iter(extra_tys)) | ||
| } else { | ||
| (caller_fn_abi.args.len(), 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"); | ||
| } | ||
| let callee_fn_abi = self.fn_abi_of_instance(instance, callee_c_variadic_args)?; | ||
|
|
||
| if caller_fn_abi.conv != callee_fn_abi.conv { | ||
| throw_ub_custom!( | ||
|
|
@@ -365,6 +380,20 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { | |
| ) | ||
| } | ||
|
|
||
| if caller_fn_abi.c_variadic != callee_fn_abi.c_variadic { | ||
| throw_ub!(CVariadicMismatch { | ||
| caller_is_c_variadic: caller_fn_abi.c_variadic, | ||
| callee_is_c_variadic: callee_fn_abi.c_variadic, | ||
| }); | ||
| } | ||
|
|
||
| if caller_fn_abi.fixed_count != callee_fn_abi.fixed_count { | ||
| throw_ub!(CVariadicFixedCountMismatch { | ||
| caller: caller_fn_abi.fixed_count, | ||
| callee: callee_fn_abi.fixed_count, | ||
| }); | ||
| } | ||
|
|
||
| // Check that all target features required by the callee (i.e., from | ||
| // the attribute `#[target_feature(enable = ...)]`) are enabled at | ||
| // compile time. | ||
|
|
@@ -436,16 +465,47 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { | |
| // this is a single iterator (that handles `spread_arg`), then | ||
| // `pass_argument` would be the loop body. It takes care to | ||
| // not advance `caller_iter` for ignored arguments. | ||
| let mut callee_args_abis = callee_fn_abi.args.iter().enumerate(); | ||
| for local in body.args_iter() { | ||
| let mut callee_args_abis = if caller_fn_abi.c_variadic { | ||
| // Only the fixed arguments are passed normally. | ||
| callee_fn_abi.args[..fixed_count].iter().enumerate() | ||
| } else { | ||
| // NOTE: this handles the extra caller location argument | ||
| // when `#[track_caller]` is used. | ||
| callee_fn_abi.args.iter().enumerate() | ||
| }; | ||
|
|
||
| let mut it = body.args_iter().peekable(); | ||
| while let Some(local) = it.next() { | ||
| // Construct the destination place for this argument. At this point all | ||
| // locals are still dead, so we cannot construct a `PlaceTy`. | ||
| let dest = mir::Place::from(local); | ||
| // `layout_of_local` does more than just the instantiation we need to get the | ||
| // type, but the result gets cached so this avoids calling the instantiation | ||
| // query *again* the next time this local is accessed. | ||
| let ty = self.layout_of_local(self.frame(), local, None)?.ty; | ||
| if Some(local) == body.spread_arg { | ||
| if caller_fn_abi.c_variadic && it.peek().is_none() { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this uses a peekable iterator just so that we can tell if the current argument is the last argument, right? I wonder if there's a more elegant way to do that...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that's the idea. idk, I've never found anything obviously better. Manually counting is an option but not great either. |
||
| // This is the last callee-side argument of a variadic function. | ||
| // This argument is a VaList holding the remaining caller-side arguments. | ||
| self.storage_live(local)?; | ||
|
|
||
| let place = self.eval_place(dest)?; | ||
| let mplace = self.force_allocation(&place)?; | ||
|
|
||
| // Consume the remaining arguments by putting them into the variable argument | ||
| // list. | ||
| let varargs = self.allocate_varargs(&mut caller_args)?; | ||
| let key = self.va_list_ptr(varargs); | ||
|
|
||
| // Zero the VaList, so it is fully initialized. | ||
| self.write_bytes_ptr( | ||
| mplace.ptr(), | ||
| (0..mplace.layout.size.bytes()).map(|_| 0u8), | ||
| )?; | ||
|
|
||
| // Store the "key" pointer in the right field. | ||
| let key_mplace = self.va_list_key_field(&mplace)?; | ||
| self.write_pointer(key, &key_mplace)?; | ||
| } else if Some(local) == body.spread_arg { | ||
| // Make the local live once, then fill in the value field by field. | ||
| self.storage_live(local)?; | ||
| // Must be a tuple | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have tests ensuring that c-variadics in const remain unstable?
The usual place that would be enforces is
compiler/rustc_const_eval/src/check_consts/mod.rs.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should get its own feature gate then? Because this PR also includes const c-variadic calls, and non-const c-variadic calls are already stable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A new feature gate sounds good. That should then cover both declaring variadic const fn and calling them from inside const contexts. (Those could also be different feature gates but that doesn't seem necessary.)