From 01f3cc127227e607c2699fcd5fb2d17f1217a0cd Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 24 Jul 2023 21:15:11 +1000 Subject: [PATCH 01/10] coverage: Obtain the `__llvm_covfun` section name outside a per-function loop This section name is always constant for a given target, but obtaining it from LLVM requires a few intermediate allocations. There's no need to do so repeatedly from inside a per-function loop. --- .../src/coverageinfo/mapgen.rs | 11 +++++++- .../src/coverageinfo/mod.rs | 28 ++++++++++++++----- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs index 6e815ab3ff511..9149b06886b38 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs @@ -100,9 +100,11 @@ pub fn finalize(cx: &CodegenCx<'_, '_>) { // Generate the LLVM IR representation of the coverage map and store it in a well-known global let cov_data_val = mapgen.generate_coverage_map(cx, version, filenames_size, filenames_val); + let covfun_section_name = coverageinfo::covfun_section_name(cx); for (mangled_function_name, source_hash, is_used, coverage_mapping_buffer) in function_data { save_function_record( cx, + &covfun_section_name, mangled_function_name, source_hash, filenames_ref, @@ -228,6 +230,7 @@ impl CoverageMapGenerator { /// specific, well-known section and name. fn save_function_record( cx: &CodegenCx<'_, '_>, + covfun_section_name: &str, mangled_function_name: &str, source_hash: u64, filenames_ref: u64, @@ -254,7 +257,13 @@ fn save_function_record( /*packed=*/ true, ); - coverageinfo::save_func_record_to_mod(cx, func_name_hash, func_record_val, is_used); + coverageinfo::save_func_record_to_mod( + cx, + covfun_section_name, + func_name_hash, + func_record_val, + is_used, + ); } /// When finalizing the coverage map, `FunctionCoverage` only has the `CodeRegion`s and counters for diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 1f8f403393131..60cbb3d3d9d6b 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -408,6 +408,7 @@ pub(crate) fn save_cov_data_to_mod<'ll, 'tcx>( pub(crate) fn save_func_record_to_mod<'ll, 'tcx>( cx: &CodegenCx<'ll, 'tcx>, + covfun_section_name: &str, func_name_hash: u64, func_record_val: &'ll llvm::Value, is_used: bool, @@ -423,20 +424,33 @@ pub(crate) fn save_func_record_to_mod<'ll, 'tcx>( let func_record_var_name = format!("__covrec_{:X}{}", func_name_hash, if is_used { "u" } else { "" }); debug!("function record var name: {:?}", func_record_var_name); - - let func_record_section_name = llvm::build_string(|s| unsafe { - llvm::LLVMRustCoverageWriteFuncSectionNameToString(cx.llmod, s); - }) - .expect("Rust Coverage function record section name failed UTF-8 conversion"); - debug!("function record section name: {:?}", func_record_section_name); + debug!("function record section name: {:?}", covfun_section_name); let llglobal = llvm::add_global(cx.llmod, cx.val_ty(func_record_val), &func_record_var_name); llvm::set_initializer(llglobal, func_record_val); llvm::set_global_constant(llglobal, true); llvm::set_linkage(llglobal, llvm::Linkage::LinkOnceODRLinkage); llvm::set_visibility(llglobal, llvm::Visibility::Hidden); - llvm::set_section(llglobal, &func_record_section_name); + llvm::set_section(llglobal, covfun_section_name); llvm::set_alignment(llglobal, VAR_ALIGN_BYTES); llvm::set_comdat(cx.llmod, llglobal, &func_record_var_name); cx.add_used_global(llglobal); } + +/// Returns the section name string to pass through to the linker when embedding +/// per-function coverage information in the object file, according to the target +/// platform's object file format. +/// +/// LLVM's coverage tools read coverage mapping details from this section when +/// producing coverage reports. +/// +/// Typical values are: +/// - `__llvm_covfun` on Linux +/// - `__LLVM_COV,__llvm_covfun` on macOS (includes `__LLVM_COV,` segment prefix) +/// - `.lcovfun$M` on Windows (includes `$M` sorting suffix) +pub(crate) fn covfun_section_name(cx: &CodegenCx<'_, '_>) -> String { + llvm::build_string(|s| unsafe { + llvm::LLVMRustCoverageWriteFuncSectionNameToString(cx.llmod, s); + }) + .expect("Rust Coverage function record section name failed UTF-8 conversion") +} From c06a7eb2a6dcbc7649de76472f6845e30f2fb97f Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 24 Jul 2023 15:04:29 +0100 Subject: [PATCH 02/10] builtin_macros: expect raw strings too `expr_to_string` allows raw strings through so this code should be expected to handle those. Signed-off-by: David Wood --- compiler/rustc_builtin_macros/src/env.rs | 2 +- tests/ui/macros/builtin-env-issue-114010.rs | 6 ++++++ tests/ui/macros/builtin-env-issue-114010.stderr | 11 +++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 tests/ui/macros/builtin-env-issue-114010.rs create mode 100644 tests/ui/macros/builtin-env-issue-114010.stderr diff --git a/compiler/rustc_builtin_macros/src/env.rs b/compiler/rustc_builtin_macros/src/env.rs index bcff475f62672..30d74ba593c97 100644 --- a/compiler/rustc_builtin_macros/src/env.rs +++ b/compiler/rustc_builtin_macros/src/env.rs @@ -84,7 +84,7 @@ pub fn expand_env<'cx>( // Use the string literal in the code in the diagnostic to avoid confusing diagnostics, // e.g. when the literal contains escape sequences. let ast::ExprKind::Lit(ast::token::Lit { - kind: ast::token::LitKind::Str, + kind: ast::token::LitKind::Str | ast::token::LitKind::StrRaw(..), symbol: original_var, .. }) = &var_expr.kind diff --git a/tests/ui/macros/builtin-env-issue-114010.rs b/tests/ui/macros/builtin-env-issue-114010.rs new file mode 100644 index 0000000000000..eb97c767fa2e9 --- /dev/null +++ b/tests/ui/macros/builtin-env-issue-114010.rs @@ -0,0 +1,6 @@ +// unset-rustc-env:oopsie + +env![r#"oopsie"#]; +//~^ ERROR environment variable `oopsie` not defined at compile time + +fn main() {} diff --git a/tests/ui/macros/builtin-env-issue-114010.stderr b/tests/ui/macros/builtin-env-issue-114010.stderr new file mode 100644 index 0000000000000..7934ce1b79099 --- /dev/null +++ b/tests/ui/macros/builtin-env-issue-114010.stderr @@ -0,0 +1,11 @@ +error: environment variable `oopsie` not defined at compile time + --> $DIR/builtin-env-issue-114010.rs:3:1 + | +LL | env![r#"oopsie"#]; + | ^^^^^^^^^^^^^^^^^ + | + = help: use `std::env::var("oopsie")` to read the variable at run time + = note: this error originates in the macro `env` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to previous error + From 3857d9c2da1250546ccb87ab7ddcc68904f45dc5 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 24 Jul 2023 15:47:03 +0100 Subject: [PATCH 03/10] borrowck/errors: fix i18n error in delayed bug During borrowck, the `MultiSpan` from a buffered diagnostic is cloned and used to emit a delayed bug indicating a diagnostic was buffered - when the buffered diagnostic is translated, then the cloned `MultiSpan` may contain labels which can only render with the diagnostic's arguments, but the delayed bug being emitted won't have those arguments. Adds a function which clones `MultiSpan` without also cloning the contained labels, and use this function when creating the buffered diagnostic delayed bug. Signed-off-by: David Wood --- compiler/rustc_borrowck/src/lib.rs | 9 ++++----- compiler/rustc_error_messages/src/lib.rs | 8 ++++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index ea32506fc8980..2fd42e2f56a63 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -2306,11 +2306,10 @@ mod error { pub fn buffer_error(&mut self, t: DiagnosticBuilder<'_, ErrorGuaranteed>) { if let None = self.tainted_by_errors { - self.tainted_by_errors = Some( - self.tcx - .sess - .delay_span_bug(t.span.clone(), "diagnostic buffered but not emitted"), - ) + self.tainted_by_errors = Some(self.tcx.sess.delay_span_bug( + t.span.clone_ignoring_labels(), + "diagnostic buffered but not emitted", + )) } t.buffer(&mut self.buffered); } diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs index 1879ece59e329..a49b842d9ffea 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -533,6 +533,14 @@ impl MultiSpan { pub fn has_span_labels(&self) -> bool { self.span_labels.iter().any(|(sp, _)| !sp.is_dummy()) } + + /// Clone this `MultiSpan` without keeping any of the span labels - sometimes a `MultiSpan` is + /// to be re-used in another diagnostic, but includes `span_labels` which have translated + /// messages. These translated messages would fail to translate without their diagnostic + /// arguments which are unlikely to be cloned alongside the `Span`. + pub fn clone_ignoring_labels(&self) -> Self { + Self { primary_spans: self.primary_spans.clone(), ..MultiSpan::new() } + } } impl From for MultiSpan { From 40dd5a337cf8e565ae8c364e69e4e4e2d83ea4a7 Mon Sep 17 00:00:00 2001 From: DragonBillow Date: Tue, 25 Jul 2023 12:21:30 +0800 Subject: [PATCH 04/10] docs(LazyLock): add example pass local LazyLock variable to struct Signed-off-by: DragonBillow --- library/std/src/sync/lazy_lock.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/library/std/src/sync/lazy_lock.rs b/library/std/src/sync/lazy_lock.rs index a6bc468b09281..37bdec5abcc74 100644 --- a/library/std/src/sync/lazy_lock.rs +++ b/library/std/src/sync/lazy_lock.rs @@ -25,6 +25,8 @@ union Data { /// /// # Examples /// +/// Initialize static variables with `LazyLock`. +/// /// ``` /// #![feature(lazy_cell)] /// @@ -54,6 +56,24 @@ union Data { /// // Some("Hoyten") /// } /// ``` +/// Initialize fields with `LazyLock`. +/// ``` +/// #![feature(lazy_cell)] +/// +/// use std::sync::LazyLock; +/// +/// #[derive(Debug)] +/// struct UseCellLock { +/// number: LazyLock, +/// } +/// fn main() { +/// let lock: LazyLock = LazyLock::new(|| 0u32); +/// +/// let data = UseCellLock { number: lock }; +/// println!("{}", *data.number); +/// } +/// ``` + #[unstable(feature = "lazy_cell", issue = "109736")] pub struct LazyLock T> { once: Once, From 75df62d4a2c45175b8cb1eb4bdc10fa423c11c4c Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 25 Jul 2023 11:12:52 +0100 Subject: [PATCH 05/10] builtin_macros: raw str in diagnostic output If a raw string was used in the `env!` invocation, then it should also be shown in the diagnostic messages as a raw string. Signed-off-by: David Wood --- compiler/rustc_builtin_macros/messages.ftl | 4 +- compiler/rustc_builtin_macros/src/env.rs | 41 ++++++++------- compiler/rustc_builtin_macros/src/errors.rs | 52 +++++++++---------- compiler/rustc_errors/src/diagnostic_impls.rs | 6 +++ tests/ui/macros/builtin-env-issue-114010.rs | 4 ++ .../ui/macros/builtin-env-issue-114010.stderr | 15 ++++-- 6 files changed, 73 insertions(+), 49 deletions(-) diff --git a/compiler/rustc_builtin_macros/messages.ftl b/compiler/rustc_builtin_macros/messages.ftl index 322222ae33039..79fae7f92f1ee 100644 --- a/compiler/rustc_builtin_macros/messages.ftl +++ b/compiler/rustc_builtin_macros/messages.ftl @@ -109,8 +109,8 @@ builtin_macros_derive_path_args_value = traits in `#[derive(...)]` don't accept .suggestion = remove the value builtin_macros_env_not_defined = environment variable `{$var}` not defined at compile time - .cargo = Cargo sets build script variables at run time. Use `std::env::var("{$var}")` instead - .other = use `std::env::var("{$var}")` to read the variable at run time + .cargo = Cargo sets build script variables at run time. Use `std::env::var({$var_expr})` instead + .custom = use `std::env::var({$var_expr})` to read the variable at run time builtin_macros_env_takes_args = `env!()` takes 1 or 2 arguments diff --git a/compiler/rustc_builtin_macros/src/env.rs b/compiler/rustc_builtin_macros/src/env.rs index 30d74ba593c97..92da0c069e51a 100644 --- a/compiler/rustc_builtin_macros/src/env.rs +++ b/compiler/rustc_builtin_macros/src/env.rs @@ -4,7 +4,7 @@ // use rustc_ast::tokenstream::TokenStream; -use rustc_ast::{self as ast, GenericArg}; +use rustc_ast::{self as ast, AstDeref, GenericArg}; use rustc_expand::base::{self, *}; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::Span; @@ -76,27 +76,36 @@ pub fn expand_env<'cx>( }, }; - let sp = cx.with_def_site_ctxt(sp); + let span = cx.with_def_site_ctxt(sp); let value = env::var(var.as_str()).ok().as_deref().map(Symbol::intern); cx.sess.parse_sess.env_depinfo.borrow_mut().insert((var, value)); let e = match value { None => { - // Use the string literal in the code in the diagnostic to avoid confusing diagnostics, - // e.g. when the literal contains escape sequences. let ast::ExprKind::Lit(ast::token::Lit { kind: ast::token::LitKind::Str | ast::token::LitKind::StrRaw(..), - symbol: original_var, + symbol, .. }) = &var_expr.kind else { unreachable!("`expr_to_string` ensures this is a string lit") }; - cx.emit_err(errors::EnvNotDefined { - span: sp, - msg: custom_msg, - var: *original_var, - help: custom_msg.is_none().then(|| help_for_missing_env_var(var.as_str())), - }); + + if let Some(msg_from_user) = custom_msg { + cx.emit_err(errors::EnvNotDefinedWithUserMessage { span, msg_from_user }); + } else if is_cargo_env_var(var.as_str()) { + cx.emit_err(errors::EnvNotDefined::CargoEnvVar { + span, + var: *symbol, + var_expr: var_expr.ast_deref(), + }); + } else { + cx.emit_err(errors::EnvNotDefined::CustomEnvVar { + span, + var: *symbol, + var_expr: var_expr.ast_deref(), + }); + } + return DummyResult::any(sp); } Some(value) => cx.expr_str(sp, value), @@ -104,13 +113,9 @@ pub fn expand_env<'cx>( MacEager::expr(e) } -fn help_for_missing_env_var(var: &str) -> errors::EnvNotDefinedHelp { - if var.starts_with("CARGO_") +/// Returns `true` if an environment variable from `env!` is one used by Cargo. +fn is_cargo_env_var(var: &str) -> bool { + var.starts_with("CARGO_") || var.starts_with("DEP_") || matches!(var, "OUT_DIR" | "OPT_LEVEL" | "PROFILE" | "HOST" | "TARGET") - { - errors::EnvNotDefinedHelp::CargoVar - } else { - errors::EnvNotDefinedHelp::Other - } } diff --git a/compiler/rustc_builtin_macros/src/errors.rs b/compiler/rustc_builtin_macros/src/errors.rs index 7b2a375a822bf..fbf0395bb05ac 100644 --- a/compiler/rustc_builtin_macros/src/errors.rs +++ b/compiler/rustc_builtin_macros/src/errors.rs @@ -440,43 +440,43 @@ pub(crate) struct EnvTakesArgs { pub(crate) span: Span, } -//#[derive(Diagnostic)] -//#[diag(builtin_macros_env_not_defined)] -pub(crate) struct EnvNotDefined { +pub(crate) struct EnvNotDefinedWithUserMessage { pub(crate) span: Span, - pub(crate) msg: Option, - pub(crate) var: Symbol, - pub(crate) help: Option, + pub(crate) msg_from_user: Symbol, } -// Hand-written implementation to support custom user messages -impl<'a, G: EmissionGuarantee> IntoDiagnostic<'a, G> for EnvNotDefined { +// Hand-written implementation to support custom user messages. +impl<'a, G: EmissionGuarantee> IntoDiagnostic<'a, G> for EnvNotDefinedWithUserMessage { #[track_caller] fn into_diagnostic(self, handler: &'a Handler) -> DiagnosticBuilder<'a, G> { - let mut diag = if let Some(msg) = self.msg { - #[expect( - rustc::untranslatable_diagnostic, - reason = "cannot translate user-provided messages" - )] - handler.struct_diagnostic(msg.to_string()) - } else { - handler.struct_diagnostic(crate::fluent_generated::builtin_macros_env_not_defined) - }; - diag.set_arg("var", self.var); + #[expect( + rustc::untranslatable_diagnostic, + reason = "cannot translate user-provided messages" + )] + let mut diag = handler.struct_diagnostic(self.msg_from_user.to_string()); diag.set_span(self.span); - if let Some(help) = self.help { - diag.subdiagnostic(help); - } diag } } -#[derive(Subdiagnostic)] -pub(crate) enum EnvNotDefinedHelp { +#[derive(Diagnostic)] +pub(crate) enum EnvNotDefined<'a> { + #[diag(builtin_macros_env_not_defined)] #[help(builtin_macros_cargo)] - CargoVar, - #[help(builtin_macros_other)] - Other, + CargoEnvVar { + #[primary_span] + span: Span, + var: Symbol, + var_expr: &'a rustc_ast::Expr, + }, + #[diag(builtin_macros_env_not_defined)] + #[help(builtin_macros_custom)] + CustomEnvVar { + #[primary_span] + span: Span, + var: Symbol, + var_expr: &'a rustc_ast::Expr, + }, } #[derive(Diagnostic)] diff --git a/compiler/rustc_errors/src/diagnostic_impls.rs b/compiler/rustc_errors/src/diagnostic_impls.rs index 10fe7fc74a873..fb2e84bb7a130 100644 --- a/compiler/rustc_errors/src/diagnostic_impls.rs +++ b/compiler/rustc_errors/src/diagnostic_impls.rs @@ -164,6 +164,12 @@ impl IntoDiagnosticArg for hir::ConstContext { } } +impl IntoDiagnosticArg for ast::Expr { + fn into_diagnostic_arg(self) -> DiagnosticArgValue<'static> { + DiagnosticArgValue::Str(Cow::Owned(pprust::expr_to_string(&self))) + } +} + impl IntoDiagnosticArg for ast::Path { fn into_diagnostic_arg(self) -> DiagnosticArgValue<'static> { DiagnosticArgValue::Str(Cow::Owned(pprust::path_to_string(&self))) diff --git a/tests/ui/macros/builtin-env-issue-114010.rs b/tests/ui/macros/builtin-env-issue-114010.rs index eb97c767fa2e9..819b8b1e8abe0 100644 --- a/tests/ui/macros/builtin-env-issue-114010.rs +++ b/tests/ui/macros/builtin-env-issue-114010.rs @@ -1,6 +1,10 @@ // unset-rustc-env:oopsie +// unset-rustc-env:a""a env![r#"oopsie"#]; //~^ ERROR environment variable `oopsie` not defined at compile time +env![r#"a""a"#]; +//~^ ERROR environment variable `a""a` not defined at compile time + fn main() {} diff --git a/tests/ui/macros/builtin-env-issue-114010.stderr b/tests/ui/macros/builtin-env-issue-114010.stderr index 7934ce1b79099..0da42089cce77 100644 --- a/tests/ui/macros/builtin-env-issue-114010.stderr +++ b/tests/ui/macros/builtin-env-issue-114010.stderr @@ -1,11 +1,20 @@ error: environment variable `oopsie` not defined at compile time - --> $DIR/builtin-env-issue-114010.rs:3:1 + --> $DIR/builtin-env-issue-114010.rs:4:1 | LL | env![r#"oopsie"#]; | ^^^^^^^^^^^^^^^^^ | - = help: use `std::env::var("oopsie")` to read the variable at run time + = help: use `std::env::var(r#"oopsie"#)` to read the variable at run time = note: this error originates in the macro `env` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to previous error +error: environment variable `a""a` not defined at compile time + --> $DIR/builtin-env-issue-114010.rs:7:1 + | +LL | env![r#"a""a"#]; + | ^^^^^^^^^^^^^^^ + | + = help: use `std::env::var(r#"a""a"#)` to read the variable at run time + = note: this error originates in the macro `env` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 2 previous errors From 5d32fd1b0623651cca15acd71ecc901324f3170c Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Tue, 25 Jul 2023 12:42:54 +0200 Subject: [PATCH 06/10] Add regression test for invalid unused const in method The warning can be reproduced with 1.63 but not with 1.64. $ rustc +1.63 tests/ui/lint/unused/const-local-var.rs warning: constant `F` is never used --> tests/ui/lint/unused/const-local-var.rs:14:9 | 14 | const F: i32 = 2; | ^^^^^^^^^^^^^^^^^ | = note: `#[warn(dead_code)]` on by default $ rustc +1.64 tests/ui/lint/unused/const-local-var.rs Add a regression test to prevent the problem from re-appearing. --- tests/ui/lint/unused/const-local-var.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 tests/ui/lint/unused/const-local-var.rs diff --git a/tests/ui/lint/unused/const-local-var.rs b/tests/ui/lint/unused/const-local-var.rs new file mode 100644 index 0000000000000..89ca16fe003e9 --- /dev/null +++ b/tests/ui/lint/unused/const-local-var.rs @@ -0,0 +1,23 @@ +// regression test for https://github.com/rust-lang/rust/issues/69016 +// check-pass + +#![warn(unused)] +#![deny(warnings)] + +fn _unused1(x: i32) -> i32 { + const F: i32 = 2; + let g = 1; + x * F + g +} + +pub struct Foo {} + +impl Foo { + fn _unused2(x: i32) -> i32 { + const F: i32 = 2; + let g = 1; + x * F + g + } +} + +fn main() {} From 516a56581e4c72381991a09bf7c2da7ae85f7948 Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 25 Jul 2023 11:56:54 +0100 Subject: [PATCH 07/10] clippy: `env!` invocations can't be b"" literals Signed-off-by: David Wood --- src/tools/clippy/clippy_lints/src/strings.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/clippy/clippy_lints/src/strings.rs b/src/tools/clippy/clippy_lints/src/strings.rs index 58724852b3c4d..4d45091f4b5b7 100644 --- a/src/tools/clippy/clippy_lints/src/strings.rs +++ b/src/tools/clippy/clippy_lints/src/strings.rs @@ -328,7 +328,7 @@ impl<'tcx> LateLintPass<'tcx> for StringLitAsBytes { { // Don't lint. Byte strings produce `&[u8; N]` whereas `as_bytes()` produces // `&[u8]`. This change would prevent matching with different sized slices. - } else { + } else if !callsite.starts_with("env!") { span_lint_and_sugg( cx, STRING_LIT_AS_BYTES, From c83dfe9aedf79942e3416749b00803e0b5dcb5f2 Mon Sep 17 00:00:00 2001 From: clubby789 Date: Tue, 25 Jul 2023 11:23:11 +0000 Subject: [PATCH 08/10] Suggest `{Option,Result}::as_ref()` instead of `cloned()` in some cases --- compiler/rustc_hir_typeck/src/demand.rs | 2 +- .../src/fn_ctxt/suggestions.rs | 19 +++++++++++++++---- .../dont-suggest-cyclic-constraint.fixed | 13 +++++++++++++ .../dont-suggest-cyclic-constraint.rs | 4 +++- .../dont-suggest-cyclic-constraint.stderr | 6 +++++- tests/ui/suggestions/copied-and-cloned.fixed | 8 ++++++++ tests/ui/suggestions/copied-and-cloned.rs | 8 ++++++++ tests/ui/suggestions/copied-and-cloned.stderr | 15 ++++++++++++++- 8 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 tests/ui/associated-types/dont-suggest-cyclic-constraint.fixed diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index 45085534f72fb..26fa3d80d553f 100644 --- a/compiler/rustc_hir_typeck/src/demand.rs +++ b/compiler/rustc_hir_typeck/src/demand.rs @@ -53,7 +53,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { || self.suggest_no_capture_closure(err, expected, expr_ty) || self.suggest_boxing_when_appropriate(err, expr.span, expr.hir_id, expected, expr_ty) || self.suggest_block_to_brackets_peeling_refs(err, expr, expr_ty, expected) - || self.suggest_copied_or_cloned(err, expr, expr_ty, expected) + || self.suggest_copied_cloned_or_as_ref(err, expr, expr_ty, expected, expected_ty_expr) || self.suggest_clone_for_ref(err, expr, expr_ty, expected) || self.suggest_into(err, expr, expr_ty, expected) || self.suggest_floating_point_literal(err, expr, expected) diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs index ec19d017c259b..acea5f8c1f63f 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs @@ -1085,12 +1085,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { false } - pub(crate) fn suggest_copied_or_cloned( + pub(crate) fn suggest_copied_cloned_or_as_ref( &self, diag: &mut Diagnostic, expr: &hir::Expr<'_>, expr_ty: Ty<'tcx>, expected_ty: Ty<'tcx>, + expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>, ) -> bool { let ty::Adt(adt_def, args) = expr_ty.kind() else { return false; @@ -1102,7 +1103,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return false; } - let mut suggest_copied_or_cloned = || { + let mut suggest_copied_cloned_or_as_ref = || { let expr_inner_ty = args.type_at(0); let expected_inner_ty = expected_args.type_at(0); if let &ty::Ref(_, ty, hir::Mutability::Not) = expr_inner_ty.kind() @@ -1119,6 +1120,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Applicability::MachineApplicable, ); return true; + } else if let Some(expected_ty_expr) = expected_ty_expr { + diag.span_suggestion_verbose( + expected_ty_expr.span.shrink_to_hi(), + format!( + "use `{def_path}::as_ref()` to convert `{expected_ty}` to `{expr_ty}`" + ), + ".as_ref()", + Applicability::MachineApplicable, + ); + return true; } else if let Some(clone_did) = self.tcx.lang_items().clone_trait() && rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions( self, @@ -1146,11 +1157,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Check that the error types are equal && self.can_eq(self.param_env, args.type_at(1), expected_args.type_at(1)) { - return suggest_copied_or_cloned(); + return suggest_copied_cloned_or_as_ref(); } else if let Some(option_did) = self.tcx.get_diagnostic_item(sym::Option) && adt_def.did() == option_did { - return suggest_copied_or_cloned(); + return suggest_copied_cloned_or_as_ref(); } false diff --git a/tests/ui/associated-types/dont-suggest-cyclic-constraint.fixed b/tests/ui/associated-types/dont-suggest-cyclic-constraint.fixed new file mode 100644 index 0000000000000..ec4165cc71e28 --- /dev/null +++ b/tests/ui/associated-types/dont-suggest-cyclic-constraint.fixed @@ -0,0 +1,13 @@ +// run-rustfix + +use std::fmt::Debug; + +pub fn foo(mut iter: I, value: &I::Item) +where + I::Item: Eq + Debug, +{ + debug_assert_eq!(iter.next().as_ref(), Some(value)); + //~^ ERROR mismatched types +} + +fn main() {} diff --git a/tests/ui/associated-types/dont-suggest-cyclic-constraint.rs b/tests/ui/associated-types/dont-suggest-cyclic-constraint.rs index 6894f6b6cc4a0..0b4df08783d63 100644 --- a/tests/ui/associated-types/dont-suggest-cyclic-constraint.rs +++ b/tests/ui/associated-types/dont-suggest-cyclic-constraint.rs @@ -1,6 +1,8 @@ +// run-rustfix + use std::fmt::Debug; -fn foo(mut iter: I, value: &I::Item) +pub fn foo(mut iter: I, value: &I::Item) where I::Item: Eq + Debug, { diff --git a/tests/ui/associated-types/dont-suggest-cyclic-constraint.stderr b/tests/ui/associated-types/dont-suggest-cyclic-constraint.stderr index 3ecac9c83e571..c06c506a311a8 100644 --- a/tests/ui/associated-types/dont-suggest-cyclic-constraint.stderr +++ b/tests/ui/associated-types/dont-suggest-cyclic-constraint.stderr @@ -1,11 +1,15 @@ error[E0308]: mismatched types - --> $DIR/dont-suggest-cyclic-constraint.rs:7:35 + --> $DIR/dont-suggest-cyclic-constraint.rs:9:35 | LL | debug_assert_eq!(iter.next(), Some(value)); | ^^^^^^^^^^^ expected `Option<::Item>`, found `Option<&::Item>` | = note: expected enum `Option<::Item>` found enum `Option<&::Item>` +help: use `Option::as_ref()` to convert `Option<::Item>` to `Option<&::Item>` + | +LL | debug_assert_eq!(iter.next().as_ref(), Some(value)); + | +++++++++ error: aborting due to previous error diff --git a/tests/ui/suggestions/copied-and-cloned.fixed b/tests/ui/suggestions/copied-and-cloned.fixed index f801403feec4f..13031f424cbe7 100644 --- a/tests/ui/suggestions/copied-and-cloned.fixed +++ b/tests/ui/suggestions/copied-and-cloned.fixed @@ -20,4 +20,12 @@ fn main() { expect::>(x.cloned()); //~^ ERROR mismatched types //~| HELP use `Result::cloned` to clone the value inside the `Result` + + let s = String::new(); + let x = Some(s.clone()); + let y = Some(&s); + println!("{}", x.as_ref() == y); + //~^ ERROR mismatched types + //~| HELP use `Option::as_ref()` to convert `Option` to `Option<&String>` + } diff --git a/tests/ui/suggestions/copied-and-cloned.rs b/tests/ui/suggestions/copied-and-cloned.rs index 640450b765527..2927d66dea94e 100644 --- a/tests/ui/suggestions/copied-and-cloned.rs +++ b/tests/ui/suggestions/copied-and-cloned.rs @@ -20,4 +20,12 @@ fn main() { expect::>(x); //~^ ERROR mismatched types //~| HELP use `Result::cloned` to clone the value inside the `Result` + + let s = String::new(); + let x = Some(s.clone()); + let y = Some(&s); + println!("{}", x == y); + //~^ ERROR mismatched types + //~| HELP use `Option::as_ref()` to convert `Option` to `Option<&String>` + } diff --git a/tests/ui/suggestions/copied-and-cloned.stderr b/tests/ui/suggestions/copied-and-cloned.stderr index 0678081418248..19aaf6e00b133 100644 --- a/tests/ui/suggestions/copied-and-cloned.stderr +++ b/tests/ui/suggestions/copied-and-cloned.stderr @@ -78,6 +78,19 @@ help: use `Result::cloned` to clone the value inside the `Result` LL | expect::>(x.cloned()); | +++++++++ -error: aborting due to 4 previous errors +error[E0308]: mismatched types + --> $DIR/copied-and-cloned.rs:27:25 + | +LL | println!("{}", x == y); + | ^ expected `Option`, found `Option<&String>` + | + = note: expected enum `Option` + found enum `Option<&String>` +help: use `Option::as_ref()` to convert `Option` to `Option<&String>` + | +LL | println!("{}", x.as_ref() == y); + | +++++++++ + +error: aborting due to 5 previous errors For more information about this error, try `rustc --explain E0308`. From 037b27430bc338ca5722aeb87a4614e70a5b3df4 Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 25 Jul 2023 15:24:58 +0100 Subject: [PATCH 09/10] abi: unsized field in union - assert to delay bug Unions cannot have unsized fields, and as such, layout computation for unions asserts that each union field is sized (as this would normally have halted compilation earlier). However, if a generator ends up with an unsized local - a circumstance in which an error will always have been emitted earlier, for example, if attempting to dereference a `&str` - then the generator transform will produce a union with an unsized field. Since #110107, later passes will be run, such as constant propagation, and can attempt layout computation on the generator, which will result in layout computation of `str` in the context of it being a field of a union - and so the aforementioned assertion would cause an ICE. It didn't seem appropriate to try and detect this case in the MIR body and skip this specific pass; tainting the MIR body or delaying a bug from the generator transform (or elsewhere) wouldn't prevent this either (as neither would prevent the later pass from running); and tainting when the deref of `&str` is reported, if that's possible, would unnecessarily prevent potential other errors from being reported later in compilation, and is very tailored to this specific case of getting a unsized type in a generator. Given that this circumstance can only happen when an error should have already been reported, the correct fix appears to be just changing the assert to a delayed bug. This will still assert if there is some circumstance where this occurs and no error has been reported, but it won't crash the compiler in this instance. Signed-off-by: David Wood --- compiler/rustc_abi/src/layout.rs | 4 +++- compiler/rustc_abi/src/lib.rs | 1 - tests/ui/generator/issue-113279.rs | 27 ++++++++++++++++++++++++++ tests/ui/generator/issue-113279.stderr | 16 +++++++++++++++ 4 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 tests/ui/generator/issue-113279.rs create mode 100644 tests/ui/generator/issue-113279.stderr diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index aea88641f82b1..489a8403c3b1f 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -763,7 +763,9 @@ pub trait LayoutCalculator { let mut size = Size::ZERO; let only_variant = &variants[FIRST_VARIANT]; for field in only_variant { - assert!(field.0.is_sized()); + if field.0.is_unsized() { + self.delay_bug("unsized field in union".to_string()); + } align = align.max(field.align()); max_repr_align = max_repr_align.max(field.max_repr_align()); diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index ef0c763ac2038..93b29d037d67e 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -1345,7 +1345,6 @@ impl Abi { /// Discard validity range information and allow undef. pub fn to_union(&self) -> Self { - assert!(self.is_sized()); match *self { Abi::Scalar(s) => Abi::Scalar(s.to_union()), Abi::ScalarPair(s1, s2) => Abi::ScalarPair(s1.to_union(), s2.to_union()), diff --git a/tests/ui/generator/issue-113279.rs b/tests/ui/generator/issue-113279.rs new file mode 100644 index 0000000000000..f69f804b716d5 --- /dev/null +++ b/tests/ui/generator/issue-113279.rs @@ -0,0 +1,27 @@ +#![feature(generators)] + +// `foo` attempts to dereference `""`, which results in an error being reported. Later, the +// generator transform for `foo` then produces a union which contains a `str` type - unions should +// not contain unsized types, but this is okay because an error has been reported already. +// When const propagation happens later in compilation, it attempts to compute the layout of the +// generator (as part of checking whether something can be const propagated) and in turn attempts +// to compute the layout of `str` in the context of a union - where this caused an ICE. This test +// makes sure that doesn't happen again. + +fn foo() { + let _y = static || { + let x = &mut 0; + *{ + yield; + x + } += match { *"" }.len() { + //~^ ERROR cannot move a value of type `str` [E0161] + //~^^ ERROR cannot move out of a shared reference [E0507] + _ => 0, + }; + }; +} + +fn main() { + foo() +} diff --git a/tests/ui/generator/issue-113279.stderr b/tests/ui/generator/issue-113279.stderr new file mode 100644 index 0000000000000..cc9b64ef9ac2f --- /dev/null +++ b/tests/ui/generator/issue-113279.stderr @@ -0,0 +1,16 @@ +error[E0161]: cannot move a value of type `str` + --> $DIR/issue-113279.rs:17:20 + | +LL | } += match { *"" }.len() { + | ^^^^^^^ the size of `str` cannot be statically determined + +error[E0507]: cannot move out of a shared reference + --> $DIR/issue-113279.rs:17:22 + | +LL | } += match { *"" }.len() { + | ^^^ move occurs because value has type `str`, which does not implement the `Copy` trait + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0161, E0507. +For more information about an error, try `rustc --explain E0161`. From e0c479eea2382930bb550bbb7ddb98a841f8c54b Mon Sep 17 00:00:00 2001 From: yukang Date: Mon, 24 Jul 2023 13:09:52 +0800 Subject: [PATCH 10/10] Add help for crate arg when crate name is invalid --- compiler/rustc_session/messages.ftl | 1 + compiler/rustc_session/src/errors.rs | 8 ++++++++ compiler/rustc_session/src/output.rs | 13 +++++++++++-- src/tools/tidy/src/ui_tests.rs | 4 +++- tests/ui/command/need-crate-arg-ignore-tidy.x.rs | 2 ++ .../ui/command/need-crate-arg-ignore-tidy.x.stderr | 6 ++++++ 6 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 tests/ui/command/need-crate-arg-ignore-tidy.x.rs create mode 100644 tests/ui/command/need-crate-arg-ignore-tidy.x.stderr diff --git a/compiler/rustc_session/messages.ftl b/compiler/rustc_session/messages.ftl index ee24c6d902fa5..d4042a2e61ad6 100644 --- a/compiler/rustc_session/messages.ftl +++ b/compiler/rustc_session/messages.ftl @@ -45,6 +45,7 @@ session_int_literal_too_large = integer literal is too large .note = value exceeds limit of `{$limit}` session_invalid_character_in_create_name = invalid character `{$character}` in crate name: `{$crate_name}` +session_invalid_character_in_create_name_help = you can either pass `--crate-name` on the command line or add `#![crate_name="…"]` to set the crate name session_invalid_float_literal_suffix = invalid suffix `{$suffix}` for float literal .label = invalid suffix `{$suffix}` diff --git a/compiler/rustc_session/src/errors.rs b/compiler/rustc_session/src/errors.rs index dd15ad45145f1..1ffee01b2f1c6 100644 --- a/compiler/rustc_session/src/errors.rs +++ b/compiler/rustc_session/src/errors.rs @@ -199,6 +199,14 @@ pub struct InvalidCharacterInCrateName { pub span: Option, pub character: char, pub crate_name: Symbol, + #[subdiagnostic] + pub crate_name_help: Option, +} + +#[derive(Subdiagnostic)] +pub enum InvalidCrateNameHelp { + #[help(session_invalid_character_in_create_name_help)] + AddCrateName, } #[derive(Subdiagnostic)] diff --git a/compiler/rustc_session/src/output.rs b/compiler/rustc_session/src/output.rs index 2088744bc5bff..c0884fb21cd18 100644 --- a/compiler/rustc_session/src/output.rs +++ b/compiler/rustc_session/src/output.rs @@ -2,7 +2,7 @@ use crate::config::{CrateType, Input, OutFileName, OutputFilenames, OutputType}; use crate::errors::{ CrateNameDoesNotMatch, CrateNameEmpty, CrateNameInvalid, FileIsNotWriteable, - InvalidCharacterInCrateName, + InvalidCharacterInCrateName, InvalidCrateNameHelp, }; use crate::Session; use rustc_ast::{self as ast, attr}; @@ -101,7 +101,16 @@ pub fn validate_crate_name(sess: &Session, s: Symbol, sp: Option) { continue; } err_count += 1; - sess.emit_err(InvalidCharacterInCrateName { span: sp, character: c, crate_name: s }); + sess.emit_err(InvalidCharacterInCrateName { + span: sp, + character: c, + crate_name: s, + crate_name_help: if sp.is_none() { + Some(InvalidCrateNameHelp::AddCrateName) + } else { + None + }, + }); } } diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index c3a639528413b..18cee7d3d5c2c 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -116,7 +116,9 @@ pub fn check(path: &Path, bad: &mut bool) { // must strip all of them. let testname = file_path.file_name().unwrap().to_str().unwrap().split_once('.').unwrap().0; - if !file_path.with_file_name(testname).with_extension("rs").exists() { + if !file_path.with_file_name(testname).with_extension("rs").exists() + && !testname.contains("ignore-tidy") + { tidy_error!(bad, "Stray file with UI testing output: {:?}", file_path); } diff --git a/tests/ui/command/need-crate-arg-ignore-tidy.x.rs b/tests/ui/command/need-crate-arg-ignore-tidy.x.rs new file mode 100644 index 0000000000000..b1ac4a4ae2167 --- /dev/null +++ b/tests/ui/command/need-crate-arg-ignore-tidy.x.rs @@ -0,0 +1,2 @@ +// issue: 113981 +pub fn main() {} diff --git a/tests/ui/command/need-crate-arg-ignore-tidy.x.stderr b/tests/ui/command/need-crate-arg-ignore-tidy.x.stderr new file mode 100644 index 0000000000000..305f76694f71a --- /dev/null +++ b/tests/ui/command/need-crate-arg-ignore-tidy.x.stderr @@ -0,0 +1,6 @@ +error: invalid character `'.'` in crate name: `need_crate_arg_ignore_tidy.x` + | + = help: you can either pass `--crate-name` on the command line or add `#![crate_name="…"]` to set the crate name + +error: aborting due to previous error +