drop glue takes in mutable references, it should reflect that in its type #56165
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -182,20 +182,28 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> | |
| } | ||
|
|
||
| fn check_argument_compat( | ||
| rust_abi: bool, | ||
| caller: TyLayout<'tcx>, | ||
| callee: TyLayout<'tcx>, | ||
| ) -> bool { | ||
| if caller.ty == callee.ty { | ||
| // No question | ||
| return true; | ||
| } | ||
| if !rust_abi { | ||
| // Don't risk anything | ||
| return false; | ||
| } | ||
| // Compare layout | ||
| match (&caller.abi, &callee.abi) { | ||
| // Different valid ranges are okay (once we enforce validity, | ||
| // that will take care to make it UB to leave the range, just | ||
| // like for transmute). | ||
| (layout::Abi::Scalar(ref caller), layout::Abi::Scalar(ref callee)) => | ||
| caller.value == callee.value, | ||
| (layout::Abi::ScalarPair(ref caller1, ref caller2), | ||
| layout::Abi::ScalarPair(ref callee1, ref callee2)) => | ||
| caller1.value == callee1.value && caller2.value == callee2.value, | ||
|
There was a problem hiding this comment. Are you not using There was a problem hiding this comment. Which rules for aggregates can there be when the ABIs are the same except for the ranges...? I thought these ABIs encode everything.^^
Nope, you didn't yet convince me that it would make anything simpler.^^ There was a problem hiding this comment.
It would make things correct, which seems more important than simplicity. It's not like that hard to use, just have to move some code from (assuming you don't want to emulate the runtime behavior of mismatched ABIs :P) There was a problem hiding this comment.
Ouch. Okay, I will restrict type mismatches to Rust Abi functions.
You assume correctly. :P
Would you mentor someone? I could open an E-mentor issue. There was a problem hiding this comment.
Sure, always happy to mentor compiler cleanups! The main issue to moving that stuff around is rust/src/librustc_codegen_llvm/abi.rs Line 487 in dae6c93 Everything else is mostly relying on There was a problem hiding this comment.
Done. |
||
| // Be conservative | ||
| _ => false | ||
| } | ||
|
|
@@ -204,22 +212,22 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> | |
| /// Pass a single argument, checking the types for compatibility. | ||
| fn pass_argument( | ||
| &mut self, | ||
| rust_abi: bool, | ||
| caller_arg: &mut impl Iterator<Item=OpTy<'tcx, M::PointerTag>>, | ||
| callee_arg: PlaceTy<'tcx, M::PointerTag>, | ||
| ) -> EvalResult<'tcx> { | ||
| if rust_abi && callee_arg.layout.is_zst() { | ||
| // Nothing to do. | ||
| trace!("Skipping callee ZST"); | ||
| return Ok(()); | ||
| } | ||
| let caller_arg = caller_arg.next() | ||
| .ok_or_else(|| EvalErrorKind::FunctionArgCountMismatch)?; | ||
| if rust_abi { | ||
| debug_assert!(!caller_arg.layout.is_zst(), "ZSTs must have been already filtered out"); | ||
| } | ||
| // Now, check | ||
| if !Self::check_argument_compat(rust_abi, caller_arg.layout, callee_arg.layout) { | ||
| return err!(FunctionArgMismatch(caller_arg.layout.ty, callee_arg.layout.ty)); | ||
| } | ||
| // We allow some transmutes here | ||
|
|
@@ -319,7 +327,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> | |
| // Figure out how to pass which arguments. | ||
| // We have two iterators: Where the arguments come from, | ||
| // and where they go to. | ||
| let rust_abi = match caller_abi { | ||
| Abi::Rust | Abi::RustCall => true, | ||
| _ => false | ||
| }; | ||
|
|
@@ -344,7 +352,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> | |
| }; | ||
| // Skip ZSTs | ||
| let mut caller_iter = caller_args.iter() | ||
| .filter(|op| !rust_abi || !op.layout.is_zst()) | ||
| .map(|op| *op); | ||
|
|
||
| // Now we have to spread them out across the callee's locals, | ||
|
|
@@ -359,11 +367,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> | |
| // Must be a tuple | ||
| for i in 0..dest.layout.fields.count() { | ||
| let dest = self.place_field(dest, i as u64)?; | ||
| self.pass_argument(rust_abi, &mut caller_iter, dest)?; | ||
| } | ||
| } else { | ||
| // Normal argument | ||
| self.pass_argument(rust_abi, &mut caller_iter, dest)?; | ||
| } | ||
| } | ||
| // Now we should have no more caller args | ||
|
|
@@ -374,7 +382,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> | |
| // Don't forget to check the return type! | ||
| if let Some(caller_ret) = dest { | ||
| let callee_ret = self.eval_place(&mir::Place::Local(mir::RETURN_PLACE))?; | ||
| if !Self::check_argument_compat( | ||
| rust_abi, | ||
| caller_ret.layout, | ||
| callee_ret.layout, | ||
| ) { | ||
| return err!(FunctionRetMismatch( | ||
| caller_ret.layout.ty, callee_ret.layout.ty | ||
| )); | ||
|
|
||
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 changes the behavior in the case of
(Scalar, Scalar)pairs, right? (They didn't use to be dependent on therust_abi) -- @eddyb is that what you intended?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.
Yes, scalar pairs are only used in the Rust ABI, two types with the same scalar pair optimization can have incompatible by-value ABI with
extern "C".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.
Yes. It also changes behavior on non-Rust ABIs for
Scalar, where we no longer allow any kind of mismatch -- mostly for consistency.