Skip to content

Commit 270eb58

Browse files
committed
Auto merge of #115452 - matthiaskrgr:rollup-he30fhv, r=matthiaskrgr
Rollup of 5 pull requests Successful merges: - #115411 (miri ABI check: fix handling of 1-ZST; don't accept sign differences) - #115424 (diagnostics: avoid wrong `unused_parens` on `x as (T) < y`) - #115425 (remove unnecessary heap allocation) - #115446 (fix version for abi_thiscall to 1.73.0, which was forgotten to change when stabilized and (later) moved to beta) - #115447 (Add comment so pub items are not removed) r? `@ghost` `@rustbot` modify labels: rollup
2 parents 361f8ba + ead5f80 commit 270eb58

File tree

11 files changed

+233
-49
lines changed

11 files changed

+233
-49
lines changed

compiler/rustc_codegen_llvm/src/back/archive.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ impl<'a> LlvmArchiveBuilder<'a> {
367367
match addition {
368368
Addition::File { path, name_in_archive } => {
369369
let path = CString::new(path.to_str().unwrap())?;
370-
let name = CString::new(name_in_archive.clone())?;
370+
let name = CString::new(name_in_archive.as_bytes())?;
371371
members.push(llvm::LLVMRustArchiveMemberNew(
372372
path.as_ptr(),
373373
name.as_ptr(),

compiler/rustc_codegen_llvm/src/back/lto.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ fn thin_lto(
441441

442442
for (i, (name, buffer)) in modules.into_iter().enumerate() {
443443
info!("local module: {} - {}", i, name);
444-
let cname = CString::new(name.clone()).unwrap();
444+
let cname = CString::new(name.as_bytes()).unwrap();
445445
thin_modules.push(llvm::ThinLTOModule {
446446
identifier: cname.as_ptr(),
447447
data: buffer.data().as_ptr(),

compiler/rustc_const_eval/src/interpret/terminator.rs

+23-24
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
254254
}
255255

256256
/// Find the wrapped inner type of a transparent wrapper.
257+
/// Must not be called on 1-ZST (as they don't have a uniquely defined "wrapped field").
257258
fn unfold_transparent(&self, layout: TyAndLayout<'tcx>) -> TyAndLayout<'tcx> {
258259
match layout.ty.kind() {
259260
ty::Adt(adt_def, _) if adt_def.repr().transparent() => {
@@ -263,11 +264,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
263264
let field = layout.field(self, idx);
264265
if field.is_1zst() { None } else { Some(field) }
265266
});
266-
let Some(first) = non_1zst_fields.next() else {
267-
// All fields are 1-ZST, so this is basically the same as `()`.
268-
// (We still also compare the `PassMode`, so if this target does something strange with 1-ZST there, we'll know.)
269-
return self.layout_of(self.tcx.types.unit).unwrap();
270-
};
267+
let first = non_1zst_fields.next().expect("`unfold_transparent` called on 1-ZST");
271268
assert!(
272269
non_1zst_fields.next().is_none(),
273270
"more than one non-1-ZST field in a transparent type"
@@ -289,17 +286,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
289286
caller_layout: TyAndLayout<'tcx>,
290287
callee_layout: TyAndLayout<'tcx>,
291288
) -> bool {
292-
fn primitive_abi_compat(a1: abi::Primitive, a2: abi::Primitive) -> bool {
293-
match (a1, a2) {
294-
// For integers, ignore the sign.
295-
(abi::Primitive::Int(int_ty1, _sign1), abi::Primitive::Int(int_ty2, _sign2)) => {
296-
int_ty1 == int_ty2
297-
}
298-
// For everything else we require full equality.
299-
_ => a1 == a2,
300-
}
301-
}
302-
303289
if caller_layout.ty == callee_layout.ty {
304290
// Fast path: equal types are definitely compatible.
305291
return true;
@@ -308,27 +294,40 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
308294
match (caller_layout.abi, callee_layout.abi) {
309295
// If both sides have Scalar/Vector/ScalarPair ABI, we can easily directly compare them.
310296
// Different valid ranges are okay (the validity check will complain if this leads to
311-
// invalid transmutes).
297+
// invalid transmutes). Different signs are *not* okay on some targets (e.g. `extern
298+
// "C"` on `s390x` where small integers are passed zero/sign-extended in large
299+
// registers), so we generally reject them to increase portability.
300+
// NOTE: this is *not* a stable guarantee! It just reflects a property of our current
301+
// ABIs. It's also fragile; the same pair of types might be considered ABI-compatible
302+
// when used directly by-value but not considered compatible as a struct field or array
303+
// element.
312304
(abi::Abi::Scalar(caller), abi::Abi::Scalar(callee)) => {
313-
primitive_abi_compat(caller.primitive(), callee.primitive())
305+
caller.primitive() == callee.primitive()
314306
}
315307
(
316308
abi::Abi::Vector { element: caller_element, count: caller_count },
317309
abi::Abi::Vector { element: callee_element, count: callee_count },
318310
) => {
319-
primitive_abi_compat(caller_element.primitive(), callee_element.primitive())
311+
caller_element.primitive() == callee_element.primitive()
320312
&& caller_count == callee_count
321313
}
322314
(abi::Abi::ScalarPair(caller1, caller2), abi::Abi::ScalarPair(callee1, callee2)) => {
323-
primitive_abi_compat(caller1.primitive(), callee1.primitive())
324-
&& primitive_abi_compat(caller2.primitive(), callee2.primitive())
315+
caller1.primitive() == callee1.primitive()
316+
&& caller2.primitive() == callee2.primitive()
325317
}
326318
(abi::Abi::Aggregate { .. }, abi::Abi::Aggregate { .. }) => {
327-
// Aggregates are compatible only if they newtype-wrap the same type.
319+
// Aggregates are compatible only if they newtype-wrap the same type, or if they are both 1-ZST.
320+
// (The latter part is needed to ensure e.g. that `struct Zst` is compatible with `struct Wrap((), Zst)`.)
328321
// This is conservative, but also means that our check isn't quite so heavily dependent on the `PassMode`,
329322
// which means having ABI-compatibility on one target is much more likely to imply compatibility for other targets.
330-
self.unfold_transparent(caller_layout).ty
331-
== self.unfold_transparent(callee_layout).ty
323+
if caller_layout.is_1zst() || callee_layout.is_1zst() {
324+
// If either is a 1-ZST, both must be.
325+
caller_layout.is_1zst() && callee_layout.is_1zst()
326+
} else {
327+
// Neither is a 1-ZST, so we can check what they are wrapping.
328+
self.unfold_transparent(caller_layout).ty
329+
== self.unfold_transparent(callee_layout).ty
330+
}
332331
}
333332
// What remains is `Abi::Uninhabited` (which can never be passed anyway) and
334333
// mismatching ABIs, that should all be rejected.

compiler/rustc_errors/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ use std::num::NonZeroUsize;
5555
use std::panic;
5656
use std::path::{Path, PathBuf};
5757

58+
// Used by external projects such as `rust-gpu`.
59+
// See https://github.com/rust-lang/rust/pull/115393.
5860
pub use termcolor::{Color, ColorSpec, WriteColor};
5961

6062
pub mod annotate_snippet_emitter_writer;

compiler/rustc_feature/src/accepted.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ declare_features! (
5454
/// instead of just the platforms on which it is the C ABI.
5555
(accepted, abi_sysv64, "1.24.0", Some(36167), None),
5656
/// Allows using the `thiscall` ABI.
57-
(accepted, abi_thiscall, "1.19.0", None, None),
57+
(accepted, abi_thiscall, "1.73.0", None, None),
5858
/// Allows using ADX intrinsics from `core::arch::{x86, x86_64}`.
5959
(accepted, adx_target_feature, "1.61.0", Some(44839), None),
6060
/// Allows explicit discriminants on non-unit enum variants.

compiler/rustc_lint/src/early.rs

+1
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
228228
}) => self.check_id(closure_id),
229229
_ => {}
230230
}
231+
lint_callback!(self, check_expr_post, e);
231232
}
232233

233234
fn visit_generic_arg(&mut self, arg: &'a ast::GenericArg) {

compiler/rustc_lint/src/passes.rs

+1
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ macro_rules! early_lint_methods {
153153
fn check_pat(a: &ast::Pat);
154154
fn check_pat_post(a: &ast::Pat);
155155
fn check_expr(a: &ast::Expr);
156+
fn check_expr_post(a: &ast::Expr);
156157
fn check_ty(a: &ast::Ty);
157158
fn check_generic_arg(a: &ast::GenericArg);
158159
fn check_generic_param(a: &ast::GenericParam);

compiler/rustc_lint/src/unused.rs

+28-1
Original file line numberDiff line numberDiff line change
@@ -955,11 +955,14 @@ declare_lint! {
955955

956956
pub struct UnusedParens {
957957
with_self_ty_parens: bool,
958+
/// `1 as (i32) < 2` parses to ExprKind::Lt
959+
/// `1 as i32 < 2` parses to i32::<2[missing angle bracket]
960+
parens_in_cast_in_lt: Vec<ast::NodeId>,
958961
}
959962

960963
impl UnusedParens {
961964
pub fn new() -> Self {
962-
Self { with_self_ty_parens: false }
965+
Self { with_self_ty_parens: false, parens_in_cast_in_lt: Vec::new() }
963966
}
964967
}
965968

@@ -1055,6 +1058,14 @@ impl UnusedParens {
10551058
impl EarlyLintPass for UnusedParens {
10561059
#[inline]
10571060
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
1061+
if let ExprKind::Binary(op, lhs, _rhs) = &e.kind &&
1062+
(op.node == ast::BinOpKind::Lt || op.node == ast::BinOpKind::Shl) &&
1063+
let ExprKind::Cast(_expr, ty) = &lhs.kind &&
1064+
let ast::TyKind::Paren(_) = &ty.kind
1065+
{
1066+
self.parens_in_cast_in_lt.push(ty.id);
1067+
}
1068+
10581069
match e.kind {
10591070
ExprKind::Let(ref pat, _, _) | ExprKind::ForLoop(ref pat, ..) => {
10601071
self.check_unused_parens_pat(cx, pat, false, false, (true, true));
@@ -1101,6 +1112,17 @@ impl EarlyLintPass for UnusedParens {
11011112
<Self as UnusedDelimLint>::check_expr(self, cx, e)
11021113
}
11031114

1115+
fn check_expr_post(&mut self, _cx: &EarlyContext<'_>, e: &ast::Expr) {
1116+
if let ExprKind::Binary(op, lhs, _rhs) = &e.kind &&
1117+
(op.node == ast::BinOpKind::Lt || op.node == ast::BinOpKind::Shl) &&
1118+
let ExprKind::Cast(_expr, ty) = &lhs.kind &&
1119+
let ast::TyKind::Paren(_) = &ty.kind
1120+
{
1121+
let id = self.parens_in_cast_in_lt.pop().expect("check_expr and check_expr_post must balance");
1122+
assert_eq!(id, ty.id, "check_expr, check_ty, and check_expr_post are called, in that order, by the visitor");
1123+
}
1124+
}
1125+
11041126
fn check_pat(&mut self, cx: &EarlyContext<'_>, p: &ast::Pat) {
11051127
use ast::{Mutability, PatKind::*};
11061128
let keep_space = (false, false);
@@ -1141,6 +1163,11 @@ impl EarlyLintPass for UnusedParens {
11411163
}
11421164

11431165
fn check_ty(&mut self, cx: &EarlyContext<'_>, ty: &ast::Ty) {
1166+
if let ast::TyKind::Paren(_) = ty.kind &&
1167+
Some(&ty.id) == self.parens_in_cast_in_lt.last()
1168+
{
1169+
return;
1170+
}
11441171
match &ty.kind {
11451172
ast::TyKind::Array(_, len) => {
11461173
self.check_unused_delims_expr(

src/tools/miri/tests/pass/function_calls/abi_compat.rs

+16-21
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
1-
#![feature(portable_simd)]
21
use std::mem;
32
use std::num;
4-
use std::simd;
53

6-
#[derive(Copy, Clone)]
4+
#[derive(Copy, Clone, Default)]
75
struct Zst;
86

97
fn test_abi_compat<T: Copy, U: Copy>(t: T, u: U) {
@@ -33,7 +31,7 @@ fn test_abi_compat<T: Copy, U: Copy>(t: T, u: U) {
3331
}
3432

3533
/// Ensure that `T` is compatible with various repr(transparent) wrappers around `T`.
36-
fn test_abi_newtype<T: Copy>(t: T) {
34+
fn test_abi_newtype<T: Copy + Default>() {
3735
#[repr(transparent)]
3836
#[derive(Copy, Clone)]
3937
struct Wrapper1<T>(T);
@@ -47,6 +45,7 @@ fn test_abi_newtype<T: Copy>(t: T) {
4745
#[derive(Copy, Clone)]
4846
struct Wrapper3<T>(Zst, T, [u8; 0]);
4947

48+
let t = T::default();
5049
test_abi_compat(t, Wrapper1(t));
5150
test_abi_compat(t, Wrapper2(t, ()));
5251
test_abi_compat(t, Wrapper2a((), t));
@@ -56,36 +55,32 @@ fn test_abi_newtype<T: Copy>(t: T) {
5655

5756
fn main() {
5857
// Here we check:
59-
// - unsigned vs signed integer is allowed
60-
// - u32/i32 vs char is allowed
58+
// - u32 vs char is allowed
6159
// - u32 vs NonZeroU32/Option<NonZeroU32> is allowed
6260
// - reference vs raw pointer is allowed
6361
// - references to things of the same size and alignment are allowed
6462
// These are very basic tests that should work on all ABIs. However it is not clear that any of
6563
// these would be stably guaranteed. Code that relies on this is equivalent to code that relies
6664
// on the layout of `repr(Rust)` types. They are also fragile: the same mismatches in the fields
6765
// of a struct (even with `repr(C)`) will not always be accepted by Miri.
68-
test_abi_compat(0u32, 0i32);
69-
test_abi_compat(simd::u32x8::splat(1), simd::i32x8::splat(1));
66+
// Note that `bool` and `u8` are *not* compatible, at least on x86-64!
67+
// One of them has `arg_ext: Zext`, the other does not.
68+
// Similarly, `i32` and `u32` are not compatible on s390x due to different `arg_ext`.
7069
test_abi_compat(0u32, 'x');
71-
test_abi_compat(0i32, 'x');
7270
test_abi_compat(42u32, num::NonZeroU32::new(1).unwrap());
7371
test_abi_compat(0u32, Some(num::NonZeroU32::new(1).unwrap()));
7472
test_abi_compat(&0u32, &0u32 as *const u32);
7573
test_abi_compat(&0u32, &([true; 4], [0u32; 0]));
76-
// Note that `bool` and `u8` are *not* compatible, at least on x86-64!
77-
// One of them has `arg_ext: Zext`, the other does not.
7874

7975
// These must work for *any* type, since we guarantee that `repr(transparent)` is ABI-compatible
8076
// with the wrapped field.
81-
test_abi_newtype(());
82-
// FIXME: this still fails! test_abi_newtype(Zst);
83-
test_abi_newtype(0u32);
84-
test_abi_newtype(0f32);
85-
test_abi_newtype((0u32, 1u32, 2u32));
86-
// FIXME: skipping the array tests on mips64 due to https://github.com/rust-lang/rust/issues/115404
87-
if !cfg!(target_arch = "mips64") {
88-
test_abi_newtype([0u32, 1u32, 2u32]);
89-
test_abi_newtype([0i32; 0]);
90-
}
77+
test_abi_newtype::<()>();
78+
test_abi_newtype::<Zst>();
79+
test_abi_newtype::<u32>();
80+
test_abi_newtype::<f32>();
81+
test_abi_newtype::<(u8, u16, f32)>();
82+
test_abi_newtype::<[u8; 0]>();
83+
test_abi_newtype::<[u32; 0]>();
84+
test_abi_newtype::<[u32; 2]>();
85+
test_abi_newtype::<[u32; 32]>();
9186
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// check-pass
2+
#![warn(unused_parens)]
3+
4+
fn id<T>(t: T) -> T { t }
5+
6+
fn main() {
7+
// This should not warn
8+
let _ = 1 as (i32) < 2;
9+
let _ = id(1 as (i32) < 2);
10+
let _ = id(1 as i32)
11+
as (i32) < 2;
12+
// These should warn
13+
let _ = 1 as ((i32)) < 2; //~WARN unnecessary parentheses
14+
let _ = 1 as (i32); //~WARN unnecessary parentheses
15+
let _ = 1 as (i32) > 2; //~WARN unnecessary parentheses
16+
let _ = id(1 as (i32)) //~WARN unnecessary parentheses
17+
as (i32) < 2;
18+
let _ = id(1 as (i32)) < 2; //~WARN unnecessary parentheses
19+
20+
// This should not warn
21+
let _ = 1 as (i32) << 2;
22+
let _ = id(1 as (i32) << 2);
23+
let _ = id(1 as i32)
24+
as (i32) << 2;
25+
// These should warn
26+
let _ = 1 as ((i32)) << 2; //~WARN unnecessary parentheses
27+
let _ = 1 as (i32); //~WARN unnecessary parentheses
28+
let _ = 1 as (i32) >> 2; //~WARN unnecessary parentheses
29+
let _ = id(1 as (i32)) //~WARN unnecessary parentheses
30+
as (i32) << 2;
31+
let _ = id(1 as (i32)) << 2; //~WARN unnecessary parentheses
32+
}

0 commit comments

Comments
 (0)