From f5d6eb30a845109ab6f69855b4720a15573e8fad Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 30 Jan 2024 22:03:16 +0300 Subject: [PATCH 01/16] hir: Stop keeping prefixes for most of `use` list stems And make sure all other imports have non-empty resolution lists. --- compiler/rustc_ast_lowering/src/item.rs | 19 +++++++++++++------ compiler/rustc_ast_lowering/src/lib.rs | 12 +++++++++--- compiler/rustc_ast_lowering/src/path.rs | 1 + 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index dd3f7289a60..60685119fb9 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -496,8 +496,7 @@ fn lower_use_tree( } } - let res = - self.expect_full_res_from_use(id).map(|res| self.lower_res(res)).collect(); + let res = self.lower_import_res(id, path.span); let path = self.lower_use_path(res, &path, ParamMode::Explicit); hir::ItemKind::Use(path, hir::UseKind::Single) } @@ -533,7 +532,8 @@ fn lower_use_tree( // for that we return the `{}` import (called the // `ListStem`). - let prefix = Path { segments, span: prefix.span.to(path.span), tokens: None }; + let span = prefix.span.to(path.span); + let prefix = Path { segments, span, tokens: None }; // Add all the nested `PathListItem`s to the HIR. for &(ref use_tree, id) in trees { @@ -567,9 +567,16 @@ fn lower_use_tree( }); } - let res = - self.expect_full_res_from_use(id).map(|res| self.lower_res(res)).collect(); - let path = self.lower_use_path(res, &prefix, ParamMode::Explicit); + let path = if trees.is_empty() && !prefix.segments.is_empty() { + // For empty lists we need to lower the prefix so it is checked for things + // like stability later. + let res = self.lower_import_res(id, span); + self.lower_use_path(res, &prefix, ParamMode::Explicit) + } else { + // For non-empty lists we can just drop all the data, the prefix is already + // present in HIR as a part of nested imports. + self.arena.alloc(hir::UsePath { res: smallvec![], segments: &[], span }) + }; hir::ItemKind::Use(path, hir::UseKind::ListStem) } } diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 3621844efc8..14a83c7eda6 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -63,7 +63,7 @@ use rustc_session::parse::{add_feature_diagnostics, feature_err}; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{DesugaringKind, Span, DUMMY_SP}; -use smallvec::SmallVec; +use smallvec::{smallvec, SmallVec}; use std::collections::hash_map::Entry; use thin_vec::ThinVec; @@ -749,8 +749,14 @@ fn expect_full_res(&mut self, id: NodeId) -> Res { self.resolver.get_partial_res(id).map_or(Res::Err, |pr| pr.expect_full_res()) } - fn expect_full_res_from_use(&mut self, id: NodeId) -> impl Iterator> { - self.resolver.get_import_res(id).present_items() + fn lower_import_res(&mut self, id: NodeId, span: Span) -> SmallVec<[Res; 3]> { + let res = self.resolver.get_import_res(id).present_items(); + let res: SmallVec<_> = res.map(|res| self.lower_res(res)).collect(); + if res.is_empty() { + self.dcx().span_delayed_bug(span, "no resolution for an import"); + return smallvec![Res::Err]; + } + res } fn make_lang_item_qpath(&mut self, lang_item: hir::LangItem, span: Span) -> hir::QPath<'hir> { diff --git a/compiler/rustc_ast_lowering/src/path.rs b/compiler/rustc_ast_lowering/src/path.rs index c679ee56fcd..4df68f8e929 100644 --- a/compiler/rustc_ast_lowering/src/path.rs +++ b/compiler/rustc_ast_lowering/src/path.rs @@ -156,6 +156,7 @@ pub(crate) fn lower_use_path( p: &Path, param_mode: ParamMode, ) -> &'hir hir::UsePath<'hir> { + assert!((1..=3).contains(&res.len())); self.arena.alloc(hir::UsePath { res, segments: self.arena.alloc_from_iter(p.segments.iter().map(|segment| { From 17f0919e8acba48224558c7ebcfc06f14d30d235 Mon Sep 17 00:00:00 2001 From: klensy Date: Sat, 3 Feb 2024 12:33:00 +0300 Subject: [PATCH 02/16] rustc_monomorphize: fix outdated comment in partition --- compiler/rustc_monomorphize/src/partitioning.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_monomorphize/src/partitioning.rs b/compiler/rustc_monomorphize/src/partitioning.rs index 7ff182381b8..032686101d9 100644 --- a/compiler/rustc_monomorphize/src/partitioning.rs +++ b/compiler/rustc_monomorphize/src/partitioning.rs @@ -156,7 +156,7 @@ fn partition<'tcx, I>( placed }; - // Merge until we have at most `max_cgu_count` codegen units. + // Merge until we don't exceed the max CGU count. // `merge_codegen_units` is responsible for updating the CGU size // estimates. { From 934618fe472727cf5e45535c945516c2c62dd75e Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sat, 3 Feb 2024 19:57:47 -0500 Subject: [PATCH 03/16] Emit a diagnostic for invalid target options --- compiler/rustc_codegen_llvm/messages.ftl | 2 ++ compiler/rustc_codegen_llvm/src/errors.rs | 6 ++++++ compiler/rustc_codegen_llvm/src/llvm_util.rs | 16 ++++++++-------- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_codegen_llvm/messages.ftl b/compiler/rustc_codegen_llvm/messages.ftl index d5bc04f594d..d14fe0299e6 100644 --- a/compiler/rustc_codegen_llvm/messages.ftl +++ b/compiler/rustc_codegen_llvm/messages.ftl @@ -28,6 +28,8 @@ codegen_llvm_invalid_minimum_alignment_not_power_of_two = codegen_llvm_invalid_minimum_alignment_too_large = invalid minimum global alignment: {$align} is too large +codegen_llvm_invalid_target_feature_prefix = target feature `{$feature}` must begin with a `+` or `-`" + codegen_llvm_load_bitcode = failed to load bitcode of module "{$name}" codegen_llvm_load_bitcode_with_llvm_err = failed to load bitcode of module "{$name}": {$llvm_err} diff --git a/compiler/rustc_codegen_llvm/src/errors.rs b/compiler/rustc_codegen_llvm/src/errors.rs index d82ff6656f4..43250d73b48 100644 --- a/compiler/rustc_codegen_llvm/src/errors.rs +++ b/compiler/rustc_codegen_llvm/src/errors.rs @@ -253,3 +253,9 @@ pub struct MismatchedDataLayout<'a> { pub llvm_target: &'a str, pub llvm_layout: &'a str, } + +#[derive(Diagnostic)] +#[diag(codegen_llvm_invalid_target_feature_prefix)] +pub(crate) struct InvalidTargetFeaturePrefix<'a> { + pub feature: &'a str, +} diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index 99f4488ac0f..4bb400b1879 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -1,7 +1,7 @@ use crate::back::write::create_informational_target_machine; use crate::errors::{ - PossibleFeature, TargetFeatureDisableOrEnable, UnknownCTargetFeature, - UnknownCTargetFeaturePrefix, UnstableCTargetFeature, + InvalidTargetFeaturePrefix, PossibleFeature, TargetFeatureDisableOrEnable, + UnknownCTargetFeature, UnknownCTargetFeaturePrefix, UnstableCTargetFeature, }; use crate::llvm; use libc::c_int; @@ -511,7 +511,7 @@ pub(crate) fn global_llvm_features(sess: &Session, diagnostics: bool) -> Vec Vec Vec Option<&str> { +fn backend_feature_name<'a>(sess: &Session, s: &'a str) -> Option<&'a str> { // features must start with a `+` or `-`. - let feature = s.strip_prefix(&['+', '-'][..]).unwrap_or_else(|| { - bug!("target feature `{}` must begin with a `+` or `-`", s); - }); + let feature = s + .strip_prefix(&['+', '-'][..]) + .unwrap_or_else(|| sess.dcx().emit_fatal(InvalidTargetFeaturePrefix { feature: s })); // Rustc-specific feature requests like `+crt-static` or `-crt-static` // are not passed down to LLVM. if RUSTC_SPECIFIC_FEATURES.contains(&feature) { From a4cb55f2aca24f3cbf33b025c147d7a894bceefd Mon Sep 17 00:00:00 2001 From: trevyn <230691+trevyn@users.noreply.github.com> Date: Sat, 3 Feb 2024 10:37:28 -0800 Subject: [PATCH 04/16] For E0223, suggest methods that look similar to the path --- .../rustc_hir_analysis/src/astconv/mod.rs | 52 ++++++++++++++ tests/ui/suggestions/issue-109195.rs | 20 ++++++ tests/ui/suggestions/issue-109195.stderr | 69 +++++++++++++++++++ 3 files changed, 141 insertions(+) create mode 100644 tests/ui/suggestions/issue-109195.rs create mode 100644 tests/ui/suggestions/issue-109195.stderr diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index 89f39897ea8..d69f366a66c 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -29,6 +29,7 @@ use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; use rustc_infer::traits::ObligationCause; use rustc_middle::middle::stability::AllowUnstable; +use rustc_middle::query::Key; use rustc_middle::ty::{ self, Const, GenericArgKind, GenericArgsRef, GenericParamDefKind, IsSuggestable, ParamEnv, Ty, TyCtxt, TypeVisitableExt, @@ -1373,6 +1374,57 @@ pub fn associated_path_to_ty( ) .emit() // Already reported in an earlier stage. } else { + // suggest methods that look similar to the path + // e.g. for `String::from::utf8`, suggest `String::from_utf8` (#109195) + for (_, node) in tcx.hir().parent_iter(qself.hir_id) { + if let hir::Node::Expr(hir::Expr { + kind: + hir::ExprKind::Path(hir::QPath::TypeRelative( + hir::Ty { + kind: + hir::TyKind::Path(hir::QPath::TypeRelative( + _, + hir::PathSegment { ident: ident2, .. }, + )), + .. + }, + hir::PathSegment { ident: ident3, .. }, + )), + .. + }) = node + { + let name = format!("{ident2}_{ident3}"); + if if let Some(ty_def_id) = qself_ty.ty_def_id() + && let Ok([inherent_impl]) = tcx.inherent_impls(ty_def_id) + && let Some(ty::AssocItem { kind: ty::AssocKind::Fn, .. }) = tcx + .associated_items(inherent_impl) + .filter_by_name_unhygienic(Symbol::intern(&name)) + .next() + { + true + } else { + qself_ty.is_str() + && ["from_utf8", "from_utf8_mut"].contains(&name.as_str()) + } { + let reported = struct_span_code_err!( + tcx.dcx(), + span, + E0223, + "ambiguous associated type" + ) + .with_span_suggestion_verbose( + ident2.span.to(ident3.span), + format!("you might have meant to use `{name}`"), + name, + Applicability::MaybeIncorrect, + ) + .emit(); + self.set_tainted_by_errors(reported); + return Err(reported); + } + } + } + let traits: Vec<_> = self.probe_traits_that_match_assoc_ty(qself_ty, assoc_ident); diff --git a/tests/ui/suggestions/issue-109195.rs b/tests/ui/suggestions/issue-109195.rs new file mode 100644 index 00000000000..badb859dbb7 --- /dev/null +++ b/tests/ui/suggestions/issue-109195.rs @@ -0,0 +1,20 @@ +fn main() { + String::from::utf8; + //~^ ERROR ambiguous associated type [E0223] + //~| HELP you might have meant to use `from_utf8` + String::from::utf8(); + //~^ ERROR ambiguous associated type [E0223] + //~| HELP you might have meant to use `from_utf8` + String::from::utf16(); + //~^ ERROR ambiguous associated type [E0223] + //~| HELP you might have meant to use `from_utf16` + String::from::method_that_doesnt_exist(); + //~^ ERROR ambiguous associated type [E0223] + //~| HELP if there were a trait named `Example` with associated type `from` + str::from::utf8(); + //~^ ERROR ambiguous associated type [E0223] + //~| HELP you might have meant to use `from_utf8` + str::from::utf8_mut(); + //~^ ERROR ambiguous associated type [E0223] + //~| HELP you might have meant to use `from_utf8_mut` +} diff --git a/tests/ui/suggestions/issue-109195.stderr b/tests/ui/suggestions/issue-109195.stderr new file mode 100644 index 00000000000..5c9ef9f98f5 --- /dev/null +++ b/tests/ui/suggestions/issue-109195.stderr @@ -0,0 +1,69 @@ +error[E0223]: ambiguous associated type + --> $DIR/issue-109195.rs:2:5 + | +LL | String::from::utf8; + | ^^^^^^^^^^^^ + | +help: you might have meant to use `from_utf8` + | +LL | String::from_utf8; + | ~~~~~~~~~ + +error[E0223]: ambiguous associated type + --> $DIR/issue-109195.rs:5:5 + | +LL | String::from::utf8(); + | ^^^^^^^^^^^^ + | +help: you might have meant to use `from_utf8` + | +LL | String::from_utf8(); + | ~~~~~~~~~ + +error[E0223]: ambiguous associated type + --> $DIR/issue-109195.rs:8:5 + | +LL | String::from::utf16(); + | ^^^^^^^^^^^^ + | +help: you might have meant to use `from_utf16` + | +LL | String::from_utf16(); + | ~~~~~~~~~~ + +error[E0223]: ambiguous associated type + --> $DIR/issue-109195.rs:11:5 + | +LL | String::from::method_that_doesnt_exist(); + | ^^^^^^^^^^^^ + | +help: if there were a trait named `Example` with associated type `from` implemented for `String`, you could use the fully-qualified path + | +LL | ::from::method_that_doesnt_exist(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error[E0223]: ambiguous associated type + --> $DIR/issue-109195.rs:14:5 + | +LL | str::from::utf8(); + | ^^^^^^^^^ + | +help: you might have meant to use `from_utf8` + | +LL | str::from_utf8(); + | ~~~~~~~~~ + +error[E0223]: ambiguous associated type + --> $DIR/issue-109195.rs:17:5 + | +LL | str::from::utf8_mut(); + | ^^^^^^^^^ + | +help: you might have meant to use `from_utf8_mut` + | +LL | str::from_utf8_mut(); + | ~~~~~~~~~~~~~ + +error: aborting due to 6 previous errors + +For more information about this error, try `rustc --explain E0223`. From 285d8c225d05966040f6a69bfdea83165b54483f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sat, 3 Feb 2024 01:32:01 +0100 Subject: [PATCH 05/16] Suggest `[tail @ ..]` on `[..tail]` and `[...tail]` where `tail` is unresolved --- compiler/rustc_resolve/messages.ftl | 3 + compiler/rustc_resolve/src/errors.rs | 14 +++++ .../rustc_resolve/src/late/diagnostics.rs | 38 +++++++----- ...-pattern-meant-to-be-slice-rest-pattern.rs | 16 ++++- ...tern-meant-to-be-slice-rest-pattern.stderr | 61 +++++++++++++++++-- 5 files changed, 112 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_resolve/messages.ftl b/compiler/rustc_resolve/messages.ftl index 02847a0f5f9..769c469a5ab 100644 --- a/compiler/rustc_resolve/messages.ftl +++ b/compiler/rustc_resolve/messages.ftl @@ -292,6 +292,9 @@ resolve_underscore_lifetime_name_cannot_be_used_here = resolve_unexpected_res_change_ty_to_const_param_sugg = you might have meant to write a const parameter here +resolve_unexpected_res_use_at_op_in_slice_pat_with_range_sugg = + if you meant to collect the rest of the slice in `{$ident}`, use the at operator + resolve_unreachable_label = use of unreachable label `{$name}` .label = unreachable label `{$name}` diff --git a/compiler/rustc_resolve/src/errors.rs b/compiler/rustc_resolve/src/errors.rs index 655fc9812d7..49a0e597dbc 100644 --- a/compiler/rustc_resolve/src/errors.rs +++ b/compiler/rustc_resolve/src/errors.rs @@ -800,3 +800,17 @@ pub(crate) struct UnexpectedResChangeTyToConstParamSugg { #[applicability] pub applicability: Applicability, } + +#[derive(Subdiagnostic)] +#[suggestion( + resolve_unexpected_res_use_at_op_in_slice_pat_with_range_sugg, + code = "{snippet}", + applicability = "maybe-incorrect", + style = "verbose" +)] +pub(crate) struct UnexpectedResUseAtOpInSlicePatWithRangeSugg { + #[primary_span] + pub span: Span, + pub ident: Ident, + pub snippet: String, +} diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 310c126213a..5d712461993 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1077,24 +1077,34 @@ fn suggest_at_operator_in_slice_pat_with_range( err: &mut Diagnostic, path: &[Segment], ) { - if let Some(pat) = self.diagnostic_metadata.current_pat - && let ast::PatKind::Range(Some(start), None, range) = &pat.kind - && let ExprKind::Path(None, range_path) = &start.kind + let Some(pat) = self.diagnostic_metadata.current_pat else { return }; + let (bound, side, range) = match &pat.kind { + ast::PatKind::Range(Some(bound), None, range) => (bound, Side::Start, range), + ast::PatKind::Range(None, Some(bound), range) => (bound, Side::End, range), + _ => return, + }; + if let ExprKind::Path(None, range_path) = &bound.kind && let [segment] = &range_path.segments[..] && let [s] = path && segment.ident == s.ident + && segment.ident.span.eq_ctxt(range.span) { - // We've encountered `[first, rest..]` where the user might have meant - // `[first, rest @ ..]` (#88404). - err.span_suggestion_verbose( - segment.ident.span.between(range.span), - format!( - "if you meant to collect the rest of the slice in `{}`, use the at operator", - segment.ident, - ), - " @ ", - Applicability::MaybeIncorrect, - ); + // We've encountered `[first, rest..]` (#88404) or `[first, ..rest]` (#120591) + // where the user might have meant `[first, rest @ ..]`. + let (span, snippet) = match side { + Side::Start => (segment.ident.span.between(range.span), " @ ".into()), + Side::End => (range.span.to(segment.ident.span), format!("{} @ ..", segment.ident)), + }; + err.subdiagnostic(errors::UnexpectedResUseAtOpInSlicePatWithRangeSugg { + span, + ident: segment.ident, + snippet, + }); + } + + enum Side { + Start, + End, } } diff --git a/tests/ui/pattern/range-pattern-meant-to-be-slice-rest-pattern.rs b/tests/ui/pattern/range-pattern-meant-to-be-slice-rest-pattern.rs index a619fcafc86..1eba7aeb4d6 100644 --- a/tests/ui/pattern/range-pattern-meant-to-be-slice-rest-pattern.rs +++ b/tests/ui/pattern/range-pattern-meant-to-be-slice-rest-pattern.rs @@ -1,9 +1,23 @@ fn main() { match &[1, 2, 3][..] { - [1, rest..] => println!("{rest:?}"), + [1, rest..] => println!("{rest}"), //~^ ERROR cannot find value `rest` in this scope //~| ERROR cannot find value `rest` in this scope //~| ERROR `X..` patterns in slices are experimental _ => {} } + match &[4, 5, 6][..] { + [] => {} + [_, ..tail] => println!("{tail}"), + //~^ ERROR cannot find value `tail` in this scope + //~| ERROR cannot find value `tail` in this scope + //~| ERROR exclusive range pattern syntax is experimental + } + match &[7, 8, 9][..] { + [] => {} + [_, ...tail] => println!("{tail}"), + //~^ ERROR cannot find value `tail` in this scope + //~| ERROR cannot find value `tail` in this scope + //~| ERROR range-to patterns with `...` are not allowed + } } diff --git a/tests/ui/pattern/range-pattern-meant-to-be-slice-rest-pattern.stderr b/tests/ui/pattern/range-pattern-meant-to-be-slice-rest-pattern.stderr index c3c9131b63e..3a19517c85b 100644 --- a/tests/ui/pattern/range-pattern-meant-to-be-slice-rest-pattern.stderr +++ b/tests/ui/pattern/range-pattern-meant-to-be-slice-rest-pattern.stderr @@ -1,31 +1,82 @@ +error: range-to patterns with `...` are not allowed + --> $DIR/range-pattern-meant-to-be-slice-rest-pattern.rs:18:13 + | +LL | [_, ...tail] => println!("{tail}"), + | ^^^ help: use `..=` instead + error[E0425]: cannot find value `rest` in this scope --> $DIR/range-pattern-meant-to-be-slice-rest-pattern.rs:3:13 | -LL | [1, rest..] => println!("{rest:?}"), +LL | [1, rest..] => println!("{rest}"), | ^^^^ not found in this scope | help: if you meant to collect the rest of the slice in `rest`, use the at operator | -LL | [1, rest @ ..] => println!("{rest:?}"), +LL | [1, rest @ ..] => println!("{rest}"), | + error[E0425]: cannot find value `rest` in this scope --> $DIR/range-pattern-meant-to-be-slice-rest-pattern.rs:3:35 | -LL | [1, rest..] => println!("{rest:?}"), +LL | [1, rest..] => println!("{rest}"), | ^^^^ not found in this scope +error[E0425]: cannot find value `tail` in this scope + --> $DIR/range-pattern-meant-to-be-slice-rest-pattern.rs:11:15 + | +LL | [_, ..tail] => println!("{tail}"), + | ^^^^ not found in this scope + | +help: if you meant to collect the rest of the slice in `tail`, use the at operator + | +LL | [_, tail @ ..] => println!("{tail}"), + | ~~~~~~~~~ + +error[E0425]: cannot find value `tail` in this scope + --> $DIR/range-pattern-meant-to-be-slice-rest-pattern.rs:11:35 + | +LL | [_, ..tail] => println!("{tail}"), + | ^^^^ not found in this scope + +error[E0425]: cannot find value `tail` in this scope + --> $DIR/range-pattern-meant-to-be-slice-rest-pattern.rs:18:16 + | +LL | [_, ...tail] => println!("{tail}"), + | ^^^^ not found in this scope + | +help: if you meant to collect the rest of the slice in `tail`, use the at operator + | +LL | [_, tail @ ..] => println!("{tail}"), + | ~~~~~~~~~ + +error[E0425]: cannot find value `tail` in this scope + --> $DIR/range-pattern-meant-to-be-slice-rest-pattern.rs:18:36 + | +LL | [_, ...tail] => println!("{tail}"), + | ^^^^ not found in this scope + error[E0658]: `X..` patterns in slices are experimental --> $DIR/range-pattern-meant-to-be-slice-rest-pattern.rs:3:13 | -LL | [1, rest..] => println!("{rest:?}"), +LL | [1, rest..] => println!("{rest}"), | ^^^^^^ | = note: see issue #67264 for more information = help: add `#![feature(half_open_range_patterns_in_slices)]` to the crate attributes to enable = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date -error: aborting due to 3 previous errors +error[E0658]: exclusive range pattern syntax is experimental + --> $DIR/range-pattern-meant-to-be-slice-rest-pattern.rs:11:13 + | +LL | [_, ..tail] => println!("{tail}"), + | ^^^^^^ + | + = note: see issue #37854 for more information + = help: add `#![feature(exclusive_range_pattern)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + = help: use an inclusive range pattern, like N..=M + +error: aborting due to 9 previous errors Some errors have detailed explanations: E0425, E0658. For more information about an error, try `rustc --explain E0425`. From e8c3cbf44b9f003482871e8638859c8f65b234bb Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 31 Jan 2024 09:25:42 +1100 Subject: [PATCH 06/16] Make `Diagnostic::is_error` return false for `Level::FailureNote`. It doesn't affect behaviour, but makes sense with (a) `FailureNote` having `()` as its emission guarantee, and (b) in `Level` the `is_error` levels now are all listed before the non-`is_error` levels. --- compiler/rustc_errors/src/diagnostic.rs | 4 ++-- compiler/rustc_errors/src/lib.rs | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 8ad4925cff2..09570fe74b6 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -238,8 +238,7 @@ pub fn is_error(&self) -> bool { Level::Bug | Level::DelayedBug(DelayedBugKind::Normal) | Level::Fatal - | Level::Error - | Level::FailureNote => true, + | Level::Error => true, Level::ForceWarning(_) | Level::Warning @@ -248,6 +247,7 @@ pub fn is_error(&self) -> bool { | Level::OnceNote | Level::Help | Level::OnceHelp + | Level::FailureNote | Level::Allow | Level::Expect(_) => false, } diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index b2bd4d8eb95..42b1c85e939 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1541,6 +1541,8 @@ pub enum Level { /// /// The [`LintExpectationId`] is used for expected lint diagnostics. In all other cases this /// should be `None`. + /// + /// Its `EmissionGuarantee` is `()`. ForceWarning(Option), /// A warning about the code being compiled. Does not prevent compilation from finishing. @@ -1570,7 +1572,8 @@ pub enum Level { /// Its `EmissionGuarantee` is `()`. OnceHelp, - /// Similar to `Note`, but used in cases where compilation has failed. Rare. + /// Similar to `Note`, but used in cases where compilation has failed. When printed for human + /// consumption, it doesn't have any kind of `note:` label. Rare. /// /// Its `EmissionGuarantee` is `()`. FailureNote, From 5dd0431386867a051f726cbba6e0f46f7bba5e5d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 31 Jan 2024 13:56:22 +1100 Subject: [PATCH 07/16] Tighten the assertion in `downgrade_to_delayed_bug`. I.e. `Bug` and `Fatal` level diagnostics are never downgraded. --- compiler/rustc_errors/src/diagnostic.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 09570fe74b6..299e4a840f7 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -306,7 +306,7 @@ pub(crate) fn is_force_warn(&self) -> bool { #[track_caller] pub fn downgrade_to_delayed_bug(&mut self) { assert!( - self.is_error(), + matches!(self.level, Level::Error | Level::DelayedBug(_)), "downgrade_to_delayed_bug: cannot downgrade {:?} to DelayedBug: not an error", self.level ); From c3673868325f95203d5291f2fa3a399425c14876 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 31 Jan 2024 15:10:23 +1100 Subject: [PATCH 08/16] Refactor `emit_diagnostic`. - Combine two different blocks involving `diagnostic.level.get_expectation_id()` into one. - Combine several `if`s involving `diagnostic.level` into a single `match`. This requires reordering some of the operations, but this has no functional effect. --- compiler/rustc_errors/src/lib.rs | 84 +++++++++++++++----------------- 1 file changed, 40 insertions(+), 44 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 42b1c85e939..b3461b676be 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1247,24 +1247,41 @@ fn emit_stashed_diagnostics(&mut self) -> Option { } fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option { - // The `LintExpectationId` can be stable or unstable depending on when it was created. - // Diagnostics created before the definition of `HirId`s are unstable and can not yet - // be stored. Instead, they are buffered until the `LintExpectationId` is replaced by - // a stable one by the `LintLevelsBuilder`. - if let Some(LintExpectationId::Unstable { .. }) = diagnostic.level.get_expectation_id() { - self.unstable_expect_diagnostics.push(diagnostic); - return None; + if let Some(expectation_id) = diagnostic.level.get_expectation_id() { + // The `LintExpectationId` can be stable or unstable depending on when it was created. + // Diagnostics created before the definition of `HirId`s are unstable and can not yet + // be stored. Instead, they are buffered until the `LintExpectationId` is replaced by + // a stable one by the `LintLevelsBuilder`. + if let LintExpectationId::Unstable { .. } = expectation_id { + self.unstable_expect_diagnostics.push(diagnostic); + return None; + } + self.suppressed_expected_diag = true; + self.fulfilled_expectations.insert(expectation_id.normalize()); + } + + if diagnostic.has_future_breakage() { + // Future breakages aren't emitted if they're Level::Allow, + // but they still need to be constructed and stashed below, + // so they'll trigger the good-path bug check. + self.suppressed_expected_diag = true; + self.future_breakage_diagnostics.push(diagnostic.clone()); + } + + if matches!(diagnostic.level, DelayedBug(_)) && self.flags.eagerly_emit_delayed_bugs { + diagnostic.level = Error; } - // FIXME(eddyb) this should check for `has_errors` and stop pushing - // once *any* errors were emitted (and truncate `span_delayed_bugs` - // when an error is first emitted, also), but maybe there's a case - // in which that's not sound? otherwise this is really inefficient. match diagnostic.level { - DelayedBug(_) if self.flags.eagerly_emit_delayed_bugs => { - diagnostic.level = Error; + // This must come after the possible promotion of `DelayedBug` to `Error` above. + Fatal | Error if self.treat_next_err_as_bug() => { + diagnostic.level = Bug; } DelayedBug(DelayedBugKind::Normal) => { + // FIXME(eddyb) this should check for `has_errors` and stop pushing + // once *any* errors were emitted (and truncate `span_delayed_bugs` + // when an error is first emitted, also), but maybe there's a case + // in which that's not sound? otherwise this is really inefficient. let backtrace = std::backtrace::Backtrace::capture(); self.span_delayed_bugs .push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); @@ -1277,38 +1294,17 @@ fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option {} - } - - // This must come after the possible promotion of `DelayedBug` to - // `Error` above. - if matches!(diagnostic.level, Error | Fatal) && self.treat_next_err_as_bug() { - diagnostic.level = Bug; - } - - if diagnostic.has_future_breakage() { - // Future breakages aren't emitted if they're Level::Allow, - // but they still need to be constructed and stashed below, - // so they'll trigger the good-path bug check. - self.suppressed_expected_diag = true; - self.future_breakage_diagnostics.push(diagnostic.clone()); - } - - if let Some(expectation_id) = diagnostic.level.get_expectation_id() { - self.suppressed_expected_diag = true; - self.fulfilled_expectations.insert(expectation_id.normalize()); - } - - if diagnostic.level == Warning && !self.flags.can_emit_warnings { - if diagnostic.has_future_breakage() { - (*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {}); + Warning if !self.flags.can_emit_warnings => { + if diagnostic.has_future_breakage() { + (*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {}); + } + return None; } - return None; - } - - if matches!(diagnostic.level, Expect(_) | Allow) { - (*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {}); - return None; + Allow | Expect(_) => { + (*TRACK_DIAGNOSTIC)(diagnostic, &mut |_| {}); + return None; + } + _ => {} } let mut guaranteed = None; From 59e0bc2de7134a2d88e9c14db32884e631e90373 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 31 Jan 2024 11:23:54 +1100 Subject: [PATCH 09/16] Split `Level::DelayedBug` in two. The two kinds of delayed bug have quite different semantics so a stronger conceptual separation is nice. (`is_error` is a good example, because the two kinds have different behaviour.) The commit also moves the `DelayedBug` variant after `Error` in `Level`, to reflect the fact that it's weaker than `Error` -- it might trigger an error but also might not. (The pre-existing `downgrade_to_delayed_bug` function also reflects the notion that delayed bugs are lower/after normal errors.) Plus it condenses some of the comments on `Level` into a table, for easier reading, and introduces `can_be_top_or_sub` to indicate which levels can be used in top-level diagnostics vs. subdiagnostics. Finally, it renames `DiagCtxtInner::span_delayed_bugs` as `DiagCtxtInner::delayed_bugs`. The `span_` prefix is unnecessary because some delayed bugs don't have a span. --- compiler/rustc_error_messages/src/lib.rs | 2 +- .../src/annotate_snippet_emitter_writer.rs | 6 +- compiler/rustc_errors/src/diagnostic.rs | 18 +- compiler/rustc_errors/src/emitter.rs | 1 + compiler/rustc_errors/src/lib.rs | 161 ++++++++++-------- .../equality-in-canonical-query.clone.stderr | 2 +- ...equality_in_canonical_query.current.stderr | 2 +- 7 files changed, 102 insertions(+), 90 deletions(-) diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs index 8fd7c576479..d212e18b4cd 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -378,7 +378,7 @@ fn from(s: Cow<'static, str>) -> Self { } } -/// A workaround for "good path" ICEs when formatting types in disabled lints. +/// A workaround for good_path_delayed_bug ICEs when formatting types in disabled lints. /// /// Delays formatting until `.into(): DiagnosticMessage` is used. pub struct DelayDm(pub F); diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index 949f52ef6b5..06f6c58c5ff 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -85,7 +85,11 @@ fn source_string(file: Lrc, line: &Line) -> String { /// Maps `Diagnostic::Level` to `snippet::AnnotationType` fn annotation_type_for_level(level: Level) -> AnnotationType { match level { - Level::Bug | Level::DelayedBug(_) | Level::Fatal | Level::Error => AnnotationType::Error, + Level::Bug + | Level::Fatal + | Level::Error + | Level::DelayedBug + | Level::GoodPathDelayedBug => AnnotationType::Error, Level::ForceWarning(_) | Level::Warning => AnnotationType::Warning, Level::Note | Level::OnceNote => AnnotationType::Note, Level::Help | Level::OnceHelp => AnnotationType::Help, diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 299e4a840f7..1763c355069 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -1,8 +1,7 @@ use crate::snippet::Style; use crate::{ - CodeSuggestion, DelayedBugKind, DiagnosticBuilder, DiagnosticMessage, EmissionGuarantee, - ErrCode, Level, MultiSpan, SubdiagnosticMessage, Substitution, SubstitutionPart, - SuggestionStyle, + CodeSuggestion, DiagnosticBuilder, DiagnosticMessage, EmissionGuarantee, ErrCode, Level, + MultiSpan, SubdiagnosticMessage, Substitution, SubstitutionPart, SuggestionStyle, }; use rustc_data_structures::fx::{FxHashMap, FxIndexMap}; use rustc_error_messages::fluent_value_from_str_list_sep_by_and; @@ -235,14 +234,11 @@ pub fn level(&self) -> Level { pub fn is_error(&self) -> bool { match self.level { - Level::Bug - | Level::DelayedBug(DelayedBugKind::Normal) - | Level::Fatal - | Level::Error => true, + Level::Bug | Level::Fatal | Level::Error | Level::DelayedBug => true, - Level::ForceWarning(_) + Level::GoodPathDelayedBug + | Level::ForceWarning(_) | Level::Warning - | Level::DelayedBug(DelayedBugKind::GoodPath) | Level::Note | Level::OnceNote | Level::Help @@ -306,11 +302,11 @@ pub(crate) fn is_force_warn(&self) -> bool { #[track_caller] pub fn downgrade_to_delayed_bug(&mut self) { assert!( - matches!(self.level, Level::Error | Level::DelayedBug(_)), + matches!(self.level, Level::Error | Level::DelayedBug), "downgrade_to_delayed_bug: cannot downgrade {:?} to DelayedBug: not an error", self.level ); - self.level = Level::DelayedBug(DelayedBugKind::Normal); + self.level = Level::DelayedBug; } /// Appends a labeled span to the diagnostic. diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 4be5ed923e5..6370e1d387c 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -2116,6 +2116,7 @@ fn emit_messages_default( } if !self.short_message { for child in children { + assert!(child.level.can_be_top_or_sub().1); let span = &child.span; if let Err(err) = self.emit_messages_default_inner( span, diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index b3461b676be..cfb2dfbeb98 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -439,7 +439,7 @@ struct DiagCtxtInner { has_printed: bool, emitter: Box, - span_delayed_bugs: Vec, + delayed_bugs: Vec, good_path_delayed_bugs: Vec, /// This flag indicates that an expected diagnostic was emitted and suppressed. /// This is used for the `good_path_delayed_bugs` check. @@ -523,8 +523,7 @@ fn default_track_diagnostic(diag: Diagnostic, f: &mut dyn FnMut(Diagnostic)) { pub static TRACK_DIAGNOSTIC: AtomicRef = AtomicRef::new(&(default_track_diagnostic as _)); -#[derive(Copy, PartialEq, Eq, Clone, Hash, Debug, Encodable, Decodable)] -pub enum DelayedBugKind { +enum DelayedBugKind { Normal, GoodPath, } @@ -557,11 +556,6 @@ fn drop(&mut self) { self.flush_delayed(DelayedBugKind::Normal) } - // FIXME(eddyb) this explains what `good_path_delayed_bugs` are! - // They're `span_delayed_bugs` but for "require some diagnostic happened" - // instead of "require some error happened". Sadly that isn't ideal, as - // lints can be `#[allow]`'d, potentially leading to this triggering. - // Also, "good path" should be replaced with a better naming. if !self.has_printed && !self.suppressed_expected_diag && !std::thread::panicking() { self.flush_delayed(DelayedBugKind::GoodPath); } @@ -608,7 +602,7 @@ pub fn with_emitter(emitter: Box) -> Self { deduplicated_warn_count: 0, has_printed: false, emitter, - span_delayed_bugs: Vec::new(), + delayed_bugs: Vec::new(), good_path_delayed_bugs: Vec::new(), suppressed_expected_diag: false, taught_diagnostics: Default::default(), @@ -664,7 +658,7 @@ pub fn reset_err_count(&self) { inner.has_printed = false; // actually free the underlying memory (which `clear` would not do) - inner.span_delayed_bugs = Default::default(); + inner.delayed_bugs = Default::default(); inner.good_path_delayed_bugs = Default::default(); inner.taught_diagnostics = Default::default(); inner.emitted_diagnostic_codes = Default::default(); @@ -865,8 +859,7 @@ pub fn span_bug(&self, span: impl Into, msg: impl Into) -> ErrorGuaranteed { - DiagnosticBuilder::::new(self, DelayedBug(DelayedBugKind::Normal), msg) - .emit() + DiagnosticBuilder::::new(self, DelayedBug, msg).emit() } /// Like `delayed_bug`, but takes an additional span. @@ -879,15 +872,12 @@ pub fn span_delayed_bug( sp: impl Into, msg: impl Into, ) -> ErrorGuaranteed { - DiagnosticBuilder::::new(self, DelayedBug(DelayedBugKind::Normal), msg) - .with_span(sp) - .emit() + DiagnosticBuilder::::new(self, DelayedBug, msg).with_span(sp).emit() } - // FIXME(eddyb) note the comment inside `impl Drop for DiagCtxtInner`, that's - // where the explanation of what "good path" is (also, it should be renamed). + /// Ensures that a diagnostic is printed. See `Level::GoodPathDelayedBug`. pub fn good_path_delayed_bug(&self, msg: impl Into) { - DiagnosticBuilder::<()>::new(self, DelayedBug(DelayedBugKind::GoodPath), msg).emit() + DiagnosticBuilder::<()>::new(self, GoodPathDelayedBug, msg).emit() } #[track_caller] @@ -961,7 +951,7 @@ pub fn has_errors_or_lint_errors(&self) -> Option { pub fn has_errors_or_lint_errors_or_delayed_bugs(&self) -> Option { let inner = self.inner.borrow(); let result = - inner.has_errors() || inner.lint_err_count > 0 || !inner.span_delayed_bugs.is_empty(); + inner.has_errors() || inner.lint_err_count > 0 || !inner.delayed_bugs.is_empty(); result.then(|| { #[allow(deprecated)] ErrorGuaranteed::unchecked_claim_error_was_emitted() @@ -1247,6 +1237,8 @@ fn emit_stashed_diagnostics(&mut self) -> Option { } fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option { + assert!(diagnostic.level.can_be_top_or_sub().0); + if let Some(expectation_id) = diagnostic.level.get_expectation_id() { // The `LintExpectationId` can be stable or unstable depending on when it was created. // Diagnostics created before the definition of `HirId`s are unstable and can not yet @@ -1268,27 +1260,29 @@ fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option { diagnostic.level = Bug; } - DelayedBug(DelayedBugKind::Normal) => { + DelayedBug => { // FIXME(eddyb) this should check for `has_errors` and stop pushing - // once *any* errors were emitted (and truncate `span_delayed_bugs` + // once *any* errors were emitted (and truncate `delayed_bugs` // when an error is first emitted, also), but maybe there's a case // in which that's not sound? otherwise this is really inefficient. let backtrace = std::backtrace::Backtrace::capture(); - self.span_delayed_bugs - .push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); + self.delayed_bugs.push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); #[allow(deprecated)] return Some(ErrorGuaranteed::unchecked_claim_error_was_emitted()); } - DelayedBug(DelayedBugKind::GoodPath) => { + GoodPathDelayedBug => { let backtrace = std::backtrace::Backtrace::capture(); self.good_path_delayed_bugs .push(DelayedDiagnostic::with_backtrace(diagnostic, backtrace)); @@ -1392,12 +1386,12 @@ fn failure_note(&mut self, msg: impl Into) { fn flush_delayed(&mut self, kind: DelayedBugKind) { let (bugs, note1) = match kind { DelayedBugKind::Normal => ( - std::mem::take(&mut self.span_delayed_bugs), - "no errors encountered even though `span_delayed_bug` issued", + std::mem::take(&mut self.delayed_bugs), + "no errors encountered even though delayed bugs were created", ), DelayedBugKind::GoodPath => ( std::mem::take(&mut self.good_path_delayed_bugs), - "no warnings or errors encountered even though `good_path_delayed_bugs` issued", + "no warnings or errors encountered even though good path delayed bugs were created", ), }; let note2 = "those delayed bugs will now be shown as internal compiler errors"; @@ -1436,8 +1430,8 @@ fn flush_delayed(&mut self, kind: DelayedBugKind) { let mut bug = if backtrace || self.ice_file.is_none() { bug.decorate() } else { bug.inner }; - // "Undelay" the `DelayedBug`s (into plain `Bug`s). - if !matches!(bug.level, DelayedBug(_)) { + // "Undelay" the delayed bugs (into plain `Bug`s). + if !matches!(bug.level, DelayedBug | GoodPathDelayedBug) { // NOTE(eddyb) not panicking here because we're already producing // an ICE, and the more information the merrier. bug.subdiagnostic(InvalidFlushedDelayedDiagnosticLevel { @@ -1503,85 +1497,89 @@ fn decorate(mut self) -> Diagnostic { } } +/// Level is_error EmissionGuarantee Top-level Sub Used in lints? +/// ----- -------- ----------------- --------- --- -------------- +/// Bug yes BugAbort yes - - +/// Fatal yes FatalAbort/FatalError(*) yes - - +/// Error yes ErrorGuaranteed yes - yes +/// DelayedBug yes ErrorGuaranteed yes - - +/// GoodPathDelayedBug - () yes - - +/// ForceWarning - () yes - lint-only +/// Warning - () yes yes yes +/// Note - () rare yes - +/// OnceNote - () - yes lint-only +/// Help - () rare yes - +/// OnceHelp - () - yes lint-only +/// FailureNote - () rare - - +/// Allow - () yes - lint-only +/// Expect - () yes - lint-only +/// +/// (*) `FatalAbort` normally, `FatalError` in the non-aborting "almost fatal" case that is +/// occasionally used. +/// #[derive(Copy, PartialEq, Eq, Clone, Hash, Debug, Encodable, Decodable)] pub enum Level { /// For bugs in the compiler. Manifests as an ICE (internal compiler error) panic. - /// - /// Its `EmissionGuarantee` is `BugAbort`. Bug, + /// An error that causes an immediate abort. Used for things like configuration errors, + /// internal overflows, some file operation errors. + Fatal, + + /// An error in the code being compiled, which prevents compilation from finishing. This is the + /// most common case. + Error, + /// This is a strange one: lets you register an error without emitting it. If compilation ends /// without any other errors occurring, this will be emitted as a bug. Otherwise, it will be /// silently dropped. I.e. "expect other errors are emitted" semantics. Useful on code paths /// that should only be reached when compiling erroneous code. - /// - /// Its `EmissionGuarantee` is `ErrorGuaranteed` for `Normal` delayed bugs, and `()` for - /// `GoodPath` delayed bugs. - DelayedBug(DelayedBugKind), + DelayedBug, - /// An error that causes an immediate abort. Used for things like configuration errors, - /// internal overflows, some file operation errors. + /// Like `DelayedBug`, but weaker: lets you register an error without emitting it. If + /// compilation ends without any other diagnostics being emitted (and without an expected lint + /// being suppressed), this will be emitted as a bug. Otherwise, it will be silently dropped. + /// I.e. "expect other diagnostics are emitted (or suppressed)" semantics. Useful on code paths + /// that should only be reached when emitting diagnostics, e.g. for expensive one-time + /// diagnostic formatting operations. /// - /// Its `EmissionGuarantee` is `FatalAbort`, except in the non-aborting "almost fatal" case - /// that is occasionally used, where it is `FatalError`. - Fatal, - - /// An error in the code being compiled, which prevents compilation from finishing. This is the - /// most common case. - /// - /// Its `EmissionGuarantee` is `ErrorGuaranteed`. - Error, + /// FIXME(nnethercote) good path delayed bugs are semantically strange: if printed they produce + /// an ICE, but they don't satisfy `is_error` and they don't guarantee an error is emitted. + /// Plus there's the extra complication with expected (suppressed) lints. They have limited + /// use, and are used in very few places, and "good path" isn't a good name. It would be good + /// to remove them. + GoodPathDelayedBug, /// A `force-warn` lint warning about the code being compiled. Does not prevent compilation /// from finishing. /// /// The [`LintExpectationId`] is used for expected lint diagnostics. In all other cases this /// should be `None`. - /// - /// Its `EmissionGuarantee` is `()`. ForceWarning(Option), /// A warning about the code being compiled. Does not prevent compilation from finishing. - /// - /// Its `EmissionGuarantee` is `()`. Warning, - /// A message giving additional context. Rare, because notes are more commonly attached to other - /// diagnostics such as errors. - /// - /// Its `EmissionGuarantee` is `()`. + /// A message giving additional context. Note, - /// A note that is only emitted once. Rare, mostly used in circumstances relating to lints. - /// - /// Its `EmissionGuarantee` is `()`. + /// A note that is only emitted once. OnceNote, - /// A message suggesting how to fix something. Rare, because help messages are more commonly - /// attached to other diagnostics such as errors. - /// - /// Its `EmissionGuarantee` is `()`. + /// A message suggesting how to fix something. Help, - /// A help that is only emitted once. Rare. - /// - /// Its `EmissionGuarantee` is `()`. + /// A help that is only emitted once. OnceHelp, /// Similar to `Note`, but used in cases where compilation has failed. When printed for human - /// consumption, it doesn't have any kind of `note:` label. Rare. - /// - /// Its `EmissionGuarantee` is `()`. + /// consumption, it doesn't have any kind of `note:` label. FailureNote, /// Only used for lints. - /// - /// Its `EmissionGuarantee` is `()`. Allow, /// Only used for lints. - /// - /// Its `EmissionGuarantee` is `()`. Expect(LintExpectationId), } @@ -1595,7 +1593,7 @@ impl Level { fn color(self) -> ColorSpec { let mut spec = ColorSpec::new(); match self { - Bug | DelayedBug(_) | Fatal | Error => { + Bug | Fatal | Error | DelayedBug | GoodPathDelayedBug => { spec.set_fg(Some(Color::Red)).set_intense(true); } ForceWarning(_) | Warning => { @@ -1615,7 +1613,7 @@ fn color(self) -> ColorSpec { pub fn to_str(self) -> &'static str { match self { - Bug | DelayedBug(_) => "error: internal compiler error", + Bug | DelayedBug | GoodPathDelayedBug => "error: internal compiler error", Fatal | Error => "error", ForceWarning(_) | Warning => "warning", Note | OnceNote => "note", @@ -1635,6 +1633,19 @@ pub fn get_expectation_id(&self) -> Option { _ => None, } } + + // Can this level be used in a top-level diagnostic message and/or a + // subdiagnostic message? + fn can_be_top_or_sub(&self) -> (bool, bool) { + match self { + Bug | DelayedBug | Fatal | Error | GoodPathDelayedBug | ForceWarning(_) + | FailureNote | Allow | Expect(_) => (true, false), + + Warning | Note | Help => (true, true), + + OnceNote | OnceHelp => (false, true), + } + } } // FIXME(eddyb) this doesn't belong here AFAICT, should be moved to callsite. diff --git a/tests/ui/impl-trait/equality-in-canonical-query.clone.stderr b/tests/ui/impl-trait/equality-in-canonical-query.clone.stderr index 1011fc4163b..0e3cd2ff060 100644 --- a/tests/ui/impl-trait/equality-in-canonical-query.clone.stderr +++ b/tests/ui/impl-trait/equality-in-canonical-query.clone.stderr @@ -1,4 +1,4 @@ -note: no errors encountered even though `span_delayed_bug` issued +note: no errors encountered even though delayed bugs were created note: those delayed bugs will now be shown as internal compiler errors diff --git a/tests/ui/type-alias-impl-trait/rpit_tait_equality_in_canonical_query.current.stderr b/tests/ui/type-alias-impl-trait/rpit_tait_equality_in_canonical_query.current.stderr index d92bafce142..fd76526644b 100644 --- a/tests/ui/type-alias-impl-trait/rpit_tait_equality_in_canonical_query.current.stderr +++ b/tests/ui/type-alias-impl-trait/rpit_tait_equality_in_canonical_query.current.stderr @@ -1,4 +1,4 @@ -note: no errors encountered even though `span_delayed_bug` issued +note: no errors encountered even though delayed bugs were created note: those delayed bugs will now be shown as internal compiler errors From 9cd6c68033929d60441ad4286d1270eeafbb8620 Mon Sep 17 00:00:00 2001 From: lcnr Date: Mon, 5 Feb 2024 10:51:18 +0100 Subject: [PATCH 10/16] cleanup effect var handling --- .../src/infer/canonical/canonicalizer.rs | 2 +- .../rustc_infer/src/infer/canonical/mod.rs | 8 +++- compiler/rustc_infer/src/infer/freshen.rs | 9 +--- compiler/rustc_infer/src/infer/mod.rs | 15 ++++--- .../rustc_infer/src/infer/relate/combine.rs | 38 +++-------------- compiler/rustc_infer/src/infer/resolve.rs | 9 ++-- compiler/rustc_middle/src/infer/unify_key.rs | 42 ++++++++++--------- 7 files changed, 52 insertions(+), 71 deletions(-) diff --git a/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs b/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs index e4b37f05b77..30ca70326fa 100644 --- a/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs +++ b/compiler/rustc_infer/src/infer/canonical/canonicalizer.rs @@ -480,7 +480,7 @@ fn fold_const(&mut self, mut ct: ty::Const<'tcx>) -> ty::Const<'tcx> { } ty::ConstKind::Infer(InferConst::EffectVar(vid)) => { match self.infcx.unwrap().probe_effect_var(vid) { - Some(value) => return self.fold_const(value.as_const(self.tcx)), + Some(value) => return self.fold_const(value), None => { return self.canonicalize_const_var( CanonicalVarInfo { kind: CanonicalVarKind::Effect }, diff --git a/compiler/rustc_infer/src/infer/canonical/mod.rs b/compiler/rustc_infer/src/infer/canonical/mod.rs index 386fdb09ba5..1f68a5a9c61 100644 --- a/compiler/rustc_infer/src/infer/canonical/mod.rs +++ b/compiler/rustc_infer/src/infer/canonical/mod.rs @@ -24,6 +24,7 @@ use crate::infer::{ConstVariableOrigin, ConstVariableOriginKind}; use crate::infer::{InferCtxt, RegionVariableOrigin, TypeVariableOrigin, TypeVariableOriginKind}; use rustc_index::IndexVec; +use rustc_middle::infer::unify_key::EffectVarValue; use rustc_middle::ty::fold::TypeFoldable; use rustc_middle::ty::GenericArg; use rustc_middle::ty::{self, List, Ty, TyCtxt}; @@ -152,7 +153,12 @@ pub fn instantiate_canonical_var( ) .into(), CanonicalVarKind::Effect => { - let vid = self.inner.borrow_mut().effect_unification_table().new_key(None).vid; + let vid = self + .inner + .borrow_mut() + .effect_unification_table() + .new_key(EffectVarValue::Unknown) + .vid; ty::Const::new_infer(self.tcx, ty::InferConst::EffectVar(vid), self.tcx.types.bool) .into() } diff --git a/compiler/rustc_infer/src/infer/freshen.rs b/compiler/rustc_infer/src/infer/freshen.rs index d256994d8d1..2d5fa1b5c70 100644 --- a/compiler/rustc_infer/src/infer/freshen.rs +++ b/compiler/rustc_infer/src/infer/freshen.rs @@ -151,13 +151,8 @@ fn fold_const(&mut self, ct: ty::Const<'tcx>) -> ty::Const<'tcx> { self.freshen_const(opt_ct, ty::InferConst::Var(v), ty::InferConst::Fresh, ct.ty()) } ty::ConstKind::Infer(ty::InferConst::EffectVar(v)) => { - let opt_ct = self - .infcx - .inner - .borrow_mut() - .effect_unification_table() - .probe_value(v) - .map(|effect| effect.as_const(self.infcx.tcx)); + let opt_ct = + self.infcx.inner.borrow_mut().effect_unification_table().probe_value(v).known(); self.freshen_const( opt_ct, ty::InferConst::EffectVar(v), diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index 0a39fe007fd..13baecf6002 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -8,6 +8,7 @@ pub use relate::combine::ObligationEmittingRelation; use rustc_data_structures::captures::Captures; use rustc_data_structures::undo_log::UndoLogs; +use rustc_middle::infer::unify_key::EffectVarValue; use rustc_middle::infer::unify_key::{ConstVidKey, EffectVidKey}; use self::opaque_types::OpaqueTypeStorage; @@ -25,8 +26,8 @@ use rustc_errors::{DiagCtxt, DiagnosticBuilder, ErrorGuaranteed}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_middle::infer::canonical::{Canonical, CanonicalVarValues}; +use rustc_middle::infer::unify_key::ConstVariableValue; use rustc_middle::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKind, ToType}; -use rustc_middle::infer::unify_key::{ConstVariableValue, EffectVarValue}; use rustc_middle::mir::interpret::{ErrorHandled, EvalToValTreeResult}; use rustc_middle::mir::ConstraintCategory; use rustc_middle::traits::{select, DefiningAnchor}; @@ -818,7 +819,7 @@ pub fn unsolved_effects(&self) -> Vec> { (0..table.len()) .map(|i| ty::EffectVid::from_usize(i)) - .filter(|&vid| table.probe_value(vid).is_none()) + .filter(|&vid| table.probe_value(vid).is_unknown()) .map(|v| { ty::Const::new_infer(self.tcx, ty::InferConst::EffectVar(v), self.tcx.types.bool) }) @@ -1236,7 +1237,8 @@ pub fn var_for_def(&self, span: Span, param: &ty::GenericParamDef) -> GenericArg } pub fn var_for_effect(&self, param: &ty::GenericParamDef) -> GenericArg<'tcx> { - let effect_vid = self.inner.borrow_mut().effect_unification_table().new_key(None).vid; + let effect_vid = + self.inner.borrow_mut().effect_unification_table().new_key(EffectVarValue::Unknown).vid; let ty = self .tcx .type_of(param.def_id) @@ -1416,8 +1418,8 @@ pub fn probe_const_var(&self, vid: ty::ConstVid) -> Result, ty:: } } - pub fn probe_effect_var(&self, vid: EffectVid) -> Option> { - self.inner.borrow_mut().effect_unification_table().probe_value(vid) + pub fn probe_effect_var(&self, vid: EffectVid) -> Option> { + self.inner.borrow_mut().effect_unification_table().probe_value(vid).known() } /// Attempts to resolve all type/region/const variables in @@ -1893,7 +1895,8 @@ fn fold_const(&mut self, ct: ty::Const<'tcx>) -> ty::Const<'tcx> { .borrow_mut() .effect_unification_table() .probe_value(vid) - .map_or(ct, |val| val.as_const(self.infcx.tcx)), + .known() + .unwrap_or(ct), _ => ct, } } diff --git a/compiler/rustc_infer/src/infer/relate/combine.rs b/compiler/rustc_infer/src/infer/relate/combine.rs index 1c120646f1f..13c17ee1cd2 100644 --- a/compiler/rustc_infer/src/infer/relate/combine.rs +++ b/compiler/rustc_infer/src/infer/relate/combine.rs @@ -202,11 +202,7 @@ pub fn super_combine_consts( ty::ConstKind::Infer(InferConst::EffectVar(a_vid)), ty::ConstKind::Infer(InferConst::EffectVar(b_vid)), ) => { - self.inner - .borrow_mut() - .effect_unification_table() - .unify_var_var(a_vid, b_vid) - .map_err(|a| effect_unification_error(self.tcx, relation.a_is_expected(), a))?; + self.inner.borrow_mut().effect_unification_table().union(a_vid, b_vid); return Ok(a); } @@ -233,19 +229,11 @@ pub fn super_combine_consts( } (ty::ConstKind::Infer(InferConst::EffectVar(vid)), _) => { - return self.unify_effect_variable( - relation.a_is_expected(), - vid, - EffectVarValue::Const(b), - ); + return Ok(self.unify_effect_variable(vid, b)); } (_, ty::ConstKind::Infer(InferConst::EffectVar(vid))) => { - return self.unify_effect_variable( - !relation.a_is_expected(), - vid, - EffectVarValue::Const(a), - ); + return Ok(self.unify_effect_variable(vid, a)); } (ty::ConstKind::Unevaluated(..), _) | (_, ty::ConstKind::Unevaluated(..)) @@ -366,18 +354,12 @@ fn unify_float_variable( Ok(Ty::new_float(self.tcx, val)) } - fn unify_effect_variable( - &self, - vid_is_expected: bool, - vid: ty::EffectVid, - val: EffectVarValue<'tcx>, - ) -> RelateResult<'tcx, ty::Const<'tcx>> { + fn unify_effect_variable(&self, vid: ty::EffectVid, val: ty::Const<'tcx>) -> ty::Const<'tcx> { self.inner .borrow_mut() .effect_unification_table() - .unify_var_value(vid, Some(val)) - .map_err(|e| effect_unification_error(self.tcx, vid_is_expected, e))?; - Ok(val.as_const(self.tcx)) + .union_value(vid, EffectVarValue::Known(val)); + val } } @@ -579,11 +561,3 @@ fn float_unification_error<'tcx>( let (ty::FloatVarValue(a), ty::FloatVarValue(b)) = v; TypeError::FloatMismatch(ExpectedFound::new(a_is_expected, a, b)) } - -fn effect_unification_error<'tcx>( - _tcx: TyCtxt<'tcx>, - _a_is_expected: bool, - (_a, _b): (EffectVarValue<'tcx>, EffectVarValue<'tcx>), -) -> TypeError<'tcx> { - bug!("unexpected effect unification error") -} diff --git a/compiler/rustc_infer/src/infer/resolve.rs b/compiler/rustc_infer/src/infer/resolve.rs index 959b0903127..d5999331dfa 100644 --- a/compiler/rustc_infer/src/infer/resolve.rs +++ b/compiler/rustc_infer/src/infer/resolve.rs @@ -237,14 +237,13 @@ fn fold_const(&mut self, c: ty::Const<'tcx>) -> ty::Const<'tcx> { } ty::ConstKind::Infer(ty::InferConst::EffectVar(vid)) => { debug_assert_eq!(c.ty(), self.infcx.tcx.types.bool); - match self.infcx.probe_effect_var(vid) { - Some(c) => c.as_const(self.infcx.tcx), - None => ty::Const::new_infer( + self.infcx.probe_effect_var(vid).unwrap_or_else(|| { + ty::Const::new_infer( self.infcx.tcx, ty::InferConst::EffectVar(self.infcx.root_effect_var(vid)), self.infcx.tcx.types.bool, - ), - } + ) + }) } _ => { if c.has_infer() { diff --git a/compiler/rustc_middle/src/infer/unify_key.rs b/compiler/rustc_middle/src/infer/unify_key.rs index c35799ef47f..63c0ebd5f6b 100644 --- a/compiler/rustc_middle/src/infer/unify_key.rs +++ b/compiler/rustc_middle/src/infer/unify_key.rs @@ -194,33 +194,37 @@ fn unify_values(&value1: &Self, &value2: &Self) -> Result { /// values for the effect inference variable #[derive(Clone, Copy, Debug)] pub enum EffectVarValue<'tcx> { - /// The host effect is on, enabling access to syscalls, filesystem access, etc. - Host, - /// The host effect is off. Execution is restricted to const operations only. - NoHost, - Const(ty::Const<'tcx>), + Unknown, + Known(ty::Const<'tcx>), } impl<'tcx> EffectVarValue<'tcx> { - pub fn as_const(self, tcx: TyCtxt<'tcx>) -> ty::Const<'tcx> { + pub fn known(self) -> Option> { match self { - EffectVarValue::Host => tcx.consts.true_, - EffectVarValue::NoHost => tcx.consts.false_, - EffectVarValue::Const(c) => c, + EffectVarValue::Unknown => None, + EffectVarValue::Known(value) => Some(value), + } + } + + pub fn is_unknown(self) -> bool { + match self { + EffectVarValue::Unknown => true, + EffectVarValue::Known(_) => false, } } } impl<'tcx> UnifyValue for EffectVarValue<'tcx> { - type Error = (EffectVarValue<'tcx>, EffectVarValue<'tcx>); + type Error = NoError; fn unify_values(value1: &Self, value2: &Self) -> Result { - match (value1, value2) { - (EffectVarValue::Host, EffectVarValue::Host) => Ok(EffectVarValue::Host), - (EffectVarValue::NoHost, EffectVarValue::NoHost) => Ok(EffectVarValue::NoHost), - (EffectVarValue::NoHost | EffectVarValue::Host, _) - | (_, EffectVarValue::NoHost | EffectVarValue::Host) => Err((*value1, *value2)), - (EffectVarValue::Const(_), EffectVarValue::Const(_)) => { - bug!("equating two const variables, both of which have known values") + match (*value1, *value2) { + (EffectVarValue::Unknown, EffectVarValue::Unknown) => Ok(EffectVarValue::Unknown), + (EffectVarValue::Unknown, EffectVarValue::Known(val)) + | (EffectVarValue::Known(val), EffectVarValue::Unknown) => { + Ok(EffectVarValue::Known(val)) + } + (EffectVarValue::Known(_), EffectVarValue::Known(_)) => { + bug!("equating known inference variables: {value1:?} {value2:?}") } } } @@ -229,7 +233,7 @@ fn unify_values(value1: &Self, value2: &Self) -> Result { #[derive(PartialEq, Copy, Clone, Debug)] pub struct EffectVidKey<'tcx> { pub vid: ty::EffectVid, - pub phantom: PhantomData>, + pub phantom: PhantomData>, } impl<'tcx> From for EffectVidKey<'tcx> { @@ -239,7 +243,7 @@ fn from(vid: ty::EffectVid) -> Self { } impl<'tcx> UnifyKey for EffectVidKey<'tcx> { - type Value = Option>; + type Value = EffectVarValue<'tcx>; #[inline] fn index(&self) -> u32 { self.vid.as_u32() From d9508a1fd2bdbc2f7c4e2ee28503f15487fdc8ce Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 2 Feb 2024 15:44:22 +1100 Subject: [PATCH 11/16] Make `Emitter::emit_diagnostic` consuming. All the other `emit`/`emit_diagnostic` methods were recently made consuming (e.g. #119606), but this one wasn't. But it makes sense to. Much of this is straightforward, and lots of `clone` calls are avoided. There are a couple of tricky bits. - `Emitter::primary_span_formatted` no longer takes a `Diagnostic` and returns a pair. Instead it takes the two fields from `Diagnostic` that it used (`span` and `suggestions`) as `&mut`, and modifies them. This is necessary to avoid the cloning of `diag.children` in two emitters. - `from_errors_diagnostic` is rearranged so various uses of `diag` occur before the consuming `emit_diagnostic` call. --- compiler/rustc_codegen_ssa/src/back/write.rs | 2 +- .../src/annotate_snippet_emitter_writer.rs | 16 +++--- compiler/rustc_errors/src/emitter.rs | 41 +++++++-------- compiler/rustc_errors/src/json.rs | 52 ++++++++++--------- compiler/rustc_errors/src/lib.rs | 26 ++++++---- .../passes/lint/check_code_block_syntax.rs | 2 +- src/tools/rustfmt/src/parse/session.rs | 20 +++---- 7 files changed, 84 insertions(+), 75 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 4211f875dd0..9b24339d255 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -1810,7 +1810,7 @@ fn fallback_fluent_bundle(&self) -> &FluentBundle { } impl Emitter for SharedEmitter { - fn emit_diagnostic(&mut self, diag: &rustc_errors::Diagnostic) { + fn emit_diagnostic(&mut self, diag: rustc_errors::Diagnostic) { let args: FxHashMap = diag.args().map(|(name, arg)| (name.clone(), arg.clone())).collect(); drop(self.sender.send(SharedEmitterMessage::Diagnostic(Diagnostic { diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index 949f52ef6b5..1ce75a03812 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -44,15 +44,15 @@ fn fallback_fluent_bundle(&self) -> &FluentBundle { impl Emitter for AnnotateSnippetEmitter { /// The entry point for the diagnostics generation - fn emit_diagnostic(&mut self, diag: &Diagnostic) { + fn emit_diagnostic(&mut self, mut diag: Diagnostic) { let fluent_args = to_fluent_args(diag.args()); - let mut children = diag.children.clone(); - let (mut primary_span, suggestions) = self.primary_span_formatted(diag, &fluent_args); + let mut suggestions = diag.suggestions.unwrap_or(vec![]); + self.primary_span_formatted(&mut diag.span, &mut suggestions, &fluent_args); self.fix_multispans_in_extern_macros_and_render_macro_backtrace( - &mut primary_span, - &mut children, + &mut diag.span, + &mut diag.children, &diag.level, self.macro_backtrace, ); @@ -62,9 +62,9 @@ fn emit_diagnostic(&mut self, diag: &Diagnostic) { &diag.messages, &fluent_args, &diag.code, - &primary_span, - &children, - suggestions, + &diag.span, + &diag.children, + &suggestions, ); } diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 4be5ed923e5..12d947f2098 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -193,7 +193,7 @@ fn right(&self, line_len: usize) -> usize { /// Emitter trait for emitting errors. pub trait Emitter: Translate { /// Emit a structured diagnostic. - fn emit_diagnostic(&mut self, diag: &Diagnostic); + fn emit_diagnostic(&mut self, diag: Diagnostic); /// Emit a notification that an artifact has been output. /// Currently only supported for the JSON format. @@ -230,17 +230,17 @@ fn supports_color(&self) -> bool { /// /// * If the current `Diagnostic` has only one visible `CodeSuggestion`, /// we format the `help` suggestion depending on the content of the - /// substitutions. In that case, we return the modified span only. + /// substitutions. In that case, we modify the span and clear the + /// suggestions. /// /// * If the current `Diagnostic` has multiple suggestions, - /// we return the original `primary_span` and the original suggestions. - fn primary_span_formatted<'a>( + /// we leave `primary_span` and the suggestions untouched. + fn primary_span_formatted( &mut self, - diag: &'a Diagnostic, + primary_span: &mut MultiSpan, + suggestions: &mut Vec, fluent_args: &FluentArgs<'_>, - ) -> (MultiSpan, &'a [CodeSuggestion]) { - let mut primary_span = diag.span.clone(); - let suggestions = diag.suggestions.as_deref().unwrap_or(&[]); + ) { if let Some((sugg, rest)) = suggestions.split_first() { let msg = self.translate_message(&sugg.msg, fluent_args).map_err(Report::new).unwrap(); if rest.is_empty() && @@ -287,16 +287,15 @@ fn primary_span_formatted<'a>( primary_span.push_span_label(sugg.substitutions[0].parts[0].span, msg); // We return only the modified primary_span - (primary_span, &[]) + suggestions.clear(); } else { // if there are multiple suggestions, print them all in full // to be consistent. We could try to figure out if we can // make one (or the first one) inline, but that would give // undue importance to a semi-random suggestion - (primary_span, suggestions) } } else { - (primary_span, suggestions) + // do nothing } } @@ -518,16 +517,15 @@ fn source_map(&self) -> Option<&Lrc> { self.sm.as_ref() } - fn emit_diagnostic(&mut self, diag: &Diagnostic) { + fn emit_diagnostic(&mut self, mut diag: Diagnostic) { let fluent_args = to_fluent_args(diag.args()); - let mut children = diag.children.clone(); - let (mut primary_span, suggestions) = self.primary_span_formatted(diag, &fluent_args); - debug!("emit_diagnostic: suggestions={:?}", suggestions); + let mut suggestions = diag.suggestions.unwrap_or(vec![]); + self.primary_span_formatted(&mut diag.span, &mut suggestions, &fluent_args); self.fix_multispans_in_extern_macros_and_render_macro_backtrace( - &mut primary_span, - &mut children, + &mut diag.span, + &mut diag.children, &diag.level, self.macro_backtrace, ); @@ -537,9 +535,9 @@ fn emit_diagnostic(&mut self, diag: &Diagnostic) { &diag.messages, &fluent_args, &diag.code, - &primary_span, - &children, - suggestions, + &diag.span, + &diag.children, + &suggestions, self.track_diagnostics.then_some(&diag.emitted_at), ); } @@ -576,9 +574,8 @@ fn source_map(&self) -> Option<&Lrc> { None } - fn emit_diagnostic(&mut self, diag: &Diagnostic) { + fn emit_diagnostic(&mut self, mut diag: Diagnostic) { if diag.level == Level::Fatal { - let mut diag = diag.clone(); diag.note(self.fatal_note.clone()); self.fatal_dcx.emit_diagnostic(diag); } diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 6f922998279..470e3d52452 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -176,7 +176,7 @@ fn fallback_fluent_bundle(&self) -> &FluentBundle { } impl Emitter for JsonEmitter { - fn emit_diagnostic(&mut self, diag: &crate::Diagnostic) { + fn emit_diagnostic(&mut self, diag: crate::Diagnostic) { let data = Diagnostic::from_errors_diagnostic(diag, self); let result = self.emit(EmitTyped::Diagnostic(data)); if let Err(e) = result { @@ -201,7 +201,7 @@ fn emit_future_breakage_report(&mut self, diags: Vec) { } FutureBreakageItem { diagnostic: EmitTyped::Diagnostic(Diagnostic::from_errors_diagnostic( - &diag, self, + diag, self, )), } }) @@ -340,7 +340,7 @@ struct UnusedExterns<'a, 'b, 'c> { } impl Diagnostic { - fn from_errors_diagnostic(diag: &crate::Diagnostic, je: &JsonEmitter) -> Diagnostic { + fn from_errors_diagnostic(diag: crate::Diagnostic, je: &JsonEmitter) -> Diagnostic { let args = to_fluent_args(diag.args()); let sugg = diag.suggestions.iter().flatten().map(|sugg| { let translated_message = @@ -382,6 +382,28 @@ fn reset(&mut self) -> io::Result<()> { Ok(()) } } + + let translated_message = je.translate_messages(&diag.messages, &args); + + let code = if let Some(code) = diag.code { + Some(DiagnosticCode { + code: code.to_string(), + explanation: je.registry.as_ref().unwrap().try_find_description(code).ok(), + }) + } else if let Some(IsLint { name, .. }) = &diag.is_lint { + Some(DiagnosticCode { code: name.to_string(), explanation: None }) + } else { + None + }; + let level = diag.level.to_str(); + let spans = DiagnosticSpan::from_multispan(&diag.span, &args, je); + let children = diag + .children + .iter() + .map(|c| Diagnostic::from_sub_diagnostic(c, &args, je)) + .chain(sugg) + .collect(); + let buf = BufWriter::default(); let output = buf.clone(); je.json_rendered @@ -398,30 +420,12 @@ fn reset(&mut self) -> io::Result<()> { let output = Arc::try_unwrap(output.0).unwrap().into_inner().unwrap(); let output = String::from_utf8(output).unwrap(); - let translated_message = je.translate_messages(&diag.messages, &args); - - let code = if let Some(code) = diag.code { - Some(DiagnosticCode { - code: code.to_string(), - explanation: je.registry.as_ref().unwrap().try_find_description(code).ok(), - }) - } else if let Some(IsLint { name, .. }) = &diag.is_lint { - Some(DiagnosticCode { code: name.to_string(), explanation: None }) - } else { - None - }; - Diagnostic { message: translated_message.to_string(), code, - level: diag.level.to_str(), - spans: DiagnosticSpan::from_multispan(&diag.span, &args, je), - children: diag - .children - .iter() - .map(|c| Diagnostic::from_sub_diagnostic(c, &args, je)) - .chain(sugg) - .collect(), + level, + spans, + children, rendered: Some(output), } } diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index b2bd4d8eb95..a48fcf3af9b 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -990,9 +990,13 @@ pub fn print_error_count(&self, registry: &Registry) { match (errors.len(), warnings.len()) { (0, 0) => return, - (0, _) => inner - .emitter - .emit_diagnostic(&Diagnostic::new(Warning, DiagnosticMessage::Str(warnings))), + (0, _) => { + // Use `inner.emitter` directly, otherwise the warning might not be emitted, e.g. + // with a configuration like `--cap-lints allow --force-warn bare_trait_objects`. + inner + .emitter + .emit_diagnostic(Diagnostic::new(Warning, DiagnosticMessage::Str(warnings))); + } (_, 0) => { inner.emit_diagnostic(Diagnostic::new(Fatal, errors)); } @@ -1056,7 +1060,7 @@ pub fn must_teach(&self, code: ErrCode) -> bool { } pub fn force_print_diagnostic(&self, db: Diagnostic) { - self.inner.borrow_mut().emitter.emit_diagnostic(&db); + self.inner.borrow_mut().emitter.emit_diagnostic(db); } pub fn emit_diagnostic(&self, diagnostic: Diagnostic) -> Option { @@ -1324,11 +1328,15 @@ fn emit_diagnostic(&mut self, mut diagnostic: Diagnostic) -> Option Option Option &rustc_errors::FluentBundle { } impl Emitter for BufferEmitter { - fn emit_diagnostic(&mut self, diag: &Diagnostic) { + fn emit_diagnostic(&mut self, diag: Diagnostic) { let mut buffer = self.buffer.borrow_mut(); let fluent_args = to_fluent_args(diag.args()); diff --git a/src/tools/rustfmt/src/parse/session.rs b/src/tools/rustfmt/src/parse/session.rs index 6dc3eac44d4..f0af401d3da 100644 --- a/src/tools/rustfmt/src/parse/session.rs +++ b/src/tools/rustfmt/src/parse/session.rs @@ -47,7 +47,7 @@ fn source_map(&self) -> Option<&Lrc> { None } - fn emit_diagnostic(&mut self, _db: &Diagnostic) {} + fn emit_diagnostic(&mut self, _db: Diagnostic) {} } fn silent_emitter() -> Box { @@ -64,7 +64,7 @@ struct SilentOnIgnoredFilesEmitter { } impl SilentOnIgnoredFilesEmitter { - fn handle_non_ignoreable_error(&mut self, db: &Diagnostic) { + fn handle_non_ignoreable_error(&mut self, db: Diagnostic) { self.has_non_ignorable_parser_errors = true; self.can_reset.store(false, Ordering::Release); self.emitter.emit_diagnostic(db); @@ -86,7 +86,7 @@ fn source_map(&self) -> Option<&Lrc> { None } - fn emit_diagnostic(&mut self, db: &Diagnostic) { + fn emit_diagnostic(&mut self, db: Diagnostic) { if db.level() == DiagnosticLevel::Fatal { return self.handle_non_ignoreable_error(db); } @@ -365,7 +365,7 @@ fn source_map(&self) -> Option<&Lrc> { None } - fn emit_diagnostic(&mut self, _db: &Diagnostic) { + fn emit_diagnostic(&mut self, _db: Diagnostic) { self.num_emitted_errors.fetch_add(1, Ordering::Release); } } @@ -424,7 +424,7 @@ fn handles_fatal_parse_error_in_ignored_file() { ); let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1))); let fatal_diagnostic = build_diagnostic(DiagnosticLevel::Fatal, Some(span)); - emitter.emit_diagnostic(&fatal_diagnostic); + emitter.emit_diagnostic(fatal_diagnostic); assert_eq!(num_emitted_errors.load(Ordering::Acquire), 1); assert_eq!(can_reset_errors.load(Ordering::Acquire), false); } @@ -449,7 +449,7 @@ fn handles_recoverable_parse_error_in_ignored_file() { ); let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1))); let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(span)); - emitter.emit_diagnostic(&non_fatal_diagnostic); + emitter.emit_diagnostic(non_fatal_diagnostic); assert_eq!(num_emitted_errors.load(Ordering::Acquire), 0); assert_eq!(can_reset_errors.load(Ordering::Acquire), true); } @@ -473,7 +473,7 @@ fn handles_recoverable_parse_error_in_non_ignored_file() { ); let span = MultiSpan::from_span(mk_sp(BytePos(0), BytePos(1))); let non_fatal_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(span)); - emitter.emit_diagnostic(&non_fatal_diagnostic); + emitter.emit_diagnostic(non_fatal_diagnostic); assert_eq!(num_emitted_errors.load(Ordering::Acquire), 1); assert_eq!(can_reset_errors.load(Ordering::Acquire), false); } @@ -512,9 +512,9 @@ fn handles_mix_of_recoverable_parse_error() { let bar_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(bar_span)); let foo_diagnostic = build_diagnostic(DiagnosticLevel::Warning, Some(foo_span)); let fatal_diagnostic = build_diagnostic(DiagnosticLevel::Fatal, None); - emitter.emit_diagnostic(&bar_diagnostic); - emitter.emit_diagnostic(&foo_diagnostic); - emitter.emit_diagnostic(&fatal_diagnostic); + emitter.emit_diagnostic(bar_diagnostic); + emitter.emit_diagnostic(foo_diagnostic); + emitter.emit_diagnostic(fatal_diagnostic); assert_eq!(num_emitted_errors.load(Ordering::Acquire), 2); assert_eq!(can_reset_errors.load(Ordering::Acquire), false); } From f32aa1aef991d70c5d3a72a2d35fe65a99348d48 Mon Sep 17 00:00:00 2001 From: klensy Date: Mon, 5 Feb 2024 14:16:40 +0300 Subject: [PATCH 12/16] rustc_metadata: fix typo --- compiler/rustc_metadata/src/rmeta/encoder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 542caf86223..85631bd8edb 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -386,7 +386,7 @@ fn encode_alloc_id(&mut self, alloc_id: &rustc_middle::mir::interpret::AllocId) } } -// Shorthand for `$self.$tables.$table.set_some($def_id.index, $self.lazy_value($value))`, which would +// Shorthand for `$self.$tables.$table.set_some($def_id.index, $self.lazy($value))`, which would // normally need extra variables to avoid errors about multiple mutable borrows. macro_rules! record { ($self:ident.$tables:ident.$table:ident[$def_id:expr] <- $value:expr) => {{ @@ -398,7 +398,7 @@ macro_rules! record { }}; } -// Shorthand for `$self.$tables.$table.set_some($def_id.index, $self.lazy_value($value))`, which would +// Shorthand for `$self.$tables.$table.set_some($def_id.index, $self.lazy_array($value))`, which would // normally need extra variables to avoid errors about multiple mutable borrows. macro_rules! record_array { ($self:ident.$tables:ident.$table:ident[$def_id:expr] <- $value:expr) => {{ From d3214372046eb303ee7c4e569602cd3205975ebc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Tue, 6 Feb 2024 00:37:57 +0100 Subject: [PATCH 13/16] Remove b-naber from the compiler review rotation --- triagebot.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/triagebot.toml b/triagebot.toml index 68c863bb919..eba8e283815 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -658,7 +658,6 @@ compiler-team = [ ] compiler-team-contributors = [ "@TaKO8Ki", - "@b-naber", "@nnethercote", "@fmease", ] From 0b6af718d85a2639854ad09730f5f39aa5f70f9a Mon Sep 17 00:00:00 2001 From: trevyn <230691+trevyn@users.noreply.github.com> Date: Mon, 5 Feb 2024 18:53:28 -0800 Subject: [PATCH 14/16] Create helper `maybe_report_similar_assoc_fn` --- .../rustc_hir_analysis/src/astconv/errors.rs | 51 ++++++++++++++++++ .../rustc_hir_analysis/src/astconv/mod.rs | 52 +------------------ tests/ui/suggestions/issue-109195.rs | 10 ++-- tests/ui/suggestions/issue-109195.stderr | 18 +++---- 4 files changed, 66 insertions(+), 65 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/astconv/errors.rs b/compiler/rustc_hir_analysis/src/astconv/errors.rs index 407517b15ef..869e2184006 100644 --- a/compiler/rustc_hir_analysis/src/astconv/errors.rs +++ b/compiler/rustc_hir_analysis/src/astconv/errors.rs @@ -14,6 +14,7 @@ use rustc_hir as hir; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_infer::traits::FulfillmentError; +use rustc_middle::query::Key; use rustc_middle::ty::{self, suggest_constraining_type_param, Ty, TyCtxt, TypeVisitableExt}; use rustc_session::parse::feature_err; use rustc_span::edit_distance::find_best_match_for_name; @@ -859,6 +860,56 @@ pub(crate) fn complain_about_missing_associated_types( self.set_tainted_by_errors(err.emit()); } + + /// On ambiguous associated type, look for an associated function whose name matches the + /// extended path and, if found, emit an E0223 error with a structured suggestion. + /// e.g. for `String::from::utf8`, suggest `String::from_utf8` (#109195) + pub(crate) fn maybe_report_similar_assoc_fn( + &self, + span: Span, + qself_ty: Ty<'tcx>, + qself: &hir::Ty<'_>, + ) -> Result<(), ErrorGuaranteed> { + let tcx = self.tcx(); + if let Some((_, node)) = tcx.hir().parent_iter(qself.hir_id).skip(1).next() + && let hir::Node::Expr(hir::Expr { + kind: + hir::ExprKind::Path(hir::QPath::TypeRelative( + hir::Ty { + kind: + hir::TyKind::Path(hir::QPath::TypeRelative( + _, + hir::PathSegment { ident: ident2, .. }, + )), + .. + }, + hir::PathSegment { ident: ident3, .. }, + )), + .. + }) = node + && let Some(ty_def_id) = qself_ty.ty_def_id() + && let Ok([inherent_impl]) = tcx.inherent_impls(ty_def_id) + && let name = format!("{ident2}_{ident3}") + && let Some(ty::AssocItem { kind: ty::AssocKind::Fn, .. }) = tcx + .associated_items(inherent_impl) + .filter_by_name_unhygienic(Symbol::intern(&name)) + .next() + { + let reported = + struct_span_code_err!(tcx.dcx(), span, E0223, "ambiguous associated type") + .with_span_suggestion_verbose( + ident2.span.to(ident3.span), + format!("there is an associated function with a similar name: `{name}`"), + name, + Applicability::MaybeIncorrect, + ) + .emit(); + self.set_tainted_by_errors(reported); + Err(reported) + } else { + Ok(()) + } + } } /// Emits an error regarding forbidden type binding associations diff --git a/compiler/rustc_hir_analysis/src/astconv/mod.rs b/compiler/rustc_hir_analysis/src/astconv/mod.rs index d69f366a66c..cfd38fb48f4 100644 --- a/compiler/rustc_hir_analysis/src/astconv/mod.rs +++ b/compiler/rustc_hir_analysis/src/astconv/mod.rs @@ -29,7 +29,6 @@ use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; use rustc_infer::traits::ObligationCause; use rustc_middle::middle::stability::AllowUnstable; -use rustc_middle::query::Key; use rustc_middle::ty::{ self, Const, GenericArgKind, GenericArgsRef, GenericParamDefKind, IsSuggestable, ParamEnv, Ty, TyCtxt, TypeVisitableExt, @@ -1374,56 +1373,7 @@ pub fn associated_path_to_ty( ) .emit() // Already reported in an earlier stage. } else { - // suggest methods that look similar to the path - // e.g. for `String::from::utf8`, suggest `String::from_utf8` (#109195) - for (_, node) in tcx.hir().parent_iter(qself.hir_id) { - if let hir::Node::Expr(hir::Expr { - kind: - hir::ExprKind::Path(hir::QPath::TypeRelative( - hir::Ty { - kind: - hir::TyKind::Path(hir::QPath::TypeRelative( - _, - hir::PathSegment { ident: ident2, .. }, - )), - .. - }, - hir::PathSegment { ident: ident3, .. }, - )), - .. - }) = node - { - let name = format!("{ident2}_{ident3}"); - if if let Some(ty_def_id) = qself_ty.ty_def_id() - && let Ok([inherent_impl]) = tcx.inherent_impls(ty_def_id) - && let Some(ty::AssocItem { kind: ty::AssocKind::Fn, .. }) = tcx - .associated_items(inherent_impl) - .filter_by_name_unhygienic(Symbol::intern(&name)) - .next() - { - true - } else { - qself_ty.is_str() - && ["from_utf8", "from_utf8_mut"].contains(&name.as_str()) - } { - let reported = struct_span_code_err!( - tcx.dcx(), - span, - E0223, - "ambiguous associated type" - ) - .with_span_suggestion_verbose( - ident2.span.to(ident3.span), - format!("you might have meant to use `{name}`"), - name, - Applicability::MaybeIncorrect, - ) - .emit(); - self.set_tainted_by_errors(reported); - return Err(reported); - } - } - } + self.maybe_report_similar_assoc_fn(span, qself_ty, qself)?; let traits: Vec<_> = self.probe_traits_that_match_assoc_ty(qself_ty, assoc_ident); diff --git a/tests/ui/suggestions/issue-109195.rs b/tests/ui/suggestions/issue-109195.rs index badb859dbb7..cc499b0d776 100644 --- a/tests/ui/suggestions/issue-109195.rs +++ b/tests/ui/suggestions/issue-109195.rs @@ -1,20 +1,20 @@ fn main() { String::from::utf8; //~^ ERROR ambiguous associated type [E0223] - //~| HELP you might have meant to use `from_utf8` + //~| HELP there is an associated function with a similar name: `from_utf8` String::from::utf8(); //~^ ERROR ambiguous associated type [E0223] - //~| HELP you might have meant to use `from_utf8` + //~| HELP there is an associated function with a similar name: `from_utf8` String::from::utf16(); //~^ ERROR ambiguous associated type [E0223] - //~| HELP you might have meant to use `from_utf16` + //~| HELP there is an associated function with a similar name: `from_utf16` String::from::method_that_doesnt_exist(); //~^ ERROR ambiguous associated type [E0223] //~| HELP if there were a trait named `Example` with associated type `from` str::from::utf8(); //~^ ERROR ambiguous associated type [E0223] - //~| HELP you might have meant to use `from_utf8` + //~| HELP if there were a trait named `Example` with associated type `from` str::from::utf8_mut(); //~^ ERROR ambiguous associated type [E0223] - //~| HELP you might have meant to use `from_utf8_mut` + //~| HELP if there were a trait named `Example` with associated type `from` } diff --git a/tests/ui/suggestions/issue-109195.stderr b/tests/ui/suggestions/issue-109195.stderr index 5c9ef9f98f5..10cf9cfd28c 100644 --- a/tests/ui/suggestions/issue-109195.stderr +++ b/tests/ui/suggestions/issue-109195.stderr @@ -4,7 +4,7 @@ error[E0223]: ambiguous associated type LL | String::from::utf8; | ^^^^^^^^^^^^ | -help: you might have meant to use `from_utf8` +help: there is an associated function with a similar name: `from_utf8` | LL | String::from_utf8; | ~~~~~~~~~ @@ -15,7 +15,7 @@ error[E0223]: ambiguous associated type LL | String::from::utf8(); | ^^^^^^^^^^^^ | -help: you might have meant to use `from_utf8` +help: there is an associated function with a similar name: `from_utf8` | LL | String::from_utf8(); | ~~~~~~~~~ @@ -26,7 +26,7 @@ error[E0223]: ambiguous associated type LL | String::from::utf16(); | ^^^^^^^^^^^^ | -help: you might have meant to use `from_utf16` +help: there is an associated function with a similar name: `from_utf16` | LL | String::from_utf16(); | ~~~~~~~~~~ @@ -48,10 +48,10 @@ error[E0223]: ambiguous associated type LL | str::from::utf8(); | ^^^^^^^^^ | -help: you might have meant to use `from_utf8` +help: if there were a trait named `Example` with associated type `from` implemented for `str`, you could use the fully-qualified path | -LL | str::from_utf8(); - | ~~~~~~~~~ +LL | ::from::utf8(); + | ~~~~~~~~~~~~~~~~~~~~~~ error[E0223]: ambiguous associated type --> $DIR/issue-109195.rs:17:5 @@ -59,10 +59,10 @@ error[E0223]: ambiguous associated type LL | str::from::utf8_mut(); | ^^^^^^^^^ | -help: you might have meant to use `from_utf8_mut` +help: if there were a trait named `Example` with associated type `from` implemented for `str`, you could use the fully-qualified path | -LL | str::from_utf8_mut(); - | ~~~~~~~~~~~~~ +LL | ::from::utf8_mut(); + | ~~~~~~~~~~~~~~~~~~~~~~ error: aborting due to 6 previous errors From 25635b9a96ebbbd87bbbfb1d7c31004651179f2a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 5 Feb 2024 21:48:58 +0100 Subject: [PATCH 15/16] miri: fix ICE with symbolic alignment check on extern static --- .../src/interpret/validity.rs | 5 +-- src/tools/miri/src/borrow_tracker/mod.rs | 2 +- src/tools/miri/src/machine.rs | 34 +++++++++++-------- src/tools/miri/src/provenance_gc.rs | 1 + src/tools/miri/src/shims/foreign_items.rs | 12 +++---- ...88-ice-symbolic-alignment-extern-static.rs | 12 +++++++ ...ce-symbolic-alignment-extern-static.stderr | 14 ++++++++ .../miri/tests/pass/align_offset_symbolic.rs | 23 ++++++++++++- 8 files changed, 76 insertions(+), 27 deletions(-) create mode 100644 src/tools/miri/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.rs create mode 100644 src/tools/miri/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.stderr diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs index b5cd3259520..4dbea54e224 100644 --- a/compiler/rustc_const_eval/src/interpret/validity.rs +++ b/compiler/rustc_const_eval/src/interpret/validity.rs @@ -992,10 +992,7 @@ fn validate_operand_internal( // Complain about any other kind of error -- those are bad because we'd like to // report them in a way that shows *where* in the value the issue lies. Err(err) => { - bug!( - "Unexpected Undefined Behavior error during validation: {}", - self.format_error(err) - ); + bug!("Unexpected error during validation: {}", self.format_error(err)); } } } diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index 74ff6ed4e0a..46f96a715f1 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -383,7 +383,7 @@ fn on_stack_pop( // will never cause UB on the pointer itself. let (_, _, kind) = this.get_alloc_info(*alloc_id); if matches!(kind, AllocKind::LiveData) { - let alloc_extra = this.get_alloc_extra(*alloc_id).unwrap(); + let alloc_extra = this.get_alloc_extra(*alloc_id)?; // can still fail for `extern static` let alloc_borrow_tracker = &alloc_extra.borrow_tracker.as_ref().unwrap(); alloc_borrow_tracker.release_protector(&this.machine, borrow_tracker, *tag)?; } diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 946887637ff..40d041c8fdb 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -2,7 +2,7 @@ //! `Machine` trait. use std::borrow::Cow; -use std::cell::{Cell, RefCell}; +use std::cell::RefCell; use std::collections::hash_map::Entry; use std::fmt; use std::path::Path; @@ -336,20 +336,11 @@ pub struct AllocExtra<'tcx> { /// if this allocation is leakable. The backtrace is not /// pruned yet; that should be done before printing it. pub backtrace: Option>>, - /// An offset inside this allocation that was deemed aligned even for symbolic alignment checks. - /// Invariant: the promised alignment will never be less than the native alignment of this allocation. - pub symbolic_alignment: Cell>, } impl VisitProvenance for AllocExtra<'_> { fn visit_provenance(&self, visit: &mut VisitWith<'_>) { - let AllocExtra { - borrow_tracker, - data_race, - weak_memory, - backtrace: _, - symbolic_alignment: _, - } = self; + let AllocExtra { borrow_tracker, data_race, weak_memory, backtrace: _ } = self; borrow_tracker.visit_provenance(visit); data_race.visit_provenance(visit); @@ -572,6 +563,14 @@ pub struct MiriMachine<'mir, 'tcx> { /// that is fixed per stack frame; this lets us have sometimes different results for the /// same const while ensuring consistent results within a single call. const_cache: RefCell, usize), OpTy<'tcx, Provenance>>>, + + /// For each allocation, an offset inside that allocation that was deemed aligned even for + /// symbolic alignment checks. This cannot be stored in `AllocExtra` since it needs to be + /// tracked for vtables and function allocations as well as regular allocations. + /// + /// Invariant: the promised alignment will never be less than the native alignment of the + /// allocation. + pub(crate) symbolic_alignment: RefCell>, } impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { @@ -698,6 +697,7 @@ pub(crate) fn new(config: &MiriConfig, layout_cx: LayoutCx<'tcx, TyCtxt<'tcx>>) collect_leak_backtraces: config.collect_leak_backtraces, allocation_spans: RefCell::new(FxHashMap::default()), const_cache: RefCell::new(FxHashMap::default()), + symbolic_alignment: RefCell::new(FxHashMap::default()), } } @@ -810,6 +810,7 @@ fn visit_provenance(&self, visit: &mut VisitWith<'_>) { collect_leak_backtraces: _, allocation_spans: _, const_cache: _, + symbolic_alignment: _, } = self; threads.visit_provenance(visit); @@ -893,9 +894,13 @@ fn alignment_check( return None; } // Let's see which alignment we have been promised for this allocation. - let alloc_info = ecx.get_alloc_extra(alloc_id).unwrap(); // cannot fail since the allocation is live - let (promised_offset, promised_align) = - alloc_info.symbolic_alignment.get().unwrap_or((Size::ZERO, alloc_align)); + let (promised_offset, promised_align) = ecx + .machine + .symbolic_alignment + .borrow() + .get(&alloc_id) + .copied() + .unwrap_or((Size::ZERO, alloc_align)); if promised_align < align { // Definitely not enough. Some(Misalignment { has: promised_align, required: align }) @@ -1132,7 +1137,6 @@ fn adjust_allocation<'b>( data_race: race_alloc, weak_memory: buffer_alloc, backtrace, - symbolic_alignment: Cell::new(None), }, |ptr| ecx.global_base_pointer(ptr), )?; diff --git a/src/tools/miri/src/provenance_gc.rs b/src/tools/miri/src/provenance_gc.rs index ab178f82d9f..347951ce372 100644 --- a/src/tools/miri/src/provenance_gc.rs +++ b/src/tools/miri/src/provenance_gc.rs @@ -196,6 +196,7 @@ fn remove_unreachable_allocs(&mut self, allocs: FxHashSet) { let this = self.eval_context_mut(); let allocs = LiveAllocs { ecx: this, collected: allocs }; this.machine.allocation_spans.borrow_mut().retain(|id, _| allocs.is_live(*id)); + this.machine.symbolic_alignment.borrow_mut().retain(|id, _| allocs.is_live(*id)); this.machine.intptrcast.borrow_mut().remove_unreachable_allocs(&allocs); if let Some(borrow_tracker) = &this.machine.borrow_tracker { borrow_tracker.borrow_mut().remove_unreachable_allocs(&allocs); diff --git a/src/tools/miri/src/shims/foreign_items.rs b/src/tools/miri/src/shims/foreign_items.rs index a002f2aad05..25654248db4 100644 --- a/src/tools/miri/src/shims/foreign_items.rs +++ b/src/tools/miri/src/shims/foreign_items.rs @@ -585,17 +585,17 @@ fn emulate_foreign_item_inner( } if let Ok((alloc_id, offset, ..)) = this.ptr_try_get_alloc_id(ptr) { let (_size, alloc_align, _kind) = this.get_alloc_info(alloc_id); - // Not `get_alloc_extra_mut`, need to handle read-only allocations! - let alloc_extra = this.get_alloc_extra(alloc_id)?; // If the newly promised alignment is bigger than the native alignment of this // allocation, and bigger than the previously promised alignment, then set it. if align > alloc_align - && !alloc_extra + && !this + .machine .symbolic_alignment - .get() - .is_some_and(|(_, old_align)| align <= old_align) + .get_mut() + .get(&alloc_id) + .is_some_and(|&(_, old_align)| align <= old_align) { - alloc_extra.symbolic_alignment.set(Some((offset, align))); + this.machine.symbolic_alignment.get_mut().insert(alloc_id, (offset, align)); } } } diff --git a/src/tools/miri/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.rs b/src/tools/miri/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.rs new file mode 100644 index 00000000000..44604074982 --- /dev/null +++ b/src/tools/miri/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.rs @@ -0,0 +1,12 @@ +//@compile-flags: -Zmiri-symbolic-alignment-check + +extern "C" { + static _dispatch_queue_attr_concurrent: [u8; 0]; +} + +static DISPATCH_QUEUE_CONCURRENT: &'static [u8; 0] = + unsafe { &_dispatch_queue_attr_concurrent }; + +fn main() { + let _val = *DISPATCH_QUEUE_CONCURRENT; //~ERROR: is not supported +} diff --git a/src/tools/miri/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.stderr b/src/tools/miri/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.stderr new file mode 100644 index 00000000000..a4249d2e881 --- /dev/null +++ b/src/tools/miri/tests/fail/issue-miri-3288-ice-symbolic-alignment-extern-static.stderr @@ -0,0 +1,14 @@ +error: unsupported operation: `extern` static `_dispatch_queue_attr_concurrent` from crate `issue_miri_3288_ice_symbolic_alignment_extern_static` is not supported by Miri + --> $DIR/issue-miri-3288-ice-symbolic-alignment-extern-static.rs:LL:CC + | +LL | let _val = *DISPATCH_QUEUE_CONCURRENT; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ `extern` static `_dispatch_queue_attr_concurrent` from crate `issue_miri_3288_ice_symbolic_alignment_extern_static` is not supported by Miri + | + = help: this is likely not a bug in the program; it indicates that the program performed an operation that the interpreter does not support + = note: BACKTRACE: + = note: inside `main` at $DIR/issue-miri-3288-ice-symbolic-alignment-extern-static.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/pass/align_offset_symbolic.rs b/src/tools/miri/tests/pass/align_offset_symbolic.rs index 4df364bac7a..e96f11b1efa 100644 --- a/src/tools/miri/tests/pass/align_offset_symbolic.rs +++ b/src/tools/miri/tests/pass/align_offset_symbolic.rs @@ -1,6 +1,8 @@ //@compile-flags: -Zmiri-symbolic-alignment-check #![feature(strict_provenance)] +use std::mem; + fn test_align_to() { const N: usize = 4; let d = Box::new([0u32; N]); @@ -68,7 +70,7 @@ fn test_u64_array() { #[repr(align(8))] struct AlignToU64(T); - const BYTE_LEN: usize = std::mem::size_of::<[u64; 4]>(); + const BYTE_LEN: usize = mem::size_of::<[u64; 4]>(); type Data = AlignToU64<[u8; BYTE_LEN]>; fn example(data: &Data) { @@ -101,10 +103,29 @@ fn huge_align() { let _ = std::ptr::invalid::(SIZE).align_offset(SIZE); } +// This shows that we cannot store the promised alignment info in `AllocExtra`, +// since vtables do not have an `AllocExtra`. +fn vtable() { + #[cfg(target_pointer_width = "64")] + type TWOPTR = u128; + #[cfg(target_pointer_width = "32")] + type TWOPTR = u64; + + let ptr: &dyn Send = &0; + let parts: (*const (), *const u8) = unsafe { mem::transmute(ptr) }; + let vtable = parts.1 ; + let offset = vtable.align_offset(mem::align_of::()); + let _vtable_aligned = vtable.wrapping_add(offset) as *const [TWOPTR; 0]; + // FIXME: we can't actually do the access since vtable pointers act like zero-sized allocations. + // Enable the next line once https://github.com/rust-lang/rust/issues/117945 is implemented. + //let _place = unsafe { &*vtable_aligned }; +} + fn main() { test_align_to(); test_from_utf8(); test_u64_array(); test_cstr(); huge_align(); + vtable(); } From f1308783b4a7e613d60bddfa0f5474363c198289 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 6 Feb 2024 17:41:48 +0000 Subject: [PATCH 16/16] Make async closures test use async bound modifier --- .../async-closures/async-fn-mut-for-async-fn.rs | 6 ++---- .../async-closures/async-fn-once-for-async-fn.rs | 6 ++---- .../async-await/async-closures/auxiliary/block-on.rs | 2 +- tests/ui/async-await/async-closures/brand.rs | 5 ++--- tests/ui/async-await/async-closures/drop.rs | 6 ++---- tests/ui/async-await/async-closures/mangle.rs | 7 +++---- tests/ui/async-await/async-closures/wrong-fn-kind.rs | 8 +++----- .../ui/async-await/async-closures/wrong-fn-kind.stderr | 10 +++++----- 8 files changed, 20 insertions(+), 30 deletions(-) diff --git a/tests/ui/async-await/async-closures/async-fn-mut-for-async-fn.rs b/tests/ui/async-await/async-closures/async-fn-mut-for-async-fn.rs index 8d7dc6a276b..f73b43dd152 100644 --- a/tests/ui/async-await/async-closures/async-fn-mut-for-async-fn.rs +++ b/tests/ui/async-await/async-closures/async-fn-mut-for-async-fn.rs @@ -5,17 +5,15 @@ // FIXME(async_closures): When `fn_sig_for_fn_abi` is fixed, remove this. // ignore-pass (test emits codegen-time warnings) -#![feature(async_closure, async_fn_traits)] +#![feature(async_closure)] extern crate block_on; -use std::ops::AsyncFnMut; - fn main() { block_on::block_on(async { let x = async || {}; - async fn needs_async_fn_mut(mut x: impl AsyncFnMut()) { + async fn needs_async_fn_mut(mut x: impl async FnMut()) { x().await; } needs_async_fn_mut(x).await; diff --git a/tests/ui/async-await/async-closures/async-fn-once-for-async-fn.rs b/tests/ui/async-await/async-closures/async-fn-once-for-async-fn.rs index 4afc43fe6bd..0ba323a71cd 100644 --- a/tests/ui/async-await/async-closures/async-fn-once-for-async-fn.rs +++ b/tests/ui/async-await/async-closures/async-fn-once-for-async-fn.rs @@ -5,17 +5,15 @@ // FIXME(async_closures): When `fn_sig_for_fn_abi` is fixed, remove this. // ignore-pass (test emits codegen-time warnings) -#![feature(async_closure, async_fn_traits)] +#![feature(async_closure)] extern crate block_on; -use std::ops::AsyncFnOnce; - fn main() { block_on::block_on(async { let x = async || {}; - async fn needs_async_fn_once(x: impl AsyncFnOnce()) { + async fn needs_async_fn_once(x: impl async FnOnce()) { x().await; } needs_async_fn_once(x).await; diff --git a/tests/ui/async-await/async-closures/auxiliary/block-on.rs b/tests/ui/async-await/async-closures/auxiliary/block-on.rs index 3c27548b865..902e033cfe7 100644 --- a/tests/ui/async-await/async-closures/auxiliary/block-on.rs +++ b/tests/ui/async-await/async-closures/auxiliary/block-on.rs @@ -1,6 +1,6 @@ // edition: 2021 -#![feature(async_closure, noop_waker, async_fn_traits)] +#![feature(async_closure, noop_waker)] use std::future::Future; use std::pin::pin; diff --git a/tests/ui/async-await/async-closures/brand.rs b/tests/ui/async-await/async-closures/brand.rs index 3bda7737bb4..26d2ed5a6ef 100644 --- a/tests/ui/async-await/async-closures/brand.rs +++ b/tests/ui/async-await/async-closures/brand.rs @@ -2,19 +2,18 @@ // edition:2021 // build-pass -#![feature(async_closure, async_fn_traits)] +#![feature(async_closure)] extern crate block_on; use std::future::Future; use std::marker::PhantomData; -use std::ops::AsyncFn; struct S; struct B<'b>(PhantomData<&'b mut &'b mut ()>); impl S { - async fn q)>(self, f: F) { + async fn q)>(self, f: F) { f(B(PhantomData)).await; } } diff --git a/tests/ui/async-await/async-closures/drop.rs b/tests/ui/async-await/async-closures/drop.rs index 1b7f2f8a600..a243d20774d 100644 --- a/tests/ui/async-await/async-closures/drop.rs +++ b/tests/ui/async-await/async-closures/drop.rs @@ -3,13 +3,11 @@ // run-pass // check-run-results -#![feature(async_closure, async_fn_traits)] +#![feature(async_closure)] #![allow(unused)] extern crate block_on; -use std::ops::AsyncFnOnce; - struct DropMe(i32); impl Drop for DropMe { @@ -18,7 +16,7 @@ fn drop(&mut self) { } } -async fn call_once(f: impl AsyncFnOnce()) { +async fn call_once(f: impl async FnOnce()) { println!("before call"); let fut = Box::pin(f()); println!("after call"); diff --git a/tests/ui/async-await/async-closures/mangle.rs b/tests/ui/async-await/async-closures/mangle.rs index 98065c3c711..9d231d55176 100644 --- a/tests/ui/async-await/async-closures/mangle.rs +++ b/tests/ui/async-await/async-closures/mangle.rs @@ -8,20 +8,19 @@ // FIXME(async_closures): When `fn_sig_for_fn_abi` is fixed, remove this. // ignore-pass (test emits codegen-time warnings) -#![feature(async_closure, noop_waker, async_fn_traits)] +#![feature(async_closure, noop_waker)] extern crate block_on; use std::future::Future; -use std::ops::{AsyncFnMut, AsyncFnOnce}; use std::pin::pin; use std::task::*; -async fn call_mut(f: &mut impl AsyncFnMut()) { +async fn call_mut(f: &mut impl async FnMut()) { f().await; } -async fn call_once(f: impl AsyncFnOnce()) { +async fn call_once(f: impl async FnOnce()) { f().await; } 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 24828832531..f86cee3e070 100644 --- a/tests/ui/async-await/async-closures/wrong-fn-kind.rs +++ b/tests/ui/async-await/async-closures/wrong-fn-kind.rs @@ -2,17 +2,15 @@ // FIXME(async_closures): This needs a better error message! -#![feature(async_closure, async_fn_traits)] - -use std::ops::AsyncFn; +#![feature(async_closure)] fn main() { - fn needs_async_fn(_: impl AsyncFn()) {} + fn needs_async_fn(_: impl async Fn()) {} let mut x = 1; needs_async_fn(async || { //~^ ERROR i16: ops::async_function::internal_implementation_detail::AsyncFnKindHelper - // FIXME: Should say "closure is AsyncFnMut but it needs AsyncFn" or sth. + // FIXME: Should say "closure is `async FnMut` but it needs `async Fn`" or sth. x += 1; }); } 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 ef95e6a211c..4ef8484cc34 100644 --- a/tests/ui/async-await/async-closures/wrong-fn-kind.stderr +++ b/tests/ui/async-await/async-closures/wrong-fn-kind.stderr @@ -1,21 +1,21 @@ error[E0277]: the trait bound `i16: ops::async_function::internal_implementation_detail::AsyncFnKindHelper` is not satisfied - --> $DIR/wrong-fn-kind.rs:13:20 + --> $DIR/wrong-fn-kind.rs:11:20 | LL | needs_async_fn(async || { | _____--------------_^ | | | | | required by a bound introduced by this call LL | | -LL | | // FIXME: Should say "closure is AsyncFnMut but it needs AsyncFn" or sth. +LL | | // FIXME: Should say "closure is `async FnMut` but it needs `async Fn`" or sth. LL | | x += 1; LL | | }); | |_____^ the trait `ops::async_function::internal_implementation_detail::AsyncFnKindHelper` is not implemented for `i16` | note: required by a bound in `needs_async_fn` - --> $DIR/wrong-fn-kind.rs:10:31 + --> $DIR/wrong-fn-kind.rs:8:31 | -LL | fn needs_async_fn(_: impl AsyncFn()) {} - | ^^^^^^^^^ required by this bound in `needs_async_fn` +LL | fn needs_async_fn(_: impl async Fn()) {} + | ^^^^^^^^^^ required by this bound in `needs_async_fn` error: aborting due to 1 previous error