From 205cfcba200b13af31c39e67127cb3c07d491702 Mon Sep 17 00:00:00 2001 From: klensy Date: Wed, 21 Feb 2024 12:18:59 +0300 Subject: [PATCH 1/7] llvm-wrapper: fix warning C4244 llvm-wrapper/RustWrapper.cpp(1234): warning C4244: '=': conversion from 'uint64_t' to 'unsigned int', possible loss of data nice consistency: uint64_t https://github.com/llvm/llvm-project/blob/6009708b4367171ccdbf4b5905cb6a803753fe18/llvm/include/llvm/IR/DiagnosticInfo.h#L172 but unsigned https://github.com/llvm/llvm-project/blob/6009708b4367171ccdbf4b5905cb6a803753fe18/llvm/include/llvm/IR/DiagnosticInfo.h#L1091 --- compiler/rustc_codegen_llvm/src/back/write.rs | 6 +++--- compiler/rustc_codegen_llvm/src/llvm/diagnostic.rs | 4 ++-- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 2 +- compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs index 27cb0366f17dc..08aab84986800 100644 --- a/compiler/rustc_codegen_llvm/src/back/write.rs +++ b/compiler/rustc_codegen_llvm/src/back/write.rs @@ -36,7 +36,7 @@ use rustc_span::InnerSpan; use rustc_target::spec::{CodeModel, RelocModel, SanitizerSet, SplitDebuginfo, TlsModel}; use crate::llvm::diagnostic::OptimizationDiagnosticKind; -use libc::{c_char, c_int, c_uint, c_void, size_t}; +use libc::{c_char, c_int, c_void, size_t}; use std::ffi::CString; use std::fs; use std::io::{self, Write}; @@ -406,7 +406,7 @@ fn report_inline_asm( cgcx: &CodegenContext, msg: String, level: llvm::DiagnosticLevel, - mut cookie: c_uint, + mut cookie: u64, source: Option<(String, Vec)>, ) { // In LTO build we may get srcloc values from other crates which are invalid @@ -420,7 +420,7 @@ fn report_inline_asm( llvm::DiagnosticLevel::Warning => Level::Warning, llvm::DiagnosticLevel::Note | llvm::DiagnosticLevel::Remark => Level::Note, }; - cgcx.diag_emitter.inline_asm_error(cookie as u32, msg, level, source); + cgcx.diag_emitter.inline_asm_error(cookie.try_into().unwrap(), msg, level, source); } unsafe extern "C" fn diagnostic_handler(info: &DiagnosticInfo, user: *mut c_void) { diff --git a/compiler/rustc_codegen_llvm/src/llvm/diagnostic.rs b/compiler/rustc_codegen_llvm/src/llvm/diagnostic.rs index 06e846a2b45eb..f9b28178ddb97 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/diagnostic.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/diagnostic.rs @@ -123,7 +123,7 @@ impl SrcMgrDiagnostic { #[derive(Clone)] pub struct InlineAsmDiagnostic { pub level: super::DiagnosticLevel, - pub cookie: c_uint, + pub cookie: u64, pub message: String, pub source: Option<(String, Vec)>, } @@ -149,7 +149,7 @@ impl InlineAsmDiagnostic { let smdiag = SrcMgrDiagnostic::unpack(super::LLVMRustGetSMDiagnostic(di, &mut cookie)); InlineAsmDiagnostic { level: smdiag.level, - cookie, + cookie: cookie.into(), message: smdiag.message, source: smdiag.source, } diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index d0044086c616e..90116b90478a9 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -2254,7 +2254,7 @@ extern "C" { pub fn LLVMRustUnpackInlineAsmDiagnostic<'a>( DI: &'a DiagnosticInfo, level_out: &mut DiagnosticLevel, - cookie_out: &mut c_uint, + cookie_out: &mut u64, message_out: &mut Option<&'a Twine>, ); diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index b45706fd1e5b2..e645f25c8d4e2 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1225,7 +1225,7 @@ enum class LLVMRustDiagnosticLevel { extern "C" void LLVMRustUnpackInlineAsmDiagnostic(LLVMDiagnosticInfoRef DI, LLVMRustDiagnosticLevel *LevelOut, - unsigned *CookieOut, + uint64_t *CookieOut, LLVMTwineRef *MessageOut) { // Undefined to call this not on an inline assembly diagnostic! llvm::DiagnosticInfoInlineAsm *IA = From 0ce966fd6415773698fd9f85a86ef49dd4225ecf Mon Sep 17 00:00:00 2001 From: klensy Date: Wed, 21 Feb 2024 13:13:50 +0300 Subject: [PATCH 2/7] llvm-wrapper: fix warning C4305 llvm-wrapper/ArchiveWrapper.cpp(70): warning C4305: 'argument': truncation from 'int' to 'bool' while in llvm 12 signature was static ErrorOr> getFile(const Twine &Filename, int64_t FileSize = -1, bool RequiresNullTerminator = true, bool IsVolatile = false); https://github.com/llvm/llvm-project/blame/fed41342a82f5a3a9201819a82bf7a48313e296b/llvm/include/llvm/Support/MemoryBuffer.h#L85-L87 in llvm 13 and later it was changed to static ErrorOr> getFile(const Twine &Filename, bool IsText = false, bool RequiresNullTerminator = true, bool IsVolatile = false); https://github.com/llvm/llvm-project/blame/75e33f71c2dae584b13a7d1186ae0a038ba98838/llvm/include/llvm/Support/MemoryBuffer.h#L86-L88 so code was interpreted as MemoryBuffer::getFile(Path, /*IsText*/true, /*RequiresNullTerminator=*/false), but now will be MemoryBuffer::getFile(Path, /*IsText*/false, /*RequiresNullTerminator=*/false). How that worked before? --- compiler/rustc_llvm/llvm-wrapper/ArchiveWrapper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_llvm/llvm-wrapper/ArchiveWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/ArchiveWrapper.cpp index 54fdc84c77d26..40723ff9f5ef2 100644 --- a/compiler/rustc_llvm/llvm-wrapper/ArchiveWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/ArchiveWrapper.cpp @@ -67,7 +67,7 @@ typedef RustArchiveIterator *LLVMRustArchiveIteratorRef; extern "C" LLVMRustArchiveRef LLVMRustOpenArchive(char *Path) { ErrorOr> BufOr = - MemoryBuffer::getFile(Path, -1, false); + MemoryBuffer::getFile(Path, /*IsText*/false, /*RequiresNullTerminator=*/false); if (!BufOr) { LLVMRustSetLastError(BufOr.getError().message().c_str()); return nullptr; From ccf789b674b696eaf99f28ab605d1a8e3279d1d0 Mon Sep 17 00:00:00 2001 From: surechen Date: Fri, 23 Feb 2024 17:23:01 +0800 Subject: [PATCH 3/7] By changing some attributes to only_local, reducing encoding attributes in the crate metadate. Thank you. This is part of changing attributes to only_local. I hope get your opinion whether I should split into multiple PRs, or submit in one. According to [try to not rely on attributes from extern crates](https://github.com/rust-lang/compiler-team/issues/505) and lcnr's guidance. --- compiler/rustc_feature/src/builtin_attrs.rs | 62 +++++++++++++++------ 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_feature/src/builtin_attrs.rs b/compiler/rustc_feature/src/builtin_attrs.rs index 99875ec540545..55fe80c7aefd3 100644 --- a/compiler/rustc_feature/src/builtin_attrs.rs +++ b/compiler/rustc_feature/src/builtin_attrs.rs @@ -278,25 +278,35 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ ungated!(cfg_attr, Normal, template!(List: "predicate, attr1, attr2, ..."), DuplicatesOk), // Testing: - ungated!(ignore, Normal, template!(Word, NameValueStr: "reason"), WarnFollowing), + ungated!( + ignore, Normal, template!(Word, NameValueStr: "reason"), WarnFollowing, + @only_local: true, + ), ungated!( should_panic, Normal, template!(Word, List: r#"expected = "reason""#, NameValueStr: "reason"), FutureWarnFollowing, + @only_local: true, ), // FIXME(Centril): This can be used on stable but shouldn't. - ungated!(reexport_test_harness_main, CrateLevel, template!(NameValueStr: "name"), ErrorFollowing), + ungated!( + reexport_test_harness_main, CrateLevel, template!(NameValueStr: "name"), ErrorFollowing, + @only_local: true, + ), // Macros: ungated!(automatically_derived, Normal, template!(Word), WarnFollowing), - ungated!(macro_use, Normal, template!(Word, List: "name1, name2, ..."), WarnFollowingWordOnly), - ungated!(macro_escape, Normal, template!(Word), WarnFollowing), // Deprecated synonym for `macro_use`. + ungated!( + macro_use, Normal, template!(Word, List: "name1, name2, ..."), WarnFollowingWordOnly, + @only_local: true, + ), + ungated!(macro_escape, Normal, template!(Word), WarnFollowing, @only_local: true), // Deprecated synonym for `macro_use`. ungated!(macro_export, Normal, template!(Word, List: "local_inner_macros"), WarnFollowing), - ungated!(proc_macro, Normal, template!(Word), ErrorFollowing), + ungated!(proc_macro, Normal, template!(Word), ErrorFollowing, @only_local: true), ungated!( - proc_macro_derive, Normal, - template!(List: "TraitName, /*opt*/ attributes(name1, name2, ...)"), ErrorFollowing, + proc_macro_derive, Normal, template!(List: "TraitName, /*opt*/ attributes(name1, name2, ...)"), + ErrorFollowing, @only_local: true, ), - ungated!(proc_macro_attribute, Normal, template!(Word), ErrorFollowing), + ungated!(proc_macro_attribute, Normal, template!(Word), ErrorFollowing, @only_local: true), // Lints: ungated!( @@ -309,7 +319,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ ), gated!( expect, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#), DuplicatesOk, - lint_reasons, experimental!(expect) + @only_local: true, lint_reasons, experimental!(expect) ), ungated!( forbid, Normal, template!(List: r#"lint1, lint2, ..., /*opt*/ reason = "...""#), @@ -335,32 +345,48 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ ), // Crate properties: - ungated!(crate_name, CrateLevel, template!(NameValueStr: "name"), FutureWarnFollowing), - ungated!(crate_type, CrateLevel, template!(NameValueStr: "bin|lib|..."), DuplicatesOk), + ungated!( + crate_name, CrateLevel, template!(NameValueStr: "name"), FutureWarnFollowing, + @only_local: true, + ), + ungated!( + crate_type, CrateLevel, template!(NameValueStr: "bin|lib|..."), DuplicatesOk, + @only_local: true, + ), // crate_id is deprecated - ungated!(crate_id, CrateLevel, template!(NameValueStr: "ignored"), FutureWarnFollowing), + ungated!( + crate_id, CrateLevel, template!(NameValueStr: "ignored"), FutureWarnFollowing, + @only_local: true, + ), // ABI, linking, symbols, and FFI ungated!( link, Normal, template!(List: r#"name = "...", /*opt*/ kind = "dylib|static|...", /*opt*/ wasm_import_module = "...", /*opt*/ import_name_type = "decorated|noprefix|undecorated""#), DuplicatesOk, + @only_local: true, ), ungated!(link_name, Normal, template!(NameValueStr: "name"), FutureWarnPreceding), - ungated!(no_link, Normal, template!(Word), WarnFollowing), + ungated!(no_link, Normal, template!(Word), WarnFollowing, @only_local: true), ungated!(repr, Normal, template!(List: "C"), DuplicatesOk, @only_local: true), - ungated!(export_name, Normal, template!(NameValueStr: "name"), FutureWarnPreceding), - ungated!(link_section, Normal, template!(NameValueStr: "name"), FutureWarnPreceding), + ungated!(export_name, Normal, template!(NameValueStr: "name"), FutureWarnPreceding, @only_local: true), + ungated!(link_section, Normal, template!(NameValueStr: "name"), FutureWarnPreceding, @only_local: true), ungated!(no_mangle, Normal, template!(Word), WarnFollowing, @only_local: true), ungated!(used, Normal, template!(Word, List: "compiler|linker"), WarnFollowing, @only_local: true), ungated!(link_ordinal, Normal, template!(List: "ordinal"), ErrorPreceding), // Limits: - ungated!(recursion_limit, CrateLevel, template!(NameValueStr: "N"), FutureWarnFollowing), - ungated!(type_length_limit, CrateLevel, template!(NameValueStr: "N"), FutureWarnFollowing), + ungated!( + recursion_limit, CrateLevel, template!(NameValueStr: "N"), FutureWarnFollowing, + @only_local: true + ), + ungated!( + type_length_limit, CrateLevel, template!(NameValueStr: "N"), FutureWarnFollowing, + @only_local: true + ), gated!( move_size_limit, CrateLevel, template!(NameValueStr: "N"), ErrorFollowing, - large_assignments, experimental!(move_size_limit) + @only_local: true, large_assignments, experimental!(move_size_limit) ), // Entry point: From bf7d1b54e8a3195a0f52516d48659d9f1270436d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 26 Feb 2024 09:24:03 +1100 Subject: [PATCH 4/7] Move `emit_stashed_diagnostic` call in rustfmt. This call was added to `parse_crate_mod` in #121487, to fix a case where a stashed diagnostic wasn't emitted. But there is another path where a stashed diagnostic might fail to be emitted if there's a parse error, if the `build` call in `parse_crate_inner` fails before `parse_crate_mod` is reached. So this commit moves the `emit_stashed_diagnostic` call outwards, from `parse_crate_mod` to `format_project`, just after the `Parser::parse_crate` call. This should be far out enough to catch any parsing errors. Fixes #121517. --- src/tools/rustfmt/src/formatting.rs | 22 ++++++++++++++----- src/tools/rustfmt/src/parse/parser.rs | 16 ++++---------- src/tools/rustfmt/src/parse/session.rs | 6 ++++- src/tools/rustfmt/src/test/parser.rs | 7 ++++++ .../rustfmt/tests/parser/stashed-diag2.rs | 3 +++ 5 files changed, 35 insertions(+), 19 deletions(-) create mode 100644 src/tools/rustfmt/tests/parser/stashed-diag2.rs diff --git a/src/tools/rustfmt/src/formatting.rs b/src/tools/rustfmt/src/formatting.rs index cd57a025b6726..323ae83fe6ede 100644 --- a/src/tools/rustfmt/src/formatting.rs +++ b/src/tools/rustfmt/src/formatting.rs @@ -109,7 +109,7 @@ fn format_project( let main_file = input.file_name(); let input_is_stdin = main_file == FileName::Stdin; - let parse_session = ParseSess::new(config)?; + let mut parse_session = ParseSess::new(config)?; if config.skip_children() && parse_session.ignore_file(&main_file) { return Ok(FormatReport::new()); } @@ -117,11 +117,21 @@ fn format_project( // Parse the crate. let mut report = FormatReport::new(); let directory_ownership = input.to_directory_ownership(); - let krate = match Parser::parse_crate(input, &parse_session) { - Ok(krate) => krate, - // Surface parse error via Session (errors are merged there from report) - Err(e) => { - let forbid_verbose = input_is_stdin || e != ParserError::ParsePanicError; + + // rustfmt doesn't use `run_compiler` like other tools, so it must emit any + // stashed diagnostics itself, otherwise the `DiagCtxt` will assert when + // dropped. The final result here combines the parsing result and the + // `emit_stashed_diagnostics` result. + let parse_res = Parser::parse_crate(input, &parse_session); + let stashed_res = parse_session.emit_stashed_diagnostics(); + let krate = match (parse_res, stashed_res) { + (Ok(krate), None) => krate, + (parse_res, _) => { + // Surface parse error via Session (errors are merged there from report). + let forbid_verbose = match parse_res { + Err(e) if e != ParserError::ParsePanicError => true, + _ => input_is_stdin, + }; should_emit_verbose(forbid_verbose, config, || { eprintln!("The Rust parser panicked"); }); diff --git a/src/tools/rustfmt/src/parse/parser.rs b/src/tools/rustfmt/src/parse/parser.rs index cca14353b5c18..3269fe7c7c706 100644 --- a/src/tools/rustfmt/src/parse/parser.rs +++ b/src/tools/rustfmt/src/parse/parser.rs @@ -162,22 +162,14 @@ impl<'a> Parser<'a> { fn parse_crate_mod(&mut self) -> Result { let mut parser = AssertUnwindSafe(&mut self.parser); - - // rustfmt doesn't use `run_compiler` like other tools, so it must emit - // any stashed diagnostics itself, otherwise the `DiagCtxt` will assert - // when dropped. The final result here combines the parsing result and - // the `emit_stashed_diagnostics` result. - let parse_res = catch_unwind(move || parser.parse_crate_mod()); - let stashed_res = self.parser.dcx().emit_stashed_diagnostics(); let err = Err(ParserError::ParsePanicError); - match (parse_res, stashed_res) { - (Ok(Ok(k)), None) => Ok(k), - (Ok(Ok(_)), Some(_guar)) => err, - (Ok(Err(db)), _) => { + match catch_unwind(move || parser.parse_crate_mod()) { + Ok(Ok(k)) => Ok(k), + Ok(Err(db)) => { db.emit(); err } - (Err(_), _) => err, + Err(_) => err, } } } diff --git a/src/tools/rustfmt/src/parse/session.rs b/src/tools/rustfmt/src/parse/session.rs index cff025cf2ab3a..decd3b167e0a5 100644 --- a/src/tools/rustfmt/src/parse/session.rs +++ b/src/tools/rustfmt/src/parse/session.rs @@ -6,7 +6,7 @@ use rustc_data_structures::sync::{IntoDynSyncSend, Lrc}; use rustc_errors::emitter::{DynEmitter, Emitter, HumanEmitter}; use rustc_errors::translation::Translate; use rustc_errors::{ - ColorConfig, DiagCtxt, Diagnostic, DiagnosticBuilder, Level as DiagnosticLevel, + ColorConfig, DiagCtxt, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, Level as DiagnosticLevel, }; use rustc_session::parse::ParseSess as RawParseSess; use rustc_span::{ @@ -230,6 +230,10 @@ impl ParseSess { self.ignore_path_set.as_ref().is_match(path) } + pub(crate) fn emit_stashed_diagnostics(&mut self) -> Option { + self.parse_sess.dcx.emit_stashed_diagnostics() + } + pub(crate) fn set_silent_emitter(&mut self) { self.parse_sess.dcx = DiagCtxt::with_emitter(silent_emitter()); } diff --git a/src/tools/rustfmt/src/test/parser.rs b/src/tools/rustfmt/src/test/parser.rs index da2a2ba62e009..d951c8469e661 100644 --- a/src/tools/rustfmt/src/test/parser.rs +++ b/src/tools/rustfmt/src/test/parser.rs @@ -62,3 +62,10 @@ fn crate_parsing_stashed_diag() { let filename = "tests/parser/stashed-diag.rs"; assert_parser_error(filename); } + +#[test] +fn crate_parsing_stashed_diag2() { + // See also https://github.com/rust-lang/rust/issues/121517 + let filename = "tests/parser/stashed-diag2.rs"; + assert_parser_error(filename); +} diff --git a/src/tools/rustfmt/tests/parser/stashed-diag2.rs b/src/tools/rustfmt/tests/parser/stashed-diag2.rs new file mode 100644 index 0000000000000..579a69def1633 --- /dev/null +++ b/src/tools/rustfmt/tests/parser/stashed-diag2.rs @@ -0,0 +1,3 @@ +trait Trait<'1> { s> {} + +fn main() {} From ff07f55db500dafb2d6570b5de7928bcc10a955f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 26 Feb 2024 01:36:08 +0000 Subject: [PATCH 5/7] Actually use the right closure kind when checking async Fn goals --- .../src/traits/select/confirmation.rs | 5 ++-- .../async-closures/wrong-fn-kind.rs | 8 ++++-- .../async-closures/wrong-fn-kind.stderr | 27 ++++++++++++++++--- 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs index f76be87694886..d316149731e14 100644 --- a/compiler/rustc_trait_selection/src/traits/select/confirmation.rs +++ b/compiler/rustc_trait_selection/src/traits/select/confirmation.rs @@ -934,7 +934,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { (trait_ref, Ty::from_closure_kind(tcx, ty::ClosureKind::Fn)) } ty::Closure(_, args) => { - let sig = args.as_closure().sig(); + let args = args.as_closure(); + let sig = args.sig(); let trait_ref = sig.map_bound(|sig| { ty::TraitRef::new( self.tcx(), @@ -950,7 +951,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { ty::TraitRef::new(tcx, future_trait_def_id, [sig.output()]) }), )); - (trait_ref, Ty::from_closure_kind(tcx, ty::ClosureKind::Fn)) + (trait_ref, args.kind_ty()) } _ => bug!("expected callable type for AsyncFn candidate"), }; diff --git a/tests/ui/async-await/async-closures/wrong-fn-kind.rs b/tests/ui/async-await/async-closures/wrong-fn-kind.rs index 7c3c7337e2ef0..8502bb6e2d45d 100644 --- a/tests/ui/async-await/async-closures/wrong-fn-kind.rs +++ b/tests/ui/async-await/async-closures/wrong-fn-kind.rs @@ -1,7 +1,5 @@ //@ edition:2021 -// FIXME(async_closures): This needs a better error message! - #![feature(async_closure)] fn main() { @@ -12,4 +10,10 @@ fn main() { //~^ ERROR expected a closure that implements the `async Fn` trait, but this closure only implements `async FnMut` x += 1; }); + + let x = String::new(); + needs_async_fn(move || async move { + //~^ ERROR expected a closure that implements the `async Fn` trait, but this closure only implements `async FnOnce` + println!("{x}"); + }); } diff --git a/tests/ui/async-await/async-closures/wrong-fn-kind.stderr b/tests/ui/async-await/async-closures/wrong-fn-kind.stderr index 34a6b3a485a60..d0f1948e48f83 100644 --- a/tests/ui/async-await/async-closures/wrong-fn-kind.stderr +++ b/tests/ui/async-await/async-closures/wrong-fn-kind.stderr @@ -1,5 +1,5 @@ error[E0525]: expected a closure that implements the `async Fn` trait, but this closure only implements `async FnMut` - --> $DIR/wrong-fn-kind.rs:11:20 + --> $DIR/wrong-fn-kind.rs:9:20 | LL | needs_async_fn(async || { | -------------- -^^^^^^^ @@ -14,11 +14,32 @@ LL | | }); | |_____- the requirement to implement `async Fn` derives from here | note: required by a bound in `needs_async_fn` - --> $DIR/wrong-fn-kind.rs:8:31 + --> $DIR/wrong-fn-kind.rs:6:31 | LL | fn needs_async_fn(_: impl async Fn()) {} | ^^^^^^^^^^ required by this bound in `needs_async_fn` -error: aborting due to 1 previous error +error[E0525]: expected a closure that implements the `async Fn` trait, but this closure only implements `async FnOnce` + --> $DIR/wrong-fn-kind.rs:15:20 + | +LL | needs_async_fn(move || async move { + | -------------- -^^^^^^ + | | | + | _____|______________this closure implements `async FnOnce`, not `async Fn` + | | | + | | required by a bound introduced by this call +LL | | +LL | | println!("{x}"); + | | - closure is `async FnOnce` because it moves the variable `x` out of its environment +LL | | }); + | |_____- the requirement to implement `async Fn` derives from here + | +note: required by a bound in `needs_async_fn` + --> $DIR/wrong-fn-kind.rs:6:31 + | +LL | fn needs_async_fn(_: impl async Fn()) {} + | ^^^^^^^^^^ required by this bound in `needs_async_fn` + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0525`. From 297abc35b3625335a6aa9c3541be040ac040cbe9 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 26 Feb 2024 10:40:48 +0100 Subject: [PATCH 6/7] fix some references to no-longer-existing ReprOptions.layout_seed --- compiler/rustc_abi/src/layout.rs | 2 +- compiler/rustc_abi/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index ec3ab828b7194..28e148bddb220 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -958,7 +958,7 @@ fn univariant< #[cfg(feature = "randomize")] { use rand::{seq::SliceRandom, SeedableRng}; - // `ReprOptions.layout_seed` is a deterministic seed we can use to randomize field + // `ReprOptions.field_shuffle_seed` is a deterministic seed we can use to randomize field // ordering. let mut rng = rand_xoshiro::Xoshiro128StarStar::seed_from_u64(repr.field_shuffle_seed); diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index c45a4a410f964..297dbf890290d 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -41,7 +41,7 @@ bitflags! { // Internal only for now. If true, don't reorder fields. const IS_LINEAR = 1 << 3; // If true, the type's layout can be randomized using - // the seed stored in `ReprOptions.layout_seed` + // the seed stored in `ReprOptions.field_shuffle_seed` const RANDOMIZE_LAYOUT = 1 << 4; // Any of these flags being set prevent field reordering optimisation. const IS_UNOPTIMISABLE = ReprFlags::IS_C.bits() From 633c92cd6de9e269b9c5b10f341fc10280503f0c Mon Sep 17 00:00:00 2001 From: Gurinder Singh Date: Mon, 26 Feb 2024 15:22:22 +0530 Subject: [PATCH 7/7] Do not const pop unions as they can made to produce values whose types don't match their underlying layout types which can lead to ICEs on eval --- .../src/known_panics_lint.rs | 40 ++++++++++++------- ...e-unions-known-panics-lint-issue-121534.rs | 21 ++++++++++ 2 files changed, 47 insertions(+), 14 deletions(-) create mode 100644 tests/ui/lint/ice-unions-known-panics-lint-issue-121534.rs diff --git a/compiler/rustc_mir_transform/src/known_panics_lint.rs b/compiler/rustc_mir_transform/src/known_panics_lint.rs index a9cd688c31551..7cab665099482 100644 --- a/compiler/rustc_mir_transform/src/known_panics_lint.rs +++ b/compiler/rustc_mir_transform/src/known_panics_lint.rs @@ -585,20 +585,32 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { val.into() } - Aggregate(ref kind, ref fields) => Value::Aggregate { - fields: fields - .iter() - .map(|field| self.eval_operand(field).map_or(Value::Uninit, Value::Immediate)) - .collect(), - variant: match **kind { - AggregateKind::Adt(_, variant, _, _, _) => variant, - AggregateKind::Array(_) - | AggregateKind::Tuple - | AggregateKind::Closure(_, _) - | AggregateKind::Coroutine(_, _) - | AggregateKind::CoroutineClosure(_, _) => VariantIdx::new(0), - }, - }, + Aggregate(ref kind, ref fields) => { + // Do not const pop union fields as they can be + // made to produce values that don't match their + // underlying layout's type (see ICE #121534). + // If the last element of the `Adt` tuple + // is `Some` it indicates the ADT is a union + if let AggregateKind::Adt(_, _, _, _, Some(_)) = **kind { + return None; + }; + Value::Aggregate { + fields: fields + .iter() + .map(|field| { + self.eval_operand(field).map_or(Value::Uninit, Value::Immediate) + }) + .collect(), + variant: match **kind { + AggregateKind::Adt(_, variant, _, _, _) => variant, + AggregateKind::Array(_) + | AggregateKind::Tuple + | AggregateKind::Closure(_, _) + | AggregateKind::Coroutine(_, _) + | AggregateKind::CoroutineClosure(_, _) => VariantIdx::new(0), + }, + } + } Repeat(ref op, n) => { trace!(?op, ?n); diff --git a/tests/ui/lint/ice-unions-known-panics-lint-issue-121534.rs b/tests/ui/lint/ice-unions-known-panics-lint-issue-121534.rs new file mode 100644 index 0000000000000..9fadb828b3d2d --- /dev/null +++ b/tests/ui/lint/ice-unions-known-panics-lint-issue-121534.rs @@ -0,0 +1,21 @@ +// Regression test for #121534 +// Tests that no ICE occurs in KnownPanicsLint when it +// evaluates an operation whose operands have different +// layout types even though they have the same type. +// This situation can be contrived through the use of +// unions as in this test + +//@ build-pass +union Union { + u32_field: u32, + i32_field: i32, +} + +pub fn main() { + let u32_variant = Union { u32_field: 2 }; + let i32_variant = Union { i32_field: 3 }; + let a = unsafe { u32_variant.u32_field }; + let b = unsafe { i32_variant.u32_field }; + + let _diff = a - b; +}