From 2de99ec787c80d4ba5ac4638d371a5bd8b752795 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Sat, 13 Jan 2024 10:31:09 +1100 Subject: [PATCH 01/14] Reformat `struct_span_code_err!`. --- compiler/rustc_errors/src/diagnostic_builder.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs index bd7c58d904e70..93b2dd61b059f 100644 --- a/compiler/rustc_errors/src/diagnostic_builder.rs +++ b/compiler/rustc_errors/src/diagnostic_builder.rs @@ -437,11 +437,7 @@ impl Drop for DiagnosticBuilder<'_, G> { #[macro_export] macro_rules! struct_span_code_err { ($dcx:expr, $span:expr, $code:ident, $($message:tt)*) => ({ - $dcx.struct_span_err( - $span, - format!($($message)*), - ) - .with_code($crate::error_code!($code)) + $dcx.struct_span_err($span, format!($($message)*)).with_code($crate::error_code!($code)) }) } From d71f535a6f39e82313b46ac3157c4ed9267a2e40 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Sat, 13 Jan 2024 13:11:56 +1100 Subject: [PATCH 02/14] Rework how diagnostic lints are stored. `Diagnostic::code` has the type `DiagnosticId`, which has `Error` and `Lint` variants. Plus `Diagnostic::is_lint` is a bool, which should be redundant w.r.t. `Diagnostic::code`. Seems simple. Except it's possible for a lint to have an error code, in which case its `code` field is recorded as `Error`, and `is_lint` is required to indicate that it's a lint. This is what happens with `derive(LintDiagnostic)` lints. Which means those lints don't have a lint name or a `has_future_breakage` field because those are stored in the `DiagnosticId::Lint`. It's all a bit messy and confused and seems unintentional. This commit: - removes `DiagnosticId`; - changes `Diagnostic::code` to `Option`, which means both errors and lints can straightforwardly have an error code; - changes `Diagnostic::is_lint` to `Option`, where `IsLint` is a new type containing a lint name and a `has_future_breakage` bool, so all lints can have those, error code or not. --- compiler/rustc_codegen_ssa/src/back/write.rs | 4 +- .../src/annotate_snippet_emitter_writer.rs | 12 ++--- compiler/rustc_errors/src/diagnostic.rs | 45 +++++++++---------- .../rustc_errors/src/diagnostic_builder.rs | 11 +++-- compiler/rustc_errors/src/emitter.rs | 17 ++++--- compiler/rustc_errors/src/json.rs | 37 +++++++-------- compiler/rustc_errors/src/lib.rs | 37 +++++++-------- .../src/check/compare_impl_item.rs | 4 +- .../src/structured_errors.rs | 4 +- .../missing_cast_for_variadic_arg.rs | 4 +- .../structured_errors/sized_unsized_cast.rs | 4 +- .../wrong_number_of_generic_args.rs | 6 +-- compiler/rustc_hir_typeck/src/expr.rs | 4 +- .../rustc_hir_typeck/src/fn_ctxt/checks.rs | 8 ++-- compiler/rustc_hir_typeck/src/lib.rs | 4 +- .../infer/error_reporting/need_type_info.rs | 4 +- .../src/diagnostics/diagnostic_builder.rs | 2 +- compiler/rustc_middle/src/lint.rs | 7 +-- compiler/rustc_mir_transform/src/errors.rs | 2 +- compiler/rustc_passes/src/entry.rs | 4 +- compiler/rustc_resolve/src/late.rs | 6 +-- compiler/rustc_resolve/src/lib.rs | 2 +- compiler/rustc_session/src/parse.rs | 5 +-- compiler/rustc_session/src/session.rs | 7 ++- 24 files changed, 110 insertions(+), 130 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index af1c659444601..5d497d4a1883a 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -14,7 +14,7 @@ use rustc_data_structures::memmap::Mmap; use rustc_data_structures::profiling::{SelfProfilerRef, VerboseTimingGuard}; use rustc_data_structures::sync::Lrc; use rustc_errors::emitter::Emitter; -use rustc_errors::{translation::Translate, DiagCtxt, DiagnosticId, FatalError, Level}; +use rustc_errors::{translation::Translate, DiagCtxt, FatalError, Level}; use rustc_errors::{DiagnosticBuilder, DiagnosticMessage, Style}; use rustc_fs_util::link_or_copy; use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; @@ -1000,7 +1000,7 @@ type DiagnosticArgName<'source> = Cow<'source, str>; struct Diagnostic { msgs: Vec<(DiagnosticMessage, Style)>, args: FxHashMap, rustc_errors::DiagnosticArgValue<'static>>, - code: Option, + code: Option, lvl: Level, } diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index 648c9118400e9..f0699a56f98ef 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -9,8 +9,8 @@ use crate::emitter::FileWithAnnotatedLines; use crate::snippet::Line; use crate::translation::{to_fluent_args, Translate}; use crate::{ - CodeSuggestion, Diagnostic, DiagnosticId, DiagnosticMessage, Emitter, FluentBundle, - LazyFallbackBundle, Level, MultiSpan, Style, SubDiagnostic, + CodeSuggestion, Diagnostic, DiagnosticMessage, Emitter, FluentBundle, LazyFallbackBundle, + Level, MultiSpan, Style, SubDiagnostic, }; use annotate_snippets::{Annotation, AnnotationType, Renderer, Slice, Snippet, SourceAnnotation}; use rustc_data_structures::sync::Lrc; @@ -127,7 +127,7 @@ impl AnnotateSnippetEmitter { level: &Level, messages: &[(DiagnosticMessage, Style)], args: &FluentArgs<'_>, - code: &Option, + code: &Option, msp: &MultiSpan, _children: &[SubDiagnostic], _suggestions: &[CodeSuggestion], @@ -181,11 +181,7 @@ impl AnnotateSnippetEmitter { let snippet = Snippet { title: Some(Annotation { label: Some(&message), - id: code.as_ref().map(|c| match c { - DiagnosticId::Error(val) | DiagnosticId::Lint { name: val, .. } => { - val.as_str() - } - }), + id: code.as_deref(), annotation_type: annotation_type_for_level(*level), }), footer: vec![], diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 786aced5b4f91..4934bc2450c79 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -104,7 +104,7 @@ pub struct Diagnostic { pub(crate) level: Level, pub messages: Vec<(DiagnosticMessage, Style)>, - pub code: Option, + pub code: Option, pub span: MultiSpan, pub children: Vec, pub suggestions: Result, SuggestionsDisabled>, @@ -115,9 +115,9 @@ pub struct Diagnostic { /// `span` if there is one. Otherwise, it is `DUMMY_SP`. pub sort_span: Span, - /// If diagnostic is from Lint, custom hash function ignores notes - /// otherwise hash is based on the all the fields - pub is_lint: bool, + /// If diagnostic is from Lint, custom hash function ignores children. + /// Otherwise hash is based on the all the fields. + pub is_lint: Option, /// With `-Ztrack_diagnostics` enabled, /// we print where in rustc this error was emitted. @@ -146,13 +146,11 @@ impl fmt::Display for DiagnosticLocation { } #[derive(Clone, Debug, PartialEq, Eq, Hash, Encodable, Decodable)] -pub enum DiagnosticId { - Error(String), - Lint { - name: String, - /// Indicates whether this lint should show up in cargo's future breakage report. - has_future_breakage: bool, - }, +pub struct IsLint { + /// The lint name. + pub(crate) name: String, + /// Indicates whether this lint should show up in cargo's future breakage report. + has_future_breakage: bool, } /// A "sub"-diagnostic attached to a parent diagnostic. @@ -231,7 +229,7 @@ impl Diagnostic { suggestions: Ok(vec![]), args: Default::default(), sort_span: DUMMY_SP, - is_lint: false, + is_lint: None, emitted_at: DiagnosticLocation::caller(), } } @@ -288,16 +286,13 @@ impl Diagnostic { /// Indicates whether this diagnostic should show up in cargo's future breakage report. pub(crate) fn has_future_breakage(&self) -> bool { - match self.code { - Some(DiagnosticId::Lint { has_future_breakage, .. }) => has_future_breakage, - _ => false, - } + matches!(self.is_lint, Some(IsLint { has_future_breakage: true, .. })) } pub(crate) fn is_force_warn(&self) -> bool { match self.level { Level::ForceWarning(_) => { - assert!(self.is_lint); + assert!(self.is_lint.is_some()); true } _ => false, @@ -893,12 +888,12 @@ impl Diagnostic { self } - pub fn is_lint(&mut self) -> &mut Self { - self.is_lint = true; + pub fn is_lint(&mut self, name: String, has_future_breakage: bool) -> &mut Self { + self.is_lint = Some(IsLint { name, has_future_breakage }); self } - pub fn code(&mut self, s: DiagnosticId) -> &mut Self { + pub fn code(&mut self, s: String) -> &mut Self { self.code = Some(s); self } @@ -908,8 +903,8 @@ impl Diagnostic { self } - pub fn get_code(&self) -> Option { - self.code.clone() + pub fn get_code(&self) -> Option<&str> { + self.code.as_deref() } pub fn primary_message(&mut self, msg: impl Into) -> &mut Self { @@ -995,7 +990,8 @@ impl Diagnostic { &Level, &[(DiagnosticMessage, Style)], Vec<(&Cow<'static, str>, &DiagnosticArgValue<'static>)>, - &Option, + &Option, + &Option, &MultiSpan, &Result, SuggestionsDisabled>, Option<&[SubDiagnostic]>, @@ -1005,9 +1001,10 @@ impl Diagnostic { &self.messages, self.args().collect(), &self.code, + &self.is_lint, &self.span, &self.suggestions, - (if self.is_lint { None } else { Some(&self.children) }), + (if self.is_lint.is_some() { None } else { Some(&self.children) }), ) } } diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs index 93b2dd61b059f..87e2d295c7f4c 100644 --- a/compiler/rustc_errors/src/diagnostic_builder.rs +++ b/compiler/rustc_errors/src/diagnostic_builder.rs @@ -1,8 +1,8 @@ use crate::diagnostic::IntoDiagnosticArg; use crate::{DiagCtxt, Level, MultiSpan, StashKey}; use crate::{ - Diagnostic, DiagnosticId, DiagnosticMessage, DiagnosticStyledString, ErrorGuaranteed, - ExplicitBug, SubdiagnosticMessage, + Diagnostic, DiagnosticMessage, DiagnosticStyledString, ErrorGuaranteed, ExplicitBug, + SubdiagnosticMessage, }; use rustc_lint_defs::Applicability; use rustc_span::source_map::Spanned; @@ -395,8 +395,11 @@ impl<'a, G: EmissionGuarantee> DiagnosticBuilder<'a, G> { forward!((span, with_span)( sp: impl Into, )); + forward!((is_lint, with_is_lint)( + name: String, has_future_breakage: bool, + )); forward!((code, with_code)( - s: DiagnosticId, + s: String, )); forward!((arg, with_arg)( name: impl Into>, arg: impl IntoDiagnosticArg, @@ -443,5 +446,5 @@ macro_rules! struct_span_code_err { #[macro_export] macro_rules! error_code { - ($code:ident) => {{ $crate::DiagnosticId::Error(stringify!($code).to_owned()) }}; + ($code:ident) => {{ stringify!($code).to_owned() }}; } diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 987832e6937bd..23efdaea0ebb8 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -16,9 +16,9 @@ use crate::snippet::{ use crate::styled_buffer::StyledBuffer; use crate::translation::{to_fluent_args, Translate}; use crate::{ - diagnostic::DiagnosticLocation, CodeSuggestion, DiagCtxt, Diagnostic, DiagnosticId, - DiagnosticMessage, FluentBundle, LazyFallbackBundle, Level, MultiSpan, SubDiagnostic, - SubstitutionHighlight, SuggestionStyle, TerminalUrl, + diagnostic::DiagnosticLocation, CodeSuggestion, DiagCtxt, Diagnostic, DiagnosticMessage, + FluentBundle, LazyFallbackBundle, Level, MultiSpan, SubDiagnostic, SubstitutionHighlight, + SuggestionStyle, TerminalUrl, }; use rustc_lint_defs::pluralize; @@ -1309,7 +1309,7 @@ impl HumanEmitter { msp: &MultiSpan, msgs: &[(DiagnosticMessage, Style)], args: &FluentArgs<'_>, - code: &Option, + code: &Option, level: &Level, max_line_num_len: usize, is_secondary: bool, @@ -1336,14 +1336,13 @@ impl HumanEmitter { buffer.append(0, level.to_str(), Style::Level(*level)); label_width += level.to_str().len(); } - // only render error codes, not lint codes - if let Some(DiagnosticId::Error(ref code)) = *code { + if let Some(code) = code { buffer.append(0, "[", Style::Level(*level)); let code = if let TerminalUrl::Yes = self.terminal_url { let path = "https://doc.rust-lang.org/error_codes"; - format!("\x1b]8;;{path}/{code}.html\x07{code}\x1b]8;;\x07") + Cow::Owned(format!("\x1b]8;;{path}/{code}.html\x07{code}\x1b]8;;\x07")) } else { - code.clone() + Cow::Borrowed(code) }; buffer.append(0, &code, Style::Level(*level)); buffer.append(0, "]", Style::Level(*level)); @@ -2077,7 +2076,7 @@ impl HumanEmitter { level: &Level, messages: &[(DiagnosticMessage, Style)], args: &FluentArgs<'_>, - code: &Option, + code: &Option, span: &MultiSpan, children: &[SubDiagnostic], suggestions: &[CodeSuggestion], diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 87bf9c234564b..51b064f4c617a 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -15,10 +15,9 @@ use termcolor::{ColorSpec, WriteColor}; use crate::emitter::{should_show_source_code, Emitter, HumanReadableErrorType}; use crate::registry::Registry; use crate::translation::{to_fluent_args, Translate}; -use crate::DiagnosticId; use crate::{ - CodeSuggestion, FluentBundle, LazyFallbackBundle, MultiSpan, SpanLabel, SubDiagnostic, - TerminalUrl, + diagnostic::IsLint, CodeSuggestion, FluentBundle, LazyFallbackBundle, MultiSpan, SpanLabel, + SubDiagnostic, TerminalUrl, }; use rustc_lint_defs::Applicability; @@ -301,7 +300,8 @@ struct DiagnosticSpanMacroExpansion { #[derive(Serialize)] struct DiagnosticCode { - /// The code itself. + /// The error code (e.g. "E1234"), if the diagnostic has one. Or the lint + /// name, if it's a lint without an error code. code: String, /// An explanation for the code. explanation: Option<&'static str>, @@ -399,9 +399,21 @@ impl Diagnostic { 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: DiagnosticCode::map_opt_string(diag.code.clone(), je), + code, level: diag.level.to_str(), spans: DiagnosticSpan::from_multispan(&diag.span, &args, je), children: diag @@ -592,18 +604,3 @@ impl DiagnosticSpanLine { .unwrap_or_else(|_| vec![]) } } - -impl DiagnosticCode { - fn map_opt_string(s: Option, je: &JsonEmitter) -> Option { - s.map(|s| { - let s = match s { - DiagnosticId::Error(s) => s, - DiagnosticId::Lint { name, .. } => name, - }; - let je_result = - je.registry.as_ref().map(|registry| registry.try_find_description(&s)).unwrap(); - - DiagnosticCode { code: s, explanation: je_result.ok() } - }) - } -} diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 63391a0faa6b3..141547b537dea 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -30,7 +30,7 @@ extern crate tracing; extern crate self as rustc_errors; pub use diagnostic::{ - AddToDiagnostic, DecorateLint, Diagnostic, DiagnosticArg, DiagnosticArgValue, DiagnosticId, + AddToDiagnostic, DecorateLint, Diagnostic, DiagnosticArg, DiagnosticArgValue, DiagnosticStyledString, IntoDiagnosticArg, SubDiagnostic, }; pub use diagnostic_builder::{ @@ -442,13 +442,13 @@ struct DiagCtxtInner { /// This is used for the `good_path_delayed_bugs` check. suppressed_expected_diag: bool, - /// This set contains the `DiagnosticId` of all emitted diagnostics to avoid + /// This set contains the code of all emitted diagnostics to avoid /// emitting the same diagnostic with extended help (`--teach`) twice, which /// would be unnecessary repetition. - taught_diagnostics: FxHashSet, + taught_diagnostics: FxHashSet, /// Used to suggest rustc --explain `` - emitted_diagnostic_codes: FxIndexSet, + emitted_diagnostic_codes: FxIndexSet, /// This set contains a hash of every diagnostic that has been emitted by /// this `DiagCtxt`. These hashes is used to avoid emitting the same error @@ -676,7 +676,7 @@ impl DiagCtxt { let key = (span.with_parent(None), key); if diag.is_error() { - if diag.is_lint { + if diag.is_lint.is_some() { inner.lint_err_count += 1; } else { inner.err_count += 1; @@ -695,7 +695,7 @@ impl DiagCtxt { let key = (span.with_parent(None), key); let diag = inner.stashed_diagnostics.remove(&key)?; if diag.is_error() { - if diag.is_lint { + if diag.is_lint.is_some() { inner.lint_err_count -= 1; } else { inner.err_count -= 1; @@ -715,9 +715,7 @@ impl DiagCtxt { /// Construct a builder at the `Warning` level at the given `span` and with the `msg`. /// - /// Attempting to `.emit()` the builder will only emit if either: - /// * `can_emit_warnings` is `true` - /// * `is_force_warn` was set in `DiagnosticId::Lint` + /// An `emit` call on the builder will only emit if `can_emit_warnings` is `true`. #[rustc_lint_diagnostics] #[track_caller] pub fn struct_span_warn( @@ -730,9 +728,7 @@ impl DiagCtxt { /// Construct a builder at the `Warning` level with the `msg`. /// - /// Attempting to `.emit()` the builder will only emit if either: - /// * `can_emit_warnings` is `true` - /// * `is_force_warn` was set in `DiagnosticId::Lint` + /// An `emit` call on the builder will only emit if `can_emit_warnings` is `true`. #[rustc_lint_diagnostics] #[track_caller] pub fn struct_warn(&self, msg: impl Into) -> DiagnosticBuilder<'_, ()> { @@ -1011,11 +1007,12 @@ impl DiagCtxt { let mut error_codes = inner .emitted_diagnostic_codes .iter() - .filter_map(|x| match &x { - DiagnosticId::Error(s) if registry.try_find_description(s).is_ok() => { - Some(s.clone()) + .filter_map(|code| { + if registry.try_find_description(code).is_ok().clone() { + Some(code.clone()) + } else { + None } - _ => None, }) .collect::>(); if !error_codes.is_empty() { @@ -1058,8 +1055,8 @@ impl DiagCtxt { /// /// Used to suppress emitting the same error multiple times with extended explanation when /// calling `-Zteach`. - pub fn must_teach(&self, code: &DiagnosticId) -> bool { - self.inner.borrow_mut().taught_diagnostics.insert(code.clone()) + pub fn must_teach(&self, code: &str) -> bool { + self.inner.borrow_mut().taught_diagnostics.insert(code.to_string()) } pub fn force_print_diagnostic(&self, db: Diagnostic) { @@ -1231,7 +1228,7 @@ impl DiagCtxtInner { for diag in diags { // Decrement the count tracking the stash; emitting will increment it. if diag.is_error() { - if diag.is_lint { + if diag.is_lint.is_some() { self.lint_err_count -= 1; } else { self.err_count -= 1; @@ -1363,7 +1360,7 @@ impl DiagCtxtInner { self.has_printed = true; } if diagnostic.is_error() { - if diagnostic.is_lint { + if diagnostic.is_lint.is_some() { self.lint_err_count += 1; } else { self.err_count += 1; diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index 469e7a6a13c50..774feb94f7dc2 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -2,7 +2,7 @@ use super::potentially_plural_count; use crate::errors::LifetimesOrBoundsMismatchOnTrait; use hir::def_id::{DefId, DefIdMap, LocalDefId}; use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet}; -use rustc_errors::{pluralize, struct_span_code_err, Applicability, DiagnosticId, ErrorGuaranteed}; +use rustc_errors::{pluralize, struct_span_code_err, Applicability, ErrorGuaranteed}; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::intravisit; @@ -1382,7 +1382,7 @@ fn compare_number_of_generics<'tcx>( kind = kind, ), ); - err.code(DiagnosticId::Error("E0049".into())); + err.code("E0049".into()); let msg = format!("expected {trait_count} {kind} parameter{}", pluralize!(trait_count),); diff --git a/compiler/rustc_hir_analysis/src/structured_errors.rs b/compiler/rustc_hir_analysis/src/structured_errors.rs index 04d04304e70f9..b061d6371388f 100644 --- a/compiler/rustc_hir_analysis/src/structured_errors.rs +++ b/compiler/rustc_hir_analysis/src/structured_errors.rs @@ -6,13 +6,13 @@ pub use self::{ missing_cast_for_variadic_arg::*, sized_unsized_cast::*, wrong_number_of_generic_args::*, }; -use rustc_errors::{DiagnosticBuilder, DiagnosticId}; +use rustc_errors::DiagnosticBuilder; use rustc_session::Session; pub trait StructuredDiagnostic<'tcx> { fn session(&self) -> &Session; - fn code(&self) -> DiagnosticId; + fn code(&self) -> String; fn diagnostic(&self) -> DiagnosticBuilder<'tcx> { let err = self.diagnostic_common(); diff --git a/compiler/rustc_hir_analysis/src/structured_errors/missing_cast_for_variadic_arg.rs b/compiler/rustc_hir_analysis/src/structured_errors/missing_cast_for_variadic_arg.rs index 634904e32715d..6f4435db41143 100644 --- a/compiler/rustc_hir_analysis/src/structured_errors/missing_cast_for_variadic_arg.rs +++ b/compiler/rustc_hir_analysis/src/structured_errors/missing_cast_for_variadic_arg.rs @@ -1,5 +1,5 @@ use crate::{errors, structured_errors::StructuredDiagnostic}; -use rustc_errors::{DiagnosticBuilder, DiagnosticId}; +use rustc_errors::DiagnosticBuilder; use rustc_middle::ty::{Ty, TypeVisitableExt}; use rustc_session::Session; use rustc_span::Span; @@ -16,7 +16,7 @@ impl<'tcx> StructuredDiagnostic<'tcx> for MissingCastForVariadicArg<'tcx, '_> { self.sess } - fn code(&self) -> DiagnosticId { + fn code(&self) -> String { rustc_errors::error_code!(E0617) } diff --git a/compiler/rustc_hir_analysis/src/structured_errors/sized_unsized_cast.rs b/compiler/rustc_hir_analysis/src/structured_errors/sized_unsized_cast.rs index c68d74969a548..19cac4a38aaab 100644 --- a/compiler/rustc_hir_analysis/src/structured_errors/sized_unsized_cast.rs +++ b/compiler/rustc_hir_analysis/src/structured_errors/sized_unsized_cast.rs @@ -1,5 +1,5 @@ use crate::{errors, structured_errors::StructuredDiagnostic}; -use rustc_errors::{DiagnosticBuilder, DiagnosticId}; +use rustc_errors::DiagnosticBuilder; use rustc_middle::ty::{Ty, TypeVisitableExt}; use rustc_session::Session; use rustc_span::Span; @@ -16,7 +16,7 @@ impl<'tcx> StructuredDiagnostic<'tcx> for SizedUnsizedCast<'tcx> { self.sess } - fn code(&self) -> DiagnosticId { + fn code(&self) -> String { rustc_errors::error_code!(E0607) } diff --git a/compiler/rustc_hir_analysis/src/structured_errors/wrong_number_of_generic_args.rs b/compiler/rustc_hir_analysis/src/structured_errors/wrong_number_of_generic_args.rs index 6657e3fd872fa..ba81e7f1f8165 100644 --- a/compiler/rustc_hir_analysis/src/structured_errors/wrong_number_of_generic_args.rs +++ b/compiler/rustc_hir_analysis/src/structured_errors/wrong_number_of_generic_args.rs @@ -1,7 +1,5 @@ use crate::structured_errors::StructuredDiagnostic; -use rustc_errors::{ - pluralize, Applicability, Diagnostic, DiagnosticBuilder, DiagnosticId, MultiSpan, -}; +use rustc_errors::{pluralize, Applicability, Diagnostic, DiagnosticBuilder, MultiSpan}; use rustc_hir as hir; use rustc_middle::ty::{self as ty, AssocItems, AssocKind, TyCtxt}; use rustc_session::Session; @@ -1107,7 +1105,7 @@ impl<'tcx> StructuredDiagnostic<'tcx> for WrongNumberOfGenericArgs<'_, 'tcx> { self.tcx.sess } - fn code(&self) -> DiagnosticId { + fn code(&self) -> String { rustc_errors::error_code!(E0107) } diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index af47455c16dff..ac8701a33d44e 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -26,7 +26,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_errors::{ pluralize, struct_span_code_err, AddToDiagnostic, Applicability, Diagnostic, DiagnosticBuilder, - DiagnosticId, ErrorGuaranteed, StashKey, + ErrorGuaranteed, StashKey, }; use rustc_hir as hir; use rustc_hir::def::{CtorKind, DefKind, Res}; @@ -941,7 +941,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } let mut err = self.dcx().struct_span_err(op_span, "invalid left-hand side of assignment"); - err.code(DiagnosticId::Error(err_code.into())); + err.code(err_code.into()); err.span_label(lhs.span, "cannot assign to this expression"); self.comes_from_while_condition(lhs.hir_id, |expr| { diff --git a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs index 8cd5ed3494b18..3fe42d1495a56 100644 --- a/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs +++ b/compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs @@ -11,9 +11,7 @@ use crate::{ }; use rustc_ast as ast; use rustc_data_structures::fx::FxIndexSet; -use rustc_errors::{ - pluralize, Applicability, Diagnostic, DiagnosticId, ErrorGuaranteed, MultiSpan, StashKey, -}; +use rustc_errors::{pluralize, Applicability, Diagnostic, ErrorGuaranteed, MultiSpan, StashKey}; use rustc_hir as hir; use rustc_hir::def::{CtorOf, DefKind, Res}; use rustc_hir::def_id::DefId; @@ -678,7 +676,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pluralize!("was", provided_args.len()) ), ); - err.code(DiagnosticId::Error(err_code.to_owned())); + err.code(err_code.to_owned()); err.multipart_suggestion_verbose( "wrap these arguments in parentheses to construct a tuple", vec![ @@ -828,7 +826,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { pluralize!("was", provided_args.len()) ), ) - .with_code(DiagnosticId::Error(err_code.to_owned())) + .with_code(err_code.to_owned()) }; // As we encounter issues, keep track of what we want to provide for the suggestion diff --git a/compiler/rustc_hir_typeck/src/lib.rs b/compiler/rustc_hir_typeck/src/lib.rs index 36dd06d944c4e..67c35d717a1ec 100644 --- a/compiler/rustc_hir_typeck/src/lib.rs +++ b/compiler/rustc_hir_typeck/src/lib.rs @@ -52,7 +52,7 @@ use crate::expectation::Expectation; use crate::fn_ctxt::RawTy; use crate::gather_locals::GatherLocalsVisitor; use rustc_data_structures::unord::UnordSet; -use rustc_errors::{struct_span_code_err, DiagnosticId, ErrorGuaranteed}; +use rustc_errors::{struct_span_code_err, ErrorGuaranteed}; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::intravisit::Visitor; @@ -369,7 +369,7 @@ fn report_unexpected_variant_res( let err = tcx .dcx() .struct_span_err(span, format!("expected {expected}, found {res_descr} `{path_str}`")) - .with_code(DiagnosticId::Error(err_code.into())); + .with_code(err_code.into()); match res { Res::Def(DefKind::Fn | DefKind::AssocFn, _) if err_code == "E0164" => { let patterns_url = "https://doc.rust-lang.org/book/ch18-00-patterns.html"; diff --git a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs index 4aefadf590dd4..e85c940f1b55c 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs @@ -41,8 +41,8 @@ pub enum TypeAnnotationNeeded { E0284, } -impl Into for TypeAnnotationNeeded { - fn into(self) -> rustc_errors::DiagnosticId { +impl Into for TypeAnnotationNeeded { + fn into(self) -> String { match self { Self::E0282 => rustc_errors::error_code!(E0282), Self::E0283 => rustc_errors::error_code!(E0283), diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs index fb908fe2db155..0b7ecff8148b3 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic_builder.rs @@ -211,7 +211,7 @@ impl DiagnosticDeriveVariantBuilder { let code = nested.parse::()?; tokens.extend(quote! { - diag.code(rustc_errors::DiagnosticId::Error(#code.to_string())); + diag.code(#code.to_string()); }); } else { span_err(path.span().unwrap(), "unknown argument") diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index c5e4dfaf19ee3..4ab16cf19ba93 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/compiler/rustc_middle/src/lint.rs @@ -2,7 +2,7 @@ use std::cmp; use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::sorted_map::SortedMap; -use rustc_errors::{Diagnostic, DiagnosticBuilder, DiagnosticId, DiagnosticMessage, MultiSpan}; +use rustc_errors::{Diagnostic, DiagnosticBuilder, DiagnosticMessage, MultiSpan}; use rustc_hir::{HirId, ItemLocalId}; use rustc_session::lint::{ builtin::{self, FORBIDDEN_LINT_GROUPS}, @@ -322,8 +322,6 @@ pub fn struct_lint_level( err.span(span); } - err.is_lint(); - // If this code originates in a foreign macro, aka something that this crate // did not itself author, then it's likely that there's nothing this crate // can do about it. We probably want to skip the lint entirely. @@ -351,8 +349,7 @@ pub fn struct_lint_level( // suppressed the lint due to macros. err.primary_message(msg); - let name = lint.name_lower(); - err.code(DiagnosticId::Lint { name, has_future_breakage }); + err.is_lint(lint.name_lower(), has_future_breakage); // Lint diagnostics that are covered by the expect level will not be emitted outside // the compiler. It is therefore not necessary to add any information for the user. diff --git a/compiler/rustc_mir_transform/src/errors.rs b/compiler/rustc_mir_transform/src/errors.rs index 446f13feff089..2ee660ddc9be1 100644 --- a/compiler/rustc_mir_transform/src/errors.rs +++ b/compiler/rustc_mir_transform/src/errors.rs @@ -66,7 +66,7 @@ impl<'a, G: EmissionGuarantee> IntoDiagnostic<'a, G> for RequiresUnsafe { #[track_caller] fn into_diagnostic(self, dcx: &'a DiagCtxt, level: Level) -> DiagnosticBuilder<'a, G> { let mut diag = DiagnosticBuilder::new(dcx, level, fluent::mir_transform_requires_unsafe); - diag.code(rustc_errors::DiagnosticId::Error("E0133".to_string())); + diag.code("E0133".to_string()); diag.span(self.span); diag.span_label(self.span, self.details.label()); let desc = dcx.eagerly_translate_to_string(self.details.label(), [].into_iter()); diff --git a/compiler/rustc_passes/src/entry.rs b/compiler/rustc_passes/src/entry.rs index a9d0e0d2dd3e5..25ca685cbfade 100644 --- a/compiler/rustc_passes/src/entry.rs +++ b/compiler/rustc_passes/src/entry.rs @@ -180,8 +180,8 @@ fn no_main_err(tcx: TyCtxt<'_>, visitor: &EntryContext<'_>) { Default::default() }); let main_def_opt = tcx.resolutions(()).main_def; - let diagnostic_id = error_code!(E0601); - let add_teach_note = tcx.sess.teach(&diagnostic_id); + let code = error_code!(E0601); + let add_teach_note = tcx.sess.teach(&code); // The file may be empty, which leads to the diagnostic machinery not emitting this // note. This is a relatively simple way to detect that case and emit a span-less // note instead. diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index b9e603a499266..ccf90fbff22dc 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -16,7 +16,7 @@ use rustc_ast::ptr::P; use rustc_ast::visit::{self, AssocCtxt, BoundKind, FnCtxt, FnKind, Visitor}; use rustc_ast::*; use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap}; -use rustc_errors::{Applicability, DiagnosticArgValue, DiagnosticId, IntoDiagnosticArg}; +use rustc_errors::{Applicability, DiagnosticArgValue, IntoDiagnosticArg}; use rustc_hir::def::Namespace::{self, *}; use rustc_hir::def::{self, CtorKind, DefKind, LifetimeRes, NonMacroAttrKind, PartialRes, PerNS}; use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID, LOCAL_CRATE}; @@ -531,7 +531,7 @@ impl<'a> PathSource<'a> { } } - fn error_code(self, has_unexpected_resolution: bool) -> DiagnosticId { + fn error_code(self, has_unexpected_resolution: bool) -> String { use rustc_errors::error_code; match (self, has_unexpected_resolution) { (PathSource::Trait(_), true) => error_code!(E0404), @@ -3759,7 +3759,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> { swap(&mut err.span, &mut parent_err.span); err.children = take(&mut parent_err.children); err.sort_span = parent_err.sort_span; - err.is_lint = parent_err.is_lint; + err.is_lint = parent_err.is_lint.clone(); // merge the parent's suggestions with the typo suggestions fn append_result(res1: &mut Result, E>, res2: Result, E>) { diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 0adea65ee58ea..196f28d878ddd 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -257,7 +257,7 @@ enum ResolutionError<'a> { kind: &'static str, trait_path: String, trait_item_span: Span, - code: rustc_errors::DiagnosticId, + code: String, }, /// Error E0201: multiple impl items for the same trait item. TraitImplDuplicate { name: Symbol, trait_item_span: Span, old_span: Span }, diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index c629c9884c8e1..16036e5be38c7 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/compiler/rustc_session/src/parse.rs @@ -15,8 +15,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::sync::{AppendOnlyVec, Lock, Lrc}; use rustc_errors::{emitter::SilentEmitter, DiagCtxt}; use rustc_errors::{ - fallback_fluent_bundle, Diagnostic, DiagnosticBuilder, DiagnosticId, DiagnosticMessage, - MultiSpan, StashKey, + fallback_fluent_bundle, Diagnostic, DiagnosticBuilder, DiagnosticMessage, MultiSpan, StashKey, }; use rustc_feature::{find_feature_issue, GateIssue, UnstableFeatures}; use rustc_span::edition::Edition; @@ -148,7 +147,7 @@ pub fn feature_warn_issue( // Decorate this as a future-incompatibility lint as in rustc_middle::lint::struct_lint_level let lint = UNSTABLE_SYNTAX_PRE_EXPANSION; let future_incompatible = lint.future_incompatible.as_ref().unwrap(); - err.code(DiagnosticId::Lint { name: lint.name_lower(), has_future_breakage: false }); + err.is_lint(lint.name_lower(), /* has_future_breakage */ false); err.warn(lint.desc); err.note(format!("for more information, see {}", future_incompatible.reference)); diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 1c3d5a3cb8811..3f6c70a18d395 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -20,9 +20,8 @@ use rustc_errors::emitter::{DynEmitter, HumanEmitter, HumanReadableErrorType}; use rustc_errors::json::JsonEmitter; use rustc_errors::registry::Registry; use rustc_errors::{ - error_code, fallback_fluent_bundle, DiagCtxt, DiagnosticBuilder, DiagnosticId, - DiagnosticMessage, ErrorGuaranteed, FatalAbort, FluentBundle, IntoDiagnostic, - LazyFallbackBundle, TerminalUrl, + error_code, fallback_fluent_bundle, DiagCtxt, DiagnosticBuilder, DiagnosticMessage, + ErrorGuaranteed, FatalAbort, FluentBundle, IntoDiagnostic, LazyFallbackBundle, TerminalUrl, }; use rustc_macros::HashStable_Generic; pub use rustc_span::def_id::StableCrateId; @@ -906,7 +905,7 @@ impl Session { CodegenUnits::Default(16) } - pub fn teach(&self, code: &DiagnosticId) -> bool { + pub fn teach(&self, code: &str) -> bool { self.opts.unstable_opts.teach && self.dcx().must_teach(code) } From 7df054b5a94dd8612e38c933f724faf7bbf3fffd Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Sun, 14 Jan 2024 16:18:44 -0800 Subject: [PATCH 03/14] Change return type of unstable `Waker::noop()` from `Waker` to `&Waker`. The advantage of this is that it does not need to be assigned to a variable to be used in a `Context` creation, which is the most common thing to want to do with a noop waker. If an owned noop waker is desired, it can be created by cloning, but the reverse is harder. Alternatively, both versions could be provided, like `futures::task::noop_waker()` and `futures::task::noop_waker_ref()`, but that seems to me to be API clutter for a very small benefit, whereas having the `&'static` reference available is a large benefit. Previous discussion on the tracking issue starting here: https://github.com/rust-lang/rust/issues/98286#issuecomment-1862159766 --- library/core/src/task/wake.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/library/core/src/task/wake.rs b/library/core/src/task/wake.rs index 9c41b8b4f46a6..c35d19789f7e5 100644 --- a/library/core/src/task/wake.rs +++ b/library/core/src/task/wake.rs @@ -329,12 +329,14 @@ impl Waker { Waker { waker } } - /// Creates a new `Waker` that does nothing when `wake` is called. + /// Returns a reference to a `Waker` that does nothing when used. /// /// This is mostly useful for writing tests that need a [`Context`] to poll /// some futures, but are not expecting those futures to wake the waker or /// do not need to do anything specific if it happens. /// + /// If an owned `Waker` is needed, `clone()` this one. + /// /// # Examples /// /// ``` @@ -343,8 +345,7 @@ impl Waker { /// use std::future::Future; /// use std::task; /// - /// let waker = task::Waker::noop(); - /// let mut cx = task::Context::from_waker(&waker); + /// let mut cx = task::Context::from_waker(task::Waker::noop()); /// /// let mut future = Box::pin(async { 10 }); /// assert_eq!(future.as_mut().poll(&mut cx), task::Poll::Ready(10)); @@ -352,7 +353,7 @@ impl Waker { #[inline] #[must_use] #[unstable(feature = "noop_waker", issue = "98286")] - pub const fn noop() -> Waker { + pub const fn noop() -> &'static Waker { const VTABLE: RawWakerVTable = RawWakerVTable::new( // Cloning just returns a new no-op raw waker |_| RAW, @@ -364,8 +365,9 @@ impl Waker { |_| {}, ); const RAW: RawWaker = RawWaker::new(ptr::null(), &VTABLE); + const WAKER_REF: &Waker = &Waker { waker: RAW }; - Waker { waker: RAW } + WAKER_REF } /// Get a reference to the underlying [`RawWaker`]. From 219b00df0b5fd68cd5abc4c553b9529066927f60 Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Sun, 14 Jan 2024 16:25:43 -0800 Subject: [PATCH 04/14] Remove unnecessary `let`s and borrowing from `Waker::noop()` usage. `Waker::noop()` now returns a `&'static Waker` reference, so it can be passed directly to `Context` creation with no temporary lifetime issue. --- library/core/tests/async_iter/mod.rs | 3 +-- src/tools/miri/tests/pass/async-fn.rs | 3 +-- src/tools/miri/tests/pass/dyn-star.rs | 3 +-- .../miri/tests/pass/future-self-referential.rs | 6 ++---- .../miri/tests/pass/issues/issue-miri-2068.rs | 3 +-- .../tests/pass/move-data-across-await-point.rs | 3 +-- tests/coverage/async.coverage | 3 +-- tests/coverage/async.rs | 3 +-- tests/coverage/async2.coverage | 3 +-- tests/coverage/async2.rs | 3 +-- tests/coverage/async_block.coverage | 3 +-- tests/coverage/async_block.rs | 3 +-- tests/coverage/closure_macro_async.cov-map | 16 ++++++++-------- tests/coverage/closure_macro_async.coverage | 3 +-- tests/coverage/closure_macro_async.rs | 6 +++--- tests/ui/async-await/for-await-passthrough.rs | 3 +-- tests/ui/async-await/for-await.rs | 3 +-- .../in-trait/async-default-fn-overridden.rs | 3 +-- .../dont-project-to-specializable-projection.rs | 3 +-- ...nt-project-to-specializable-projection.stderr | 2 +- tests/ui/coroutine/async-gen-yield-ty-is-unit.rs | 3 +-- tests/ui/coroutine/async_gen_fn_iter.rs | 3 +-- tests/ui/dyn-star/dispatch-on-pin-mut.rs | 5 ++--- 23 files changed, 34 insertions(+), 55 deletions(-) diff --git a/library/core/tests/async_iter/mod.rs b/library/core/tests/async_iter/mod.rs index 0c30bd1dfeac9..4f425d7286d09 100644 --- a/library/core/tests/async_iter/mod.rs +++ b/library/core/tests/async_iter/mod.rs @@ -7,8 +7,7 @@ fn into_async_iter() { let async_iter = async_iter::from_iter(0..3); let mut async_iter = pin!(async_iter.into_async_iter()); - let waker = core::task::Waker::noop(); - let mut cx = &mut core::task::Context::from_waker(&waker); + let mut cx = &mut core::task::Context::from_waker(core::task::Waker::noop()); assert_eq!(async_iter.as_mut().poll_next(&mut cx), Poll::Ready(Some(0))); assert_eq!(async_iter.as_mut().poll_next(&mut cx), Poll::Ready(Some(1))); diff --git a/src/tools/miri/tests/pass/async-fn.rs b/src/tools/miri/tests/pass/async-fn.rs index 6c92735df0af8..13400c88c7112 100644 --- a/src/tools/miri/tests/pass/async-fn.rs +++ b/src/tools/miri/tests/pass/async-fn.rs @@ -76,8 +76,7 @@ async fn uninhabited_variant() { fn run_fut(fut: impl Future) -> T { use std::task::{Context, Poll, Waker}; - let waker = Waker::noop(); - let mut context = Context::from_waker(&waker); + let mut context = Context::from_waker(Waker::noop()); let mut pinned = Box::pin(fut); loop { diff --git a/src/tools/miri/tests/pass/dyn-star.rs b/src/tools/miri/tests/pass/dyn-star.rs index 8e26c4850fa8e..dab589b465181 100644 --- a/src/tools/miri/tests/pass/dyn-star.rs +++ b/src/tools/miri/tests/pass/dyn-star.rs @@ -93,8 +93,7 @@ fn dispatch_on_pin_mut() { let mut fut = async_main(); // Poll loop, just to test the future... - let waker = Waker::noop(); - let ctx = &mut Context::from_waker(&waker); + let ctx = &mut Context::from_waker(Waker::noop()); loop { match unsafe { Pin::new_unchecked(&mut fut).poll(ctx) } { diff --git a/src/tools/miri/tests/pass/future-self-referential.rs b/src/tools/miri/tests/pass/future-self-referential.rs index 38cb700fd584f..8aeb26a7a957b 100644 --- a/src/tools/miri/tests/pass/future-self-referential.rs +++ b/src/tools/miri/tests/pass/future-self-referential.rs @@ -77,8 +77,7 @@ impl Future for DoStuff { } fn run_fut(fut: impl Future) -> T { - let waker = Waker::noop(); - let mut context = Context::from_waker(&waker); + let mut context = Context::from_waker(Waker::noop()); let mut pinned = pin!(fut); loop { @@ -90,8 +89,7 @@ fn run_fut(fut: impl Future) -> T { } fn self_referential_box() { - let waker = Waker::noop(); - let cx = &mut Context::from_waker(&waker); + let cx = &mut Context::from_waker(Waker::noop()); async fn my_fut() -> i32 { let val = 10; diff --git a/src/tools/miri/tests/pass/issues/issue-miri-2068.rs b/src/tools/miri/tests/pass/issues/issue-miri-2068.rs index f18c4a3a0655f..ccee2221e2958 100644 --- a/src/tools/miri/tests/pass/issues/issue-miri-2068.rs +++ b/src/tools/miri/tests/pass/issues/issue-miri-2068.rs @@ -6,8 +6,7 @@ use std::task::{Context, Poll, Waker}; pub fn fuzzing_block_on>(fut: F) -> O { let mut fut = std::pin::pin!(fut); - let waker = Waker::noop(); - let mut context = Context::from_waker(&waker); + let mut context = Context::from_waker(Waker::noop()); loop { match fut.as_mut().poll(&mut context) { Poll::Ready(v) => return v, diff --git a/src/tools/miri/tests/pass/move-data-across-await-point.rs b/src/tools/miri/tests/pass/move-data-across-await-point.rs index 9bea6ea574209..1a93a6bf66497 100644 --- a/src/tools/miri/tests/pass/move-data-across-await-point.rs +++ b/src/tools/miri/tests/pass/move-data-across-await-point.rs @@ -56,8 +56,7 @@ fn data_moved() { fn run_fut(fut: impl Future) -> T { use std::task::{Context, Poll, Waker}; - let waker = Waker::noop(); - let mut context = Context::from_waker(&waker); + let mut context = Context::from_waker(Waker::noop()); let mut pinned = Box::pin(fut); loop { diff --git a/tests/coverage/async.coverage b/tests/coverage/async.coverage index 015e03d5165fa..2316145885003 100644 --- a/tests/coverage/async.coverage +++ b/tests/coverage/async.coverage @@ -117,8 +117,7 @@ LL| | #[coverage(off)] LL| | pub fn block_on(mut future: F) -> F::Output { LL| | let mut future = pin!(future); - LL| | let waker = Waker::noop(); - LL| | let mut context = Context::from_waker(&waker); + LL| | let mut context = Context::from_waker(Waker::noop()); LL| | LL| | loop { LL| | if let Poll::Ready(val) = future.as_mut().poll(&mut context) { diff --git a/tests/coverage/async.rs b/tests/coverage/async.rs index abc9e5f7f6467..df29682968d3e 100644 --- a/tests/coverage/async.rs +++ b/tests/coverage/async.rs @@ -110,8 +110,7 @@ mod executor { #[coverage(off)] pub fn block_on(mut future: F) -> F::Output { let mut future = pin!(future); - let waker = Waker::noop(); - let mut context = Context::from_waker(&waker); + let mut context = Context::from_waker(Waker::noop()); loop { if let Poll::Ready(val) = future.as_mut().poll(&mut context) { diff --git a/tests/coverage/async2.coverage b/tests/coverage/async2.coverage index acd83de94934d..a69eefe72cbcf 100644 --- a/tests/coverage/async2.coverage +++ b/tests/coverage/async2.coverage @@ -41,8 +41,7 @@ LL| | #[coverage(off)] LL| | pub fn block_on(mut future: F) -> F::Output { LL| | let mut future = pin!(future); - LL| | let waker = Waker::noop(); - LL| | let mut context = Context::from_waker(&waker); + LL| | let mut context = Context::from_waker(Waker::noop()); LL| | LL| | loop { LL| | if let Poll::Ready(val) = future.as_mut().poll(&mut context) { diff --git a/tests/coverage/async2.rs b/tests/coverage/async2.rs index 393573f7dc955..ae83f0103e64d 100644 --- a/tests/coverage/async2.rs +++ b/tests/coverage/async2.rs @@ -39,8 +39,7 @@ mod executor { #[coverage(off)] pub fn block_on(mut future: F) -> F::Output { let mut future = pin!(future); - let waker = Waker::noop(); - let mut context = Context::from_waker(&waker); + let mut context = Context::from_waker(Waker::noop()); loop { if let Poll::Ready(val) = future.as_mut().poll(&mut context) { diff --git a/tests/coverage/async_block.coverage b/tests/coverage/async_block.coverage index 297397ca26c8d..0e24b80124f48 100644 --- a/tests/coverage/async_block.coverage +++ b/tests/coverage/async_block.coverage @@ -24,8 +24,7 @@ LL| | #[coverage(off)] LL| | pub fn block_on(mut future: F) -> F::Output { LL| | let mut future = pin!(future); - LL| | let waker = Waker::noop(); - LL| | let mut context = Context::from_waker(&waker); + LL| | let mut context = Context::from_waker(Waker::noop()); LL| | LL| | loop { LL| | if let Poll::Ready(val) = future.as_mut().poll(&mut context) { diff --git a/tests/coverage/async_block.rs b/tests/coverage/async_block.rs index 9d8647bf1f208..f94bcfe319371 100644 --- a/tests/coverage/async_block.rs +++ b/tests/coverage/async_block.rs @@ -23,8 +23,7 @@ mod executor { #[coverage(off)] pub fn block_on(mut future: F) -> F::Output { let mut future = pin!(future); - let waker = Waker::noop(); - let mut context = Context::from_waker(&waker); + let mut context = Context::from_waker(Waker::noop()); loop { if let Poll::Ready(val) = future.as_mut().poll(&mut context) { diff --git a/tests/coverage/closure_macro_async.cov-map b/tests/coverage/closure_macro_async.cov-map index 14b1525ca0e6c..5f5e064427890 100644 --- a/tests/coverage/closure_macro_async.cov-map +++ b/tests/coverage/closure_macro_async.cov-map @@ -1,28 +1,28 @@ Function name: closure_macro_async::load_configuration_files -Raw bytes (9): 0x[01, 01, 00, 01, 01, 1e, 01, 02, 02] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 1f, 01, 02, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 0 Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 30, 1) to (start + 2, 2) +- Code(Counter(0)) at (prev + 31, 1) to (start + 2, 2) Function name: closure_macro_async::test -Raw bytes (9): 0x[01, 01, 00, 01, 01, 22, 01, 00, 2b] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 23, 01, 00, 2b] Number of files: 1 - file 0 => global file 1 Number of expressions: 0 Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 34, 1) to (start + 0, 43) +- Code(Counter(0)) at (prev + 35, 1) to (start + 0, 43) Function name: closure_macro_async::test::{closure#0} -Raw bytes (43): 0x[01, 01, 02, 01, 05, 05, 02, 07, 01, 22, 2b, 01, 21, 02, 02, 09, 00, 0f, 05, 00, 12, 00, 13, 02, 00, 12, 00, 13, 05, 00, 54, 00, 55, 02, 02, 09, 02, 0b, 07, 03, 01, 00, 02] +Raw bytes (43): 0x[01, 01, 02, 01, 05, 05, 02, 07, 01, 23, 2b, 01, 21, 02, 02, 09, 00, 0f, 05, 00, 12, 00, 13, 02, 00, 12, 00, 13, 05, 00, 54, 00, 55, 02, 02, 09, 02, 0b, 07, 03, 01, 00, 02] Number of files: 1 - file 0 => global file 1 Number of expressions: 2 - expression 0 operands: lhs = Counter(0), rhs = Counter(1) - expression 1 operands: lhs = Counter(1), rhs = Expression(0, Sub) Number of file 0 mappings: 7 -- Code(Counter(0)) at (prev + 34, 43) to (start + 1, 33) +- Code(Counter(0)) at (prev + 35, 43) to (start + 1, 33) - Code(Expression(0, Sub)) at (prev + 2, 9) to (start + 0, 15) = (c0 - c1) - Code(Counter(1)) at (prev + 0, 18) to (start + 0, 19) @@ -35,10 +35,10 @@ Number of file 0 mappings: 7 = (c1 + (c0 - c1)) Function name: closure_macro_async::test::{closure#0}::{closure#0} -Raw bytes (9): 0x[01, 01, 00, 01, 01, 24, 12, 00, 54] +Raw bytes (9): 0x[01, 01, 00, 01, 01, 25, 12, 00, 54] Number of files: 1 - file 0 => global file 1 Number of expressions: 0 Number of file 0 mappings: 1 -- Code(Counter(0)) at (prev + 36, 18) to (start + 0, 84) +- Code(Counter(0)) at (prev + 37, 18) to (start + 0, 84) diff --git a/tests/coverage/closure_macro_async.coverage b/tests/coverage/closure_macro_async.coverage index 2c9bd4ac97a10..2336d4d0528cc 100644 --- a/tests/coverage/closure_macro_async.coverage +++ b/tests/coverage/closure_macro_async.coverage @@ -54,8 +54,7 @@ LL| | #[coverage(off)] LL| | pub fn block_on(mut future: F) -> F::Output { LL| | let mut future = pin!(future); - LL| | let waker = Waker::noop(); - LL| | let mut context = Context::from_waker(&waker); + LL| | let mut context = Context::from_waker(Waker::noop()); LL| | LL| | loop { LL| | if let Poll::Ready(val) = future.as_mut().poll(&mut context) { diff --git a/tests/coverage/closure_macro_async.rs b/tests/coverage/closure_macro_async.rs index a7f0cabb4c239..116cb72b0f375 100644 --- a/tests/coverage/closure_macro_async.rs +++ b/tests/coverage/closure_macro_async.rs @@ -15,7 +15,8 @@ macro_rules! bail { macro_rules! on_error { ($value:expr, $error_message:expr) => { - $value.or_else(|e| { // FIXME(85000): no coverage in closure macros + $value.or_else(|e| { + // FIXME(85000): no coverage in closure macros let message = format!($error_message, e); if message.len() > 0 { println!("{}", message); @@ -53,8 +54,7 @@ mod executor { #[coverage(off)] pub fn block_on(mut future: F) -> F::Output { let mut future = pin!(future); - let waker = Waker::noop(); - let mut context = Context::from_waker(&waker); + let mut context = Context::from_waker(Waker::noop()); loop { if let Poll::Ready(val) = future.as_mut().poll(&mut context) { diff --git a/tests/ui/async-await/for-await-passthrough.rs b/tests/ui/async-await/for-await-passthrough.rs index 7fa133aaedcd7..b1a382958a157 100644 --- a/tests/ui/async-await/for-await-passthrough.rs +++ b/tests/ui/async-await/for-await-passthrough.rs @@ -25,8 +25,7 @@ async fn real_main() { fn main() { let future = real_main(); - let waker = std::task::Waker::noop(); - let mut cx = &mut core::task::Context::from_waker(&waker); + let mut cx = &mut core::task::Context::from_waker(std::task::Waker::noop()); let mut future = core::pin::pin!(future); while let core::task::Poll::Pending = future.as_mut().poll(&mut cx) {} } diff --git a/tests/ui/async-await/for-await.rs b/tests/ui/async-await/for-await.rs index 6345ceb0c2798..00dbdfb2389d5 100644 --- a/tests/ui/async-await/for-await.rs +++ b/tests/ui/async-await/for-await.rs @@ -17,8 +17,7 @@ async fn real_main() { fn main() { let future = real_main(); - let waker = std::task::Waker::noop(); - let mut cx = &mut core::task::Context::from_waker(&waker); + let mut cx = &mut core::task::Context::from_waker(std::task::Waker::noop()); let mut future = core::pin::pin!(future); while let core::task::Poll::Pending = future.as_mut().poll(&mut cx) {} } diff --git a/tests/ui/async-await/in-trait/async-default-fn-overridden.rs b/tests/ui/async-await/in-trait/async-default-fn-overridden.rs index 491dfcc6ae0fd..8c01f1bddefd6 100644 --- a/tests/ui/async-await/in-trait/async-default-fn-overridden.rs +++ b/tests/ui/async-await/in-trait/async-default-fn-overridden.rs @@ -40,8 +40,7 @@ fn main() { let mut fut = pin!(async_main()); // Poll loop, just to test the future... - let waker = Waker::noop(); - let ctx = &mut Context::from_waker(&waker); + let ctx = &mut Context::from_waker(Waker::noop()); loop { match fut.as_mut().poll(ctx) { diff --git a/tests/ui/async-await/in-trait/dont-project-to-specializable-projection.rs b/tests/ui/async-await/in-trait/dont-project-to-specializable-projection.rs index f21abf012ba99..e2fd9f9dfea4c 100644 --- a/tests/ui/async-await/in-trait/dont-project-to-specializable-projection.rs +++ b/tests/ui/async-await/in-trait/dont-project-to-specializable-projection.rs @@ -43,8 +43,7 @@ fn main() { let mut fut = pin!(async_main()); // Poll loop, just to test the future... - let waker = Waker::noop(); - let ctx = &mut Context::from_waker(&waker); + let ctx = &mut Context::from_waker(Waker::noop()); loop { match fut.as_mut().poll(ctx) { diff --git a/tests/ui/async-await/in-trait/dont-project-to-specializable-projection.stderr b/tests/ui/async-await/in-trait/dont-project-to-specializable-projection.stderr index 0560cd9c5fe11..b7336485eb84d 100644 --- a/tests/ui/async-await/in-trait/dont-project-to-specializable-projection.stderr +++ b/tests/ui/async-await/in-trait/dont-project-to-specializable-projection.stderr @@ -21,7 +21,7 @@ LL | default async fn foo(_: T) -> &'static str { = note: specialization behaves in inconsistent and surprising ways with async functions in traits, and for now is disallowed error[E0599]: no method named `poll` found for struct `Pin<&mut impl Future>` in the current scope - --> $DIR/dont-project-to-specializable-projection.rs:50:28 + --> $DIR/dont-project-to-specializable-projection.rs:49:28 | LL | match fut.as_mut().poll(ctx) { | ^^^^ method not found in `Pin<&mut impl Future>` diff --git a/tests/ui/coroutine/async-gen-yield-ty-is-unit.rs b/tests/ui/coroutine/async-gen-yield-ty-is-unit.rs index aac74d3eacba8..80c0b69a6f7ed 100644 --- a/tests/ui/coroutine/async-gen-yield-ty-is-unit.rs +++ b/tests/ui/coroutine/async-gen-yield-ty-is-unit.rs @@ -11,7 +11,6 @@ async gen fn gen_fn() -> &'static str { pub fn main() { let async_iterator = pin!(gen_fn()); - let waker = Waker::noop(); - let ctx = &mut Context::from_waker(&waker); + let ctx = &mut Context::from_waker(Waker::noop()); async_iterator.poll_next(ctx); } diff --git a/tests/ui/coroutine/async_gen_fn_iter.rs b/tests/ui/coroutine/async_gen_fn_iter.rs index ec6464d004877..604156b4d373f 100644 --- a/tests/ui/coroutine/async_gen_fn_iter.rs +++ b/tests/ui/coroutine/async_gen_fn_iter.rs @@ -74,8 +74,7 @@ fn main() { let mut fut = pin!(async_main()); // Poll loop, just to test the future... - let waker = Waker::noop(); - let ctx = &mut Context::from_waker(&waker); + let ctx = &mut Context::from_waker(Waker::noop()); loop { match fut.as_mut().poll(ctx) { diff --git a/tests/ui/dyn-star/dispatch-on-pin-mut.rs b/tests/ui/dyn-star/dispatch-on-pin-mut.rs index c4ae279e6c11e..151aa9092fbe1 100644 --- a/tests/ui/dyn-star/dispatch-on-pin-mut.rs +++ b/tests/ui/dyn-star/dispatch-on-pin-mut.rs @@ -19,15 +19,14 @@ async fn async_main() { // ------------------------------------------------------------------------- // // Implementation Details Below... -use std::task::*; use std::pin::pin; +use std::task::*; fn main() { let mut fut = pin!(async_main()); // Poll loop, just to test the future... - let waker = Waker::noop(); - let ctx = &mut Context::from_waker(&waker); + let ctx = &mut Context::from_waker(Waker::noop()); loop { match fut.as_mut().poll(ctx) { From 9a8f117d7b20e5503b8499112c26076cbbf570eb Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 15 Jan 2024 15:49:58 +0000 Subject: [PATCH 05/14] Don't create the array type twice --- compiler/rustc_hir_typeck/src/expr.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index af47455c16dff..a08f36ddfda15 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -1526,13 +1526,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.check_repeat_element_needs_copy_bound(element, count, element_ty); - self.register_wf_obligation( - Ty::new_array_with_const_len(tcx, t, count).into(), - expr.span, - traits::WellFormed(None), - ); + let ty = Ty::new_array_with_const_len(tcx, t, count); + + self.register_wf_obligation(ty.into(), expr.span, traits::WellFormed(None)); - Ty::new_array_with_const_len(tcx, t, count) + ty } fn check_repeat_element_needs_copy_bound( From 7052188680347106a703b10d2779ba4b0994eb52 Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Mon, 15 Jan 2024 09:31:59 -0800 Subject: [PATCH 06/14] Document why `Waker::noop()`'s implementation is the shape it is. --- library/core/src/task/wake.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/library/core/src/task/wake.rs b/library/core/src/task/wake.rs index c35d19789f7e5..077852b0120c4 100644 --- a/library/core/src/task/wake.rs +++ b/library/core/src/task/wake.rs @@ -354,6 +354,11 @@ impl Waker { #[must_use] #[unstable(feature = "noop_waker", issue = "98286")] pub const fn noop() -> &'static Waker { + // Ideally all this data would be explicitly `static` because it is used by reference and + // only ever needs one copy. But `const fn`s (and `const` items) cannot refer to statics, + // even though their values can be promoted to static. (That might change; see #119618.) + // An alternative would be a `pub static NOOP: &Waker`, but associated static items are not + // currently allowed either, and making it non-associated would be unergonomic. const VTABLE: RawWakerVTable = RawWakerVTable::new( // Cloning just returns a new no-op raw waker |_| RAW, From 290651be72ef3c33c47ac73093b26f752e213f6d Mon Sep 17 00:00:00 2001 From: lcnr Date: Tue, 16 Jan 2024 11:06:42 +0100 Subject: [PATCH 07/14] don't store const var origins for known vars --- .../infer/error_reporting/need_type_info.rs | 33 ++++--- compiler/rustc_infer/src/infer/freshen.rs | 10 +-- compiler/rustc_infer/src/infer/fudge.rs | 11 ++- compiler/rustc_infer/src/infer/mod.rs | 21 ++--- .../rustc_infer/src/infer/relate/combine.rs | 26 +++--- .../src/infer/relate/generalize.rs | 15 ++-- compiler/rustc_infer/src/infer/resolve.rs | 90 +------------------ compiler/rustc_middle/src/infer/unify_key.rs | 29 +++--- 8 files changed, 70 insertions(+), 165 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs index 4aefadf590dd4..6e1889d3538da 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs @@ -13,7 +13,9 @@ use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::{Body, Closure, Expr, ExprKind, FnRetTy, HirId, Local, LocalSource}; use rustc_middle::hir::nested_filter; -use rustc_middle::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKind}; +use rustc_middle::infer::unify_key::{ + ConstVariableOrigin, ConstVariableOriginKind, ConstVariableValue, +}; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow}; use rustc_middle::ty::print::{FmtPrinter, PrettyPrinter, Print, Printer}; use rustc_middle::ty::{self, InferConst}; @@ -178,17 +180,23 @@ fn fmt_printer<'a, 'tcx>(infcx: &'a InferCtxt<'tcx>, ns: Namespace) -> FmtPrinte } }; printer.ty_infer_name_resolver = Some(Box::new(ty_getter)); - let const_getter = move |ct_vid| { - if infcx.probe_const_var(ct_vid).is_ok() { + let const_getter = move |ct_vid| match infcx + .inner + .borrow_mut() + .const_unification_table() + .probe_value(ct_vid) + { + ConstVariableValue::Known { value: _ } => { warn!("resolved const var in error message"); - } - if let ConstVariableOriginKind::ConstParameterDefinition(name, _) = - infcx.inner.borrow_mut().const_unification_table().probe_value(ct_vid).origin.kind - { - return Some(name); - } else { None } + ConstVariableValue::Unknown { origin, universe: _ } => { + if let ConstVariableOriginKind::ConstParameterDefinition(name, _) = origin.kind { + return Some(name); + } else { + None + } + } }; printer.const_infer_name_resolver = Some(Box::new(const_getter)); printer @@ -303,7 +311,12 @@ impl<'tcx> InferCtxt<'tcx> { GenericArgKind::Const(ct) => { if let ty::ConstKind::Infer(InferConst::Var(vid)) = ct.kind() { let origin = - self.inner.borrow_mut().const_unification_table().probe_value(vid).origin; + match self.inner.borrow_mut().const_unification_table().probe_value(vid) { + ConstVariableValue::Known { value } => { + bug!("resolved infer var: {vid:?} {value}") + } + ConstVariableValue::Unknown { origin, universe: _ } => origin, + }; if let ConstVariableOriginKind::ConstParameterDefinition(name, def_id) = origin.kind { diff --git a/compiler/rustc_infer/src/infer/freshen.rs b/compiler/rustc_infer/src/infer/freshen.rs index c7cab048db1ba..d256994d8d1fd 100644 --- a/compiler/rustc_infer/src/infer/freshen.rs +++ b/compiler/rustc_infer/src/infer/freshen.rs @@ -146,14 +146,8 @@ impl<'a, 'tcx> TypeFolder> for TypeFreshener<'a, 'tcx> { fn fold_const(&mut self, ct: ty::Const<'tcx>) -> ty::Const<'tcx> { match ct.kind() { ty::ConstKind::Infer(ty::InferConst::Var(v)) => { - let opt_ct = self - .infcx - .inner - .borrow_mut() - .const_unification_table() - .probe_value(v) - .val - .known(); + let opt_ct = + self.infcx.inner.borrow_mut().const_unification_table().probe_value(v).known(); self.freshen_const(opt_ct, ty::InferConst::Var(v), ty::InferConst::Fresh, ct.ty()) } ty::ConstKind::Infer(ty::InferConst::EffectVar(v)) => { diff --git a/compiler/rustc_infer/src/infer/fudge.rs b/compiler/rustc_infer/src/infer/fudge.rs index 8ca97ae1b8e5d..99033922bdf76 100644 --- a/compiler/rustc_infer/src/infer/fudge.rs +++ b/compiler/rustc_infer/src/infer/fudge.rs @@ -1,4 +1,4 @@ -use rustc_middle::infer::unify_key::ConstVidKey; +use rustc_middle::infer::unify_key::{ConstVariableOriginKind, ConstVariableValue, ConstVidKey}; use rustc_middle::ty::fold::{TypeFoldable, TypeFolder, TypeSuperFoldable}; use rustc_middle::ty::{self, ConstVid, FloatVid, IntVid, RegionVid, Ty, TyCtxt, TyVid}; @@ -28,10 +28,17 @@ fn const_vars_since_snapshot<'tcx>( snapshot_var_len: usize, ) -> (Range, Vec) { let range = vars_since_snapshot(table, snapshot_var_len); + ( range.start.vid..range.end.vid, (range.start.index()..range.end.index()) - .map(|index| table.probe_value(ConstVid::from_u32(index)).origin) + .map(|index| match table.probe_value(ConstVid::from_u32(index)) { + ConstVariableValue::Known { value: _ } => ConstVariableOrigin { + kind: ConstVariableOriginKind::MiscVariable, + span: rustc_span::DUMMY_SP, + }, + ConstVariableValue::Unknown { origin, universe: _ } => origin, + }) .collect(), ) } diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index e164041c5991e..002aad19c4981 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -23,8 +23,8 @@ use rustc_data_structures::unify as ut; 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::{ConstVarValue, ConstVariableValue, EffectVarValue}; 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}; @@ -1086,7 +1086,7 @@ impl<'tcx> InferCtxt<'tcx> { .inner .borrow_mut() .const_unification_table() - .new_key(ConstVarValue { origin, val: ConstVariableValue::Unknown { universe } }) + .new_key(ConstVariableValue::Unknown { origin, universe }) .vid; ty::Const::new_var(self.tcx, vid, ty) } @@ -1095,10 +1095,7 @@ impl<'tcx> InferCtxt<'tcx> { self.inner .borrow_mut() .const_unification_table() - .new_key(ConstVarValue { - origin, - val: ConstVariableValue::Unknown { universe: self.universe() }, - }) + .new_key(ConstVariableValue::Unknown { origin, universe: self.universe() }) .vid } @@ -1217,10 +1214,7 @@ impl<'tcx> InferCtxt<'tcx> { .inner .borrow_mut() .const_unification_table() - .new_key(ConstVarValue { - origin, - val: ConstVariableValue::Unknown { universe: self.universe() }, - }) + .new_key(ConstVariableValue::Unknown { origin, universe: self.universe() }) .vid; ty::Const::new_var( self.tcx, @@ -1410,9 +1404,9 @@ impl<'tcx> InferCtxt<'tcx> { } pub fn probe_const_var(&self, vid: ty::ConstVid) -> Result, ty::UniverseIndex> { - match self.inner.borrow_mut().const_unification_table().probe_value(vid).val { + match self.inner.borrow_mut().const_unification_table().probe_value(vid) { ConstVariableValue::Known { value } => Ok(value), - ConstVariableValue::Unknown { universe } => Err(universe), + ConstVariableValue::Unknown { origin: _, universe } => Err(universe), } } @@ -1709,7 +1703,7 @@ impl<'tcx> InferCtxt<'tcx> { // `ty::ConstKind::Infer(ty::InferConst::Var(v))`. // // Not `inlined_probe_value(v)` because this call site is colder. - match self.inner.borrow_mut().const_unification_table().probe_value(v).val { + match self.inner.borrow_mut().const_unification_table().probe_value(v) { ConstVariableValue::Unknown { .. } => false, ConstVariableValue::Known { .. } => true, } @@ -1876,7 +1870,6 @@ impl<'a, 'tcx> TypeFolder> for ShallowResolver<'a, 'tcx> { .borrow_mut() .const_unification_table() .probe_value(vid) - .val .known() .unwrap_or(ct), ty::ConstKind::Infer(InferConst::EffectVar(vid)) => self diff --git a/compiler/rustc_infer/src/infer/relate/combine.rs b/compiler/rustc_infer/src/infer/relate/combine.rs index 4b254fc7df518..9e1dab12b4d43 100644 --- a/compiler/rustc_infer/src/infer/relate/combine.rs +++ b/compiler/rustc_infer/src/infer/relate/combine.rs @@ -30,14 +30,12 @@ use super::sub::Sub; use crate::infer::{DefineOpaqueTypes, InferCtxt, TypeTrace}; use crate::traits::{Obligation, PredicateObligations}; use rustc_middle::infer::canonical::OriginalQueryValues; -use rustc_middle::infer::unify_key::{ConstVarValue, ConstVariableValue, EffectVarValue}; -use rustc_middle::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKind}; +use rustc_middle::infer::unify_key::{ConstVariableValue, EffectVarValue}; use rustc_middle::ty::error::{ExpectedFound, TypeError}; use rustc_middle::ty::relate::{RelateResult, TypeRelation}; use rustc_middle::ty::{self, InferConst, ToPredicate, Ty, TyCtxt, TypeVisitableExt}; use rustc_middle::ty::{AliasRelationDirection, TyVar}; use rustc_middle::ty::{IntType, UintType}; -use rustc_span::DUMMY_SP; #[derive(Clone)] pub struct CombineFields<'infcx, 'tcx> { @@ -328,8 +326,12 @@ impl<'tcx> InferCtxt<'tcx> { ct: ty::Const<'tcx>, param_env: ty::ParamEnv<'tcx>, ) -> RelateResult<'tcx, ty::Const<'tcx>> { - let span = - self.inner.borrow_mut().const_unification_table().probe_value(target_vid).origin.span; + let span = match self.inner.borrow_mut().const_unification_table().probe_value(target_vid) { + ConstVariableValue::Known { value } => { + bug!("instantiating a known const var: {target_vid:?} {value} {ct}") + } + ConstVariableValue::Unknown { origin, universe: _ } => origin.span, + }; // FIXME(generic_const_exprs): Occurs check failures for unevaluated // constants and generic expressions are not yet handled correctly. let Generalization { value_may_be_infer: value, needs_wf: _ } = generalize::generalize( @@ -340,16 +342,10 @@ impl<'tcx> InferCtxt<'tcx> { ty::Variance::Invariant, )?; - self.inner.borrow_mut().const_unification_table().union_value( - target_vid, - ConstVarValue { - origin: ConstVariableOrigin { - kind: ConstVariableOriginKind::ConstInference, - span: DUMMY_SP, - }, - val: ConstVariableValue::Known { value }, - }, - ); + self.inner + .borrow_mut() + .const_unification_table() + .union_value(target_vid, ConstVariableValue::Known { value }); Ok(value) } diff --git a/compiler/rustc_infer/src/infer/relate/generalize.rs b/compiler/rustc_infer/src/infer/relate/generalize.rs index 27d37fd93697e..417c8695e248b 100644 --- a/compiler/rustc_infer/src/infer/relate/generalize.rs +++ b/compiler/rustc_infer/src/infer/relate/generalize.rs @@ -3,7 +3,7 @@ use std::mem; use rustc_data_structures::sso::SsoHashMap; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_hir::def_id::DefId; -use rustc_middle::infer::unify_key::{ConstVarValue, ConstVariableValue}; +use rustc_middle::infer::unify_key::ConstVariableValue; use rustc_middle::ty::error::TypeError; use rustc_middle::ty::relate::{self, Relate, RelateResult, TypeRelation}; use rustc_middle::ty::visit::MaxUniverse; @@ -431,22 +431,19 @@ where let mut inner = self.infcx.inner.borrow_mut(); let variable_table = &mut inner.const_unification_table(); - let var_value = variable_table.probe_value(vid); - match var_value.val { + match variable_table.probe_value(vid) { ConstVariableValue::Known { value: u } => { drop(inner); self.relate(u, u) } - ConstVariableValue::Unknown { universe } => { + ConstVariableValue::Unknown { origin, universe } => { if self.for_universe.can_name(universe) { Ok(c) } else { let new_var_id = variable_table - .new_key(ConstVarValue { - origin: var_value.origin, - val: ConstVariableValue::Unknown { - universe: self.for_universe, - }, + .new_key(ConstVariableValue::Unknown { + origin, + universe: self.for_universe, }) .vid; Ok(ty::Const::new_var(self.tcx(), new_var_id, c.ty())) diff --git a/compiler/rustc_infer/src/infer/resolve.rs b/compiler/rustc_infer/src/infer/resolve.rs index f317ccee6918a..959b09031277c 100644 --- a/compiler/rustc_infer/src/infer/resolve.rs +++ b/compiler/rustc_infer/src/infer/resolve.rs @@ -1,12 +1,8 @@ -use super::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; -use super::{FixupError, FixupResult, InferCtxt, Span}; -use rustc_middle::infer::unify_key::{ConstVariableOrigin, ConstVariableOriginKind}; +use super::{FixupError, FixupResult, InferCtxt}; use rustc_middle::ty::fold::{FallibleTypeFolder, TypeFolder, TypeSuperFoldable}; -use rustc_middle::ty::visit::{TypeSuperVisitable, TypeVisitableExt, TypeVisitor}; +use rustc_middle::ty::visit::TypeVisitableExt; use rustc_middle::ty::{self, Const, InferConst, Ty, TyCtxt, TypeFoldable}; -use std::ops::ControlFlow; - /////////////////////////////////////////////////////////////////////////// // OPPORTUNISTIC VAR RESOLVER @@ -104,88 +100,6 @@ impl<'a, 'tcx> TypeFolder> for OpportunisticRegionResolver<'a, 'tcx } } -/////////////////////////////////////////////////////////////////////////// -// UNRESOLVED TYPE FINDER - -/// The unresolved type **finder** walks a type searching for -/// type variables that don't yet have a value. The first unresolved type is stored. -/// It does not construct the fully resolved type (which might -/// involve some hashing and so forth). -pub struct UnresolvedTypeOrConstFinder<'a, 'tcx> { - infcx: &'a InferCtxt<'tcx>, -} - -impl<'a, 'tcx> UnresolvedTypeOrConstFinder<'a, 'tcx> { - pub fn new(infcx: &'a InferCtxt<'tcx>) -> Self { - UnresolvedTypeOrConstFinder { infcx } - } -} - -impl<'a, 'tcx> TypeVisitor> for UnresolvedTypeOrConstFinder<'a, 'tcx> { - type BreakTy = (ty::Term<'tcx>, Option); - fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow { - let t = self.infcx.shallow_resolve(t); - if let ty::Infer(infer_ty) = *t.kind() { - // Since we called `shallow_resolve` above, this must - // be an (as yet...) unresolved inference variable. - let ty_var_span = if let ty::TyVar(ty_vid) = infer_ty { - let mut inner = self.infcx.inner.borrow_mut(); - let ty_vars = &inner.type_variables(); - if let TypeVariableOrigin { - kind: TypeVariableOriginKind::TypeParameterDefinition(_, _), - span, - } = ty_vars.var_origin(ty_vid) - { - Some(span) - } else { - None - } - } else { - None - }; - ControlFlow::Break((t.into(), ty_var_span)) - } else if !t.has_non_region_infer() { - // All const/type variables in inference types must already be resolved, - // no need to visit the contents. - ControlFlow::Continue(()) - } else { - // Otherwise, keep visiting. - t.super_visit_with(self) - } - } - - fn visit_const(&mut self, ct: ty::Const<'tcx>) -> ControlFlow { - let ct = self.infcx.shallow_resolve(ct); - if let ty::ConstKind::Infer(i) = ct.kind() { - // Since we called `shallow_resolve` above, this must - // be an (as yet...) unresolved inference variable. - let ct_var_span = if let ty::InferConst::Var(vid) = i { - let mut inner = self.infcx.inner.borrow_mut(); - let ct_vars = &mut inner.const_unification_table(); - if let ConstVariableOrigin { - span, - kind: ConstVariableOriginKind::ConstParameterDefinition(_, _), - } = ct_vars.probe_value(vid).origin - { - Some(span) - } else { - None - } - } else { - None - }; - ControlFlow::Break((ct.into(), ct_var_span)) - } else if !ct.has_non_region_infer() { - // All const/type variables in inference types must already be resolved, - // no need to visit the contents. - ControlFlow::Continue(()) - } else { - // Otherwise, keep visiting. - ct.super_visit_with(self) - } - } -} - /////////////////////////////////////////////////////////////////////////// // FULL TYPE RESOLUTION diff --git a/compiler/rustc_middle/src/infer/unify_key.rs b/compiler/rustc_middle/src/infer/unify_key.rs index 6e50e89404648..c35799ef47f2b 100644 --- a/compiler/rustc_middle/src/infer/unify_key.rs +++ b/compiler/rustc_middle/src/infer/unify_key.rs @@ -120,7 +120,7 @@ pub enum ConstVariableOriginKind { #[derive(Copy, Clone, Debug)] pub enum ConstVariableValue<'tcx> { Known { value: ty::Const<'tcx> }, - Unknown { universe: ty::UniverseIndex }, + Unknown { origin: ConstVariableOrigin, universe: ty::UniverseIndex }, } impl<'tcx> ConstVariableValue<'tcx> { @@ -134,12 +134,6 @@ impl<'tcx> ConstVariableValue<'tcx> { } } -#[derive(Copy, Clone, Debug)] -pub struct ConstVarValue<'tcx> { - pub origin: ConstVariableOrigin, - pub val: ConstVariableValue<'tcx>, -} - #[derive(PartialEq, Copy, Clone, Debug)] pub struct ConstVidKey<'tcx> { pub vid: ty::ConstVid, @@ -153,7 +147,7 @@ impl<'tcx> From for ConstVidKey<'tcx> { } impl<'tcx> UnifyKey for ConstVidKey<'tcx> { - type Value = ConstVarValue<'tcx>; + type Value = ConstVariableValue<'tcx>; #[inline] fn index(&self) -> u32 { self.vid.as_u32() @@ -167,23 +161,23 @@ impl<'tcx> UnifyKey for ConstVidKey<'tcx> { } } -impl<'tcx> UnifyValue for ConstVarValue<'tcx> { +impl<'tcx> UnifyValue for ConstVariableValue<'tcx> { type Error = NoError; fn unify_values(&value1: &Self, &value2: &Self) -> Result { - Ok(match (value1.val, value2.val) { + match (value1, value2) { (ConstVariableValue::Known { .. }, ConstVariableValue::Known { .. }) => { bug!("equating two const variables, both of which have known values") } // If one side is known, prefer that one. - (ConstVariableValue::Known { .. }, ConstVariableValue::Unknown { .. }) => value1, - (ConstVariableValue::Unknown { .. }, ConstVariableValue::Known { .. }) => value2, + (ConstVariableValue::Known { .. }, ConstVariableValue::Unknown { .. }) => Ok(value1), + (ConstVariableValue::Unknown { .. }, ConstVariableValue::Known { .. }) => Ok(value2), // If both sides are *unknown*, it hardly matters, does it? ( - ConstVariableValue::Unknown { universe: universe1 }, - ConstVariableValue::Unknown { universe: universe2 }, + ConstVariableValue::Unknown { origin, universe: universe1 }, + ConstVariableValue::Unknown { origin: _, universe: universe2 }, ) => { // If we unify two unbound variables, ?T and ?U, then whatever // value they wind up taking (which must be the same value) must @@ -191,12 +185,9 @@ impl<'tcx> UnifyValue for ConstVarValue<'tcx> { // universe is the minimum of the two universes, because that is // the one which contains the fewest names in scope. let universe = cmp::min(universe1, universe2); - ConstVarValue { - val: ConstVariableValue::Unknown { universe }, - origin: value1.origin, - } + Ok(ConstVariableValue::Unknown { origin, universe }) } - }) + } } } From 3599c18874b6c8cc74ca2228fe61b17731769771 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 16 Jan 2024 10:03:29 +0000 Subject: [PATCH 08/14] Skip dead code checks on items that failed typeck --- compiler/rustc_passes/src/dead.rs | 5 ++++- tests/ui/consts/const-eval/infinite_loop.rs | 10 +++++++++- tests/ui/consts/const-eval/infinite_loop.stderr | 6 +++--- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index ac2ca23ad417f..9422a5944881a 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -128,7 +128,10 @@ impl<'tcx> MarkSymbolVisitor<'tcx> { if let Some(def_id) = self.typeck_results().type_dependent_def_id(id) { self.check_def_id(def_id); } else { - bug!("no type-dependent def for method"); + assert!( + self.typeck_results().tainted_by_errors.is_some(), + "no type-dependent def for method" + ); } } diff --git a/tests/ui/consts/const-eval/infinite_loop.rs b/tests/ui/consts/const-eval/infinite_loop.rs index 2178149063791..9bdb9929becd3 100644 --- a/tests/ui/consts/const-eval/infinite_loop.rs +++ b/tests/ui/consts/const-eval/infinite_loop.rs @@ -1,7 +1,13 @@ +//! This test tests two things at once: +//! 1. we error if a const evaluation hits the deny-by-default lint limit +//! 2. we do not ICE on invalid follow-up code + +// compile-flags: -Z tiny-const-eval-limit + fn main() { // Tests the Collatz conjecture with an incorrect base case (0 instead of 1). // The value of `n` will loop indefinitely (4 - 2 - 1 - 4). - let _ = [(); { + let s = [(); { let mut n = 113383; // #20 in https://oeis.org/A006884 while n != 0 { //~^ ERROR is taking a long time @@ -9,4 +15,6 @@ fn main() { } n }]; + + s.nonexistent_method(); } diff --git a/tests/ui/consts/const-eval/infinite_loop.stderr b/tests/ui/consts/const-eval/infinite_loop.stderr index e7a0a96a1e605..37cd94bf7b7c7 100644 --- a/tests/ui/consts/const-eval/infinite_loop.stderr +++ b/tests/ui/consts/const-eval/infinite_loop.stderr @@ -1,5 +1,5 @@ error: constant evaluation is taking a long time - --> $DIR/infinite_loop.rs:6:9 + --> $DIR/infinite_loop.rs:12:9 | LL | / while n != 0 { LL | | @@ -10,9 +10,9 @@ LL | | } = note: this lint makes sure the compiler doesn't get stuck due to infinite loops in const eval. If your compilation actually takes a long time, you can safely allow the lint. help: the constant being evaluated - --> $DIR/infinite_loop.rs:4:18 + --> $DIR/infinite_loop.rs:10:18 | -LL | let _ = [(); { +LL | let s = [(); { | __________________^ LL | | let mut n = 113383; // #20 in https://oeis.org/A006884 LL | | while n != 0 { From 8bb1eaee64e170153d21e64cdb437f3d36acd194 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 14 Jan 2024 22:14:48 +0000 Subject: [PATCH 09/14] Introduce helper that deals with moving async args into the coroutine --- compiler/rustc_ast_lowering/src/item.rs | 363 +++++++++++++----------- 1 file changed, 193 insertions(+), 170 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index a3ff02f5f6954..c8e639b7a9663 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -1082,194 +1082,217 @@ impl<'hir> LoweringContext<'_, 'hir> { let (Some(coroutine_kind), Some(body)) = (coroutine_kind, body) else { return self.lower_fn_body_block(span, decl, body); }; - let closure_id = coroutine_kind.closure_id(); - self.lower_body(|this| { - let mut parameters: Vec> = Vec::new(); - let mut statements: Vec> = Vec::new(); - - // Async function parameters are lowered into the closure body so that they are - // captured and so that the drop order matches the equivalent non-async functions. - // - // from: - // - // async fn foo(: , : , : ) { - // - // } - // - // into: - // - // fn foo(__arg0: , __arg1: , __arg2: ) { - // async move { - // let __arg2 = __arg2; - // let = __arg2; - // let __arg1 = __arg1; - // let = __arg1; - // let __arg0 = __arg0; - // let = __arg0; - // drop-temps { } // see comments later in fn for details - // } - // } - // - // If `` is a simple ident, then it is lowered to a single - // `let = ;` statement as an optimization. - // - // Note that the body is embedded in `drop-temps`; an - // equivalent desugaring would be `return { - // };`. The key point is that we wish to drop all the - // let-bound variables and temporaries created in the body - // (and its tail expression!) before we drop the - // parameters (c.f. rust-lang/rust#64512). - for (index, parameter) in decl.inputs.iter().enumerate() { - let parameter = this.lower_param(parameter); - let span = parameter.pat.span; - - // Check if this is a binding pattern, if so, we can optimize and avoid adding a - // `let = __argN;` statement. In this case, we do not rename the parameter. - let (ident, is_simple_parameter) = match parameter.pat.kind { - hir::PatKind::Binding(hir::BindingAnnotation(ByRef::No, _), _, ident, _) => { - (ident, true) - } - // For `ref mut` or wildcard arguments, we can't reuse the binding, but - // we can keep the same name for the parameter. - // This lets rustdoc render it correctly in documentation. - hir::PatKind::Binding(_, _, ident, _) => (ident, false), - hir::PatKind::Wild => { - (Ident::with_dummy_span(rustc_span::symbol::kw::Underscore), false) - } - _ => { - // Replace the ident for bindings that aren't simple. - let name = format!("__arg{index}"); - let ident = Ident::from_str(&name); - - (ident, false) - } - }; - - let desugared_span = this.mark_span_with_reason(DesugaringKind::Async, span, None); - - // Construct a parameter representing `__argN: ` to replace the parameter of the - // async function. - // - // If this is the simple case, this parameter will end up being the same as the - // original parameter, but with a different pattern id. - let stmt_attrs = this.attrs.get(¶meter.hir_id.local_id).copied(); - let (new_parameter_pat, new_parameter_id) = this.pat_ident(desugared_span, ident); - let new_parameter = hir::Param { - hir_id: parameter.hir_id, - pat: new_parameter_pat, - ty_span: this.lower_span(parameter.ty_span), - span: this.lower_span(parameter.span), - }; + let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments( + decl, + body, + coroutine_kind, + CaptureBy::Value { move_kw: rustc_span::DUMMY_SP }, + ); - if is_simple_parameter { - // If this is the simple case, then we only insert one statement that is - // `let = ;`. We re-use the original argument's pattern so that - // `HirId`s are densely assigned. - let expr = this.expr_ident(desugared_span, ident, new_parameter_id); - let stmt = this.stmt_let_pat( - stmt_attrs, - desugared_span, - Some(expr), - parameter.pat, - hir::LocalSource::AsyncFn, - ); - statements.push(stmt); - } else { - // If this is not the simple case, then we construct two statements: - // - // ``` - // let __argN = __argN; - // let = __argN; - // ``` - // - // The first statement moves the parameter into the closure and thus ensures - // that the drop order is correct. - // - // The second statement creates the bindings that the user wrote. - - // Construct the `let mut __argN = __argN;` statement. It must be a mut binding - // because the user may have specified a `ref mut` binding in the next - // statement. - let (move_pat, move_id) = this.pat_ident_binding_mode( - desugared_span, - ident, - hir::BindingAnnotation::MUT, - ); - let move_expr = this.expr_ident(desugared_span, ident, new_parameter_id); - let move_stmt = this.stmt_let_pat( - None, - desugared_span, - Some(move_expr), - move_pat, - hir::LocalSource::AsyncFn, - ); + // FIXME(async_fn_track_caller): Can this be moved above? + let hir_id = this.lower_node_id(coroutine_kind.closure_id()); + this.maybe_forward_track_caller(body.span, fn_id, hir_id); - // Construct the `let = __argN;` statement. We re-use the original - // parameter's pattern so that `HirId`s are densely assigned. - let pattern_expr = this.expr_ident(desugared_span, ident, move_id); - let pattern_stmt = this.stmt_let_pat( - stmt_attrs, - desugared_span, - Some(pattern_expr), - parameter.pat, - hir::LocalSource::AsyncFn, - ); + (parameters, expr) + }) + } - statements.push(move_stmt); - statements.push(pattern_stmt); - }; + /// Lowers a desugared coroutine body after moving all of the arguments + /// into the body. This is to make sure that the future actually owns the + /// arguments that are passed to the function, and to ensure things like + /// drop order are stable. + fn lower_coroutine_body_with_moved_arguments( + &mut self, + decl: &FnDecl, + body: &Block, + coroutine_kind: CoroutineKind, + capture_clause: CaptureBy, + ) -> (&'hir [hir::Param<'hir>], hir::Expr<'hir>) { + let mut parameters: Vec> = Vec::new(); + let mut statements: Vec> = Vec::new(); + + // Async function parameters are lowered into the closure body so that they are + // captured and so that the drop order matches the equivalent non-async functions. + // + // from: + // + // async fn foo(: , : , : ) { + // + // } + // + // into: + // + // fn foo(__arg0: , __arg1: , __arg2: ) { + // async move { + // let __arg2 = __arg2; + // let = __arg2; + // let __arg1 = __arg1; + // let = __arg1; + // let __arg0 = __arg0; + // let = __arg0; + // drop-temps { } // see comments later in fn for details + // } + // } + // + // If `` is a simple ident, then it is lowered to a single + // `let = ;` statement as an optimization. + // + // Note that the body is embedded in `drop-temps`; an + // equivalent desugaring would be `return { + // };`. The key point is that we wish to drop all the + // let-bound variables and temporaries created in the body + // (and its tail expression!) before we drop the + // parameters (c.f. rust-lang/rust#64512). + for (index, parameter) in decl.inputs.iter().enumerate() { + let parameter = self.lower_param(parameter); + let span = parameter.pat.span; + + // Check if this is a binding pattern, if so, we can optimize and avoid adding a + // `let = __argN;` statement. In this case, we do not rename the parameter. + let (ident, is_simple_parameter) = match parameter.pat.kind { + hir::PatKind::Binding(hir::BindingAnnotation(ByRef::No, _), _, ident, _) => { + (ident, true) + } + // For `ref mut` or wildcard arguments, we can't reuse the binding, but + // we can keep the same name for the parameter. + // This lets rustdoc render it correctly in documentation. + hir::PatKind::Binding(_, _, ident, _) => (ident, false), + hir::PatKind::Wild => { + (Ident::with_dummy_span(rustc_span::symbol::kw::Underscore), false) + } + _ => { + // Replace the ident for bindings that aren't simple. + let name = format!("__arg{index}"); + let ident = Ident::from_str(&name); - parameters.push(new_parameter); - } + (ident, false) + } + }; - let mkbody = |this: &mut LoweringContext<'_, 'hir>| { - // Create a block from the user's function body: - let user_body = this.lower_block_expr(body); + let desugared_span = self.mark_span_with_reason(DesugaringKind::Async, span, None); - // Transform into `drop-temps { }`, an expression: - let desugared_span = - this.mark_span_with_reason(DesugaringKind::Async, user_body.span, None); - let user_body = this.expr_drop_temps(desugared_span, this.arena.alloc(user_body)); + // Construct a parameter representing `__argN: ` to replace the parameter of the + // async function. + // + // If this is the simple case, this parameter will end up being the same as the + // original parameter, but with a different pattern id. + let stmt_attrs = self.attrs.get(¶meter.hir_id.local_id).copied(); + let (new_parameter_pat, new_parameter_id) = self.pat_ident(desugared_span, ident); + let new_parameter = hir::Param { + hir_id: parameter.hir_id, + pat: new_parameter_pat, + ty_span: self.lower_span(parameter.ty_span), + span: self.lower_span(parameter.span), + }; - // As noted above, create the final block like + if is_simple_parameter { + // If this is the simple case, then we only insert one statement that is + // `let = ;`. We re-use the original argument's pattern so that + // `HirId`s are densely assigned. + let expr = self.expr_ident(desugared_span, ident, new_parameter_id); + let stmt = self.stmt_let_pat( + stmt_attrs, + desugared_span, + Some(expr), + parameter.pat, + hir::LocalSource::AsyncFn, + ); + statements.push(stmt); + } else { + // If this is not the simple case, then we construct two statements: // // ``` - // { - // let $param_pattern = $raw_param; - // ... - // drop-temps { } - // } + // let __argN = __argN; + // let = __argN; // ``` - let body = this.block_all( + // + // The first statement moves the parameter into the closure and thus ensures + // that the drop order is correct. + // + // The second statement creates the bindings that the user wrote. + + // Construct the `let mut __argN = __argN;` statement. It must be a mut binding + // because the user may have specified a `ref mut` binding in the next + // statement. + let (move_pat, move_id) = + self.pat_ident_binding_mode(desugared_span, ident, hir::BindingAnnotation::MUT); + let move_expr = self.expr_ident(desugared_span, ident, new_parameter_id); + let move_stmt = self.stmt_let_pat( + None, desugared_span, - this.arena.alloc_from_iter(statements), - Some(user_body), + Some(move_expr), + move_pat, + hir::LocalSource::AsyncFn, ); - this.expr_block(body) - }; - let desugaring_kind = match coroutine_kind { - CoroutineKind::Async { .. } => hir::CoroutineDesugaring::Async, - CoroutineKind::Gen { .. } => hir::CoroutineDesugaring::Gen, - CoroutineKind::AsyncGen { .. } => hir::CoroutineDesugaring::AsyncGen, + // Construct the `let = __argN;` statement. We re-use the original + // parameter's pattern so that `HirId`s are densely assigned. + let pattern_expr = self.expr_ident(desugared_span, ident, move_id); + let pattern_stmt = self.stmt_let_pat( + stmt_attrs, + desugared_span, + Some(pattern_expr), + parameter.pat, + hir::LocalSource::AsyncFn, + ); + + statements.push(move_stmt); + statements.push(pattern_stmt); }; - let coroutine_expr = this.make_desugared_coroutine_expr( - CaptureBy::Value { move_kw: rustc_span::DUMMY_SP }, - closure_id, - None, - body.span, - desugaring_kind, - hir::CoroutineSource::Fn, - mkbody, + + parameters.push(new_parameter); + } + + let mkbody = |this: &mut LoweringContext<'_, 'hir>| { + // Create a block from the user's function body: + let user_body = this.lower_block_expr(body); + + // Transform into `drop-temps { }`, an expression: + let desugared_span = + this.mark_span_with_reason(DesugaringKind::Async, user_body.span, None); + let user_body = this.expr_drop_temps(desugared_span, this.arena.alloc(user_body)); + + // As noted above, create the final block like + // + // ``` + // { + // let $param_pattern = $raw_param; + // ... + // drop-temps { } + // } + // ``` + let body = this.block_all( + desugared_span, + this.arena.alloc_from_iter(statements), + Some(user_body), ); - let hir_id = this.lower_node_id(closure_id); - this.maybe_forward_track_caller(body.span, fn_id, hir_id); - let expr = hir::Expr { hir_id, kind: coroutine_expr, span: this.lower_span(body.span) }; + this.expr_block(body) + }; + let desugaring_kind = match coroutine_kind { + CoroutineKind::Async { .. } => hir::CoroutineDesugaring::Async, + CoroutineKind::Gen { .. } => hir::CoroutineDesugaring::Gen, + CoroutineKind::AsyncGen { .. } => hir::CoroutineDesugaring::AsyncGen, + }; + let closure_id = coroutine_kind.closure_id(); + let coroutine_expr = self.make_desugared_coroutine_expr( + capture_clause, + closure_id, + None, + body.span, + desugaring_kind, + hir::CoroutineSource::Fn, + mkbody, + ); - (this.arena.alloc_from_iter(parameters), expr) - }) + let expr = hir::Expr { + hir_id: self.lower_node_id(closure_id), + kind: coroutine_expr, + span: self.lower_span(body.span), + }; + + (self.arena.alloc_from_iter(parameters), expr) } fn lower_method_sig( From f56a4c0ebf942389498ff7bf4273b09d8238cfe6 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 18 Dec 2023 02:11:55 +0000 Subject: [PATCH 10/14] Deny braced macro invocations in let-else --- compiler/rustc_ast/src/util/classify.rs | 10 +++++-- tests/ui/parser/bad-let-else-statement.rs | 27 ++++++++++++++++++ tests/ui/parser/bad-let-else-statement.stderr | 28 ++++++++++++++++++- 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_ast/src/util/classify.rs b/compiler/rustc_ast/src/util/classify.rs index 65036bcdc36fd..098b8f2d6d038 100644 --- a/compiler/rustc_ast/src/util/classify.rs +++ b/compiler/rustc_ast/src/util/classify.rs @@ -2,7 +2,7 @@ // Predicates on exprs and stmts that the pretty-printer and parser use -use crate::ast; +use crate::{ast, token::Delimiter}; /// Does this expression require a semicolon to be treated /// as a statement? The negation of this: 'can this expression @@ -59,8 +59,12 @@ pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option<&ast::Expr> { | While(..) | ConstBlock(_) => break Some(expr), - // FIXME: These can end in `}`, but changing these would break stable code. - InlineAsm(_) | OffsetOf(_, _) | MacCall(_) | IncludedBytes(_) | FormatArgs(_) => { + MacCall(mac) => { + break (mac.args.delim == Delimiter::Brace).then_some(expr); + } + + InlineAsm(_) | OffsetOf(_, _) | IncludedBytes(_) | FormatArgs(_) => { + // These should have been denied pre-expansion. break None; } diff --git a/tests/ui/parser/bad-let-else-statement.rs b/tests/ui/parser/bad-let-else-statement.rs index 7b927c89ba074..5866e1b751461 100644 --- a/tests/ui/parser/bad-let-else-statement.rs +++ b/tests/ui/parser/bad-let-else-statement.rs @@ -161,4 +161,31 @@ fn q() { }; } +fn r() { + use std::arch::asm; + + let ok = asm!(" nop ") else { return; }; + + let bad = asm! {" nop "} else { return; }; + //~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed +} + +fn s() { + macro_rules! a { + () => { {} } + } + + macro_rules! b { + (1) => { + let x = a!() else { return; }; + }; + (2) => { + let x = a! {} else { return; }; + //~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed + }; + } + + b!(1); b!(2); +} + fn main() {} diff --git a/tests/ui/parser/bad-let-else-statement.stderr b/tests/ui/parser/bad-let-else-statement.stderr index 7cbda25e417f3..e636a2226e205 100644 --- a/tests/ui/parser/bad-let-else-statement.stderr +++ b/tests/ui/parser/bad-let-else-statement.stderr @@ -228,5 +228,31 @@ LL | x LL ~ }) else { | -error: aborting due to 17 previous errors +error: right curly brace `}` before `else` in a `let...else` statement not allowed + --> $DIR/bad-let-else-statement.rs:169:28 + | +LL | let bad = asm! {" nop "} else { return; }; + | ^ + | +help: wrap the expression in parentheses + | +LL | let bad = (asm! {" nop "}) else { return; }; + | + + + +error: right curly brace `}` before `else` in a `let...else` statement not allowed + --> $DIR/bad-let-else-statement.rs:183:25 + | +LL | let x = a! {} else { return; }; + | ^ +... +LL | b!(1); b!(2); + | ----- in this macro invocation + | + = note: this error originates in the macro `b` (in Nightly builds, run with -Z macro-backtrace for more info) +help: wrap the expression in parentheses + | +LL | let x = (a! {}) else { return; }; + | + + + +error: aborting due to 19 previous errors From fd023696658c62b82e0b0b67578cf0733e183a9e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 18 Dec 2023 03:01:05 +0000 Subject: [PATCH 11/14] Suggest wrapping mac args in parens rather than the whole expression --- compiler/rustc_parse/messages.ftl | 2 + compiler/rustc_parse/src/errors.rs | 37 +++++++++++++------ compiler/rustc_parse/src/parser/expr.rs | 2 +- compiler/rustc_parse/src/parser/stmt.rs | 15 ++++++-- tests/ui/parser/bad-let-else-statement.stderr | 12 +++--- 5 files changed, 45 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_parse/messages.ftl b/compiler/rustc_parse/messages.ftl index d515e86a18296..e3ecd24b6a5f0 100644 --- a/compiler/rustc_parse/messages.ftl +++ b/compiler/rustc_parse/messages.ftl @@ -722,6 +722,8 @@ parse_sugg_turbofish_syntax = use `::<...>` instead of `<...>` to specify lifeti parse_sugg_wrap_expression_in_parentheses = wrap the expression in parentheses +parse_sugg_wrap_macro_in_parentheses = use parentheses instead of braces for this macro + parse_sugg_wrap_pattern_in_parens = wrap the pattern in parentheses parse_switch_mut_let_order = diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index 34b34a1bad6c9..8f57b2eb5aa97 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -722,19 +722,32 @@ pub(crate) struct LabeledLoopInBreak { #[primary_span] pub span: Span, #[subdiagnostic] - pub sub: WrapExpressionInParentheses, + pub sub: WrapInParentheses, } #[derive(Subdiagnostic)] -#[multipart_suggestion( - parse_sugg_wrap_expression_in_parentheses, - applicability = "machine-applicable" -)] -pub(crate) struct WrapExpressionInParentheses { - #[suggestion_part(code = "(")] - pub left: Span, - #[suggestion_part(code = ")")] - pub right: Span, + +pub(crate) enum WrapInParentheses { + #[multipart_suggestion( + parse_sugg_wrap_expression_in_parentheses, + applicability = "machine-applicable" + )] + Expression { + #[suggestion_part(code = "(")] + left: Span, + #[suggestion_part(code = ")")] + right: Span, + }, + #[multipart_suggestion( + parse_sugg_wrap_macro_in_parentheses, + applicability = "machine-applicable" + )] + MacroArgs { + #[suggestion_part(code = "(")] + left: Span, + #[suggestion_part(code = ")")] + right: Span, + }, } #[derive(Diagnostic)] @@ -936,7 +949,7 @@ pub(crate) struct InvalidExpressionInLetElse { pub span: Span, pub operator: &'static str, #[subdiagnostic] - pub sugg: WrapExpressionInParentheses, + pub sugg: WrapInParentheses, } #[derive(Diagnostic)] @@ -945,7 +958,7 @@ pub(crate) struct InvalidCurlyInLetElse { #[primary_span] pub span: Span, #[subdiagnostic] - pub sugg: WrapExpressionInParentheses, + pub sugg: WrapInParentheses, } #[derive(Diagnostic)] diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 8ca02452342b2..f858706805dbc 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1844,7 +1844,7 @@ impl<'a> Parser<'a> { let lexpr = self.parse_expr_labeled(label, true)?; self.dcx().emit_err(errors::LabeledLoopInBreak { span: lexpr.span, - sub: errors::WrapExpressionInParentheses { + sub: errors::WrapInParentheses::Expression { left: lexpr.span.shrink_to_lo(), right: lexpr.span.shrink_to_hi(), }, diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 1a8bbf0a15722..1bae5b3224035 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -389,7 +389,7 @@ impl<'a> Parser<'a> { self.dcx().emit_err(errors::InvalidExpressionInLetElse { span: init.span, operator: op.node.as_str(), - sugg: errors::WrapExpressionInParentheses { + sugg: errors::WrapInParentheses::Expression { left: init.span.shrink_to_lo(), right: init.span.shrink_to_hi(), }, @@ -400,12 +400,19 @@ impl<'a> Parser<'a> { fn check_let_else_init_trailing_brace(&self, init: &ast::Expr) { if let Some(trailing) = classify::expr_trailing_brace(init) { - self.dcx().emit_err(errors::InvalidCurlyInLetElse { - span: trailing.span.with_lo(trailing.span.hi() - BytePos(1)), - sugg: errors::WrapExpressionInParentheses { + let sugg = match &trailing.kind { + ExprKind::MacCall(mac) => errors::WrapInParentheses::MacroArgs { + left: mac.args.dspan.open, + right: mac.args.dspan.close, + }, + _ => errors::WrapInParentheses::Expression { left: trailing.span.shrink_to_lo(), right: trailing.span.shrink_to_hi(), }, + }; + self.dcx().emit_err(errors::InvalidCurlyInLetElse { + span: trailing.span.with_lo(trailing.span.hi() - BytePos(1)), + sugg, }); } } diff --git a/tests/ui/parser/bad-let-else-statement.stderr b/tests/ui/parser/bad-let-else-statement.stderr index e636a2226e205..7201abbb58ca0 100644 --- a/tests/ui/parser/bad-let-else-statement.stderr +++ b/tests/ui/parser/bad-let-else-statement.stderr @@ -234,10 +234,10 @@ error: right curly brace `}` before `else` in a `let...else` statement not allow LL | let bad = asm! {" nop "} else { return; }; | ^ | -help: wrap the expression in parentheses +help: use parentheses instead of braces for this macro | -LL | let bad = (asm! {" nop "}) else { return; }; - | + + +LL | let bad = asm! (" nop ") else { return; }; + | ~ ~ error: right curly brace `}` before `else` in a `let...else` statement not allowed --> $DIR/bad-let-else-statement.rs:183:25 @@ -249,10 +249,10 @@ LL | b!(1); b!(2); | ----- in this macro invocation | = note: this error originates in the macro `b` (in Nightly builds, run with -Z macro-backtrace for more info) -help: wrap the expression in parentheses +help: use parentheses instead of braces for this macro | -LL | let x = (a! {}) else { return; }; - | + + +LL | let x = a! () else { return; }; + | ~~ error: aborting due to 19 previous errors From f1ee076f8188c02a2f4d06fc2fde2f58bdb3d6f4 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 14 Jan 2024 23:12:46 +0000 Subject: [PATCH 12/14] Async closures will move params into the future always --- compiler/rustc_ast_lowering/messages.ftl | 4 -- compiler/rustc_ast_lowering/src/errors.rs | 8 ---- compiler/rustc_ast_lowering/src/expr.rs | 39 +++++++------------ compiler/rustc_ast_lowering/src/item.rs | 29 ++++++++------ .../src/error_codes/E0708.md | 6 ++- .../async-borrowck-escaping-closure-error.rs | 3 +- ...ync-borrowck-escaping-closure-error.stderr | 21 ---------- .../no-params-non-move-async-closure.rs | 2 +- .../no-params-non-move-async-closure.stderr | 11 ------ .../suggest-on-bare-closure-call.stderr | 5 --- 10 files changed, 40 insertions(+), 88 deletions(-) delete mode 100644 tests/ui/async-await/async-borrowck-escaping-closure-error.stderr delete mode 100644 tests/ui/async-await/no-params-non-move-async-closure.stderr diff --git a/compiler/rustc_ast_lowering/messages.ftl b/compiler/rustc_ast_lowering/messages.ftl index e7177402db1d7..8615016cda599 100644 --- a/compiler/rustc_ast_lowering/messages.ftl +++ b/compiler/rustc_ast_lowering/messages.ftl @@ -14,10 +14,6 @@ ast_lowering_assoc_ty_parentheses = ast_lowering_async_coroutines_not_supported = `async` coroutines are not yet supported -ast_lowering_async_non_move_closure_not_supported = - `async` non-`move` closures with parameters are not currently supported - .help = consider using `let` statements to manually capture variables by reference before entering an `async move` closure - ast_lowering_att_syntax_only_x86 = the `att_syntax` option is only supported on x86 diff --git a/compiler/rustc_ast_lowering/src/errors.rs b/compiler/rustc_ast_lowering/src/errors.rs index 2811fe104cd09..4843d36372dcc 100644 --- a/compiler/rustc_ast_lowering/src/errors.rs +++ b/compiler/rustc_ast_lowering/src/errors.rs @@ -145,14 +145,6 @@ pub struct ClosureCannotBeStatic { pub fn_decl_span: Span, } -#[derive(Diagnostic, Clone, Copy)] -#[help] -#[diag(ast_lowering_async_non_move_closure_not_supported, code = "E0708")] -pub struct AsyncNonMoveClosureNotSupported { - #[primary_span] - pub fn_decl_span: Span, -} - #[derive(Diagnostic, Clone, Copy)] #[diag(ast_lowering_functional_record_update_destructuring_assignment)] pub struct FunctionalRecordUpdateDestructuringAssignment { diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index e0b1a10c82e7d..0920de48eb87e 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -1,6 +1,6 @@ use super::errors::{ - AsyncCoroutinesNotSupported, AsyncNonMoveClosureNotSupported, AwaitOnlyInAsyncFnAndBlocks, - BaseExpressionDoubleDot, ClosureCannotBeStatic, CoroutineTooManyParameters, + AsyncCoroutinesNotSupported, AwaitOnlyInAsyncFnAndBlocks, BaseExpressionDoubleDot, + ClosureCannotBeStatic, CoroutineTooManyParameters, FunctionalRecordUpdateDestructuringAssignment, InclusiveRangeWithNoEnd, MatchArmWithNoBody, NeverPatternWithBody, NeverPatternWithGuard, NotSupportedForLifetimeBinderAsyncClosure, UnderscoreExprLhsAssign, @@ -13,7 +13,6 @@ use rustc_ast::*; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; -use rustc_middle::span_bug; use rustc_session::errors::report_lit_error; use rustc_span::source_map::{respan, Spanned}; use rustc_span::symbol::{kw, sym, Ident, Symbol}; @@ -1028,28 +1027,16 @@ impl<'hir> LoweringContext<'_, 'hir> { fn_decl_span: Span, fn_arg_span: Span, ) -> hir::ExprKind<'hir> { - let CoroutineKind::Async { closure_id: inner_closure_id, .. } = coroutine_kind else { - span_bug!(fn_decl_span, "`async gen` and `gen` closures are not supported, yet"); - }; - if let &ClosureBinder::For { span, .. } = binder { self.dcx().emit_err(NotSupportedForLifetimeBinderAsyncClosure { span }); } let (binder_clause, generic_params) = self.lower_closure_binder(binder); - let outer_decl = - FnDecl { inputs: decl.inputs.clone(), output: FnRetTy::Default(fn_decl_span) }; - let body = self.with_new_scopes(fn_decl_span, |this| { - // FIXME(cramertj): allow `async` non-`move` closures with arguments. - if capture_clause == CaptureBy::Ref && !decl.inputs.is_empty() { - this.dcx().emit_err(AsyncNonMoveClosureNotSupported { fn_decl_span }); - } - // Transform `async |x: u8| -> X { ... }` into // `|x: u8| || -> X { ... }`. - let body_id = this.lower_fn_body(&outer_decl, |this| { + let body_id = this.lower_body(|this| { let async_ret_ty = if let FnRetTy::Ty(ty) = &decl.output { let itctx = ImplTraitContext::Disallowed(ImplTraitPosition::AsyncBlock); Some(hir::FnRetTy::Return(this.lower_ty(ty, &itctx))) @@ -1057,22 +1044,26 @@ impl<'hir> LoweringContext<'_, 'hir> { None }; - let async_body = this.make_desugared_coroutine_expr( - capture_clause, - inner_closure_id, - async_ret_ty, + let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments( + decl, + |this| this.with_new_scopes(fn_decl_span, |this| this.lower_expr_mut(body)), body.span, - hir::CoroutineDesugaring::Async, + coroutine_kind, hir::CoroutineSource::Closure, - |this| this.with_new_scopes(fn_decl_span, |this| this.lower_expr_mut(body)), + async_ret_ty, ); - let hir_id = this.lower_node_id(inner_closure_id); + + let hir_id = this.lower_node_id(coroutine_kind.closure_id()); this.maybe_forward_track_caller(body.span, closure_hir_id, hir_id); - hir::Expr { hir_id, kind: async_body, span: this.lower_span(body.span) } + + (parameters, expr) }); body_id }); + let outer_decl = + FnDecl { inputs: decl.inputs.clone(), output: FnRetTy::Default(fn_decl_span) }; + let bound_generic_params = self.lower_lifetime_binder(closure_id, generic_params); // We need to lower the declaration outside the new scope, because we // have to conserve the state of being inside a loop condition for the diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index c8e639b7a9663..dd3f7289a60b2 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -1085,9 +1085,11 @@ impl<'hir> LoweringContext<'_, 'hir> { self.lower_body(|this| { let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments( decl, - body, + |this| this.lower_block_expr(body), + body.span, coroutine_kind, - CaptureBy::Value { move_kw: rustc_span::DUMMY_SP }, + hir::CoroutineSource::Fn, + None, ); // FIXME(async_fn_track_caller): Can this be moved above? @@ -1102,12 +1104,14 @@ impl<'hir> LoweringContext<'_, 'hir> { /// into the body. This is to make sure that the future actually owns the /// arguments that are passed to the function, and to ensure things like /// drop order are stable. - fn lower_coroutine_body_with_moved_arguments( + pub fn lower_coroutine_body_with_moved_arguments( &mut self, decl: &FnDecl, - body: &Block, + lower_body: impl FnOnce(&mut LoweringContext<'_, 'hir>) -> hir::Expr<'hir>, + body_span: Span, coroutine_kind: CoroutineKind, - capture_clause: CaptureBy, + coroutine_source: hir::CoroutineSource, + return_type_hint: Option>, ) -> (&'hir [hir::Param<'hir>], hir::Expr<'hir>) { let mut parameters: Vec> = Vec::new(); let mut statements: Vec> = Vec::new(); @@ -1246,7 +1250,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let mkbody = |this: &mut LoweringContext<'_, 'hir>| { // Create a block from the user's function body: - let user_body = this.lower_block_expr(body); + let user_body = lower_body(this); // Transform into `drop-temps { }`, an expression: let desugared_span = @@ -1277,19 +1281,22 @@ impl<'hir> LoweringContext<'_, 'hir> { }; let closure_id = coroutine_kind.closure_id(); let coroutine_expr = self.make_desugared_coroutine_expr( - capture_clause, + // FIXME(async_closures): This should only move locals, + // and not upvars. Capturing closure upvars by ref doesn't + // work right now anyways, so whatever. + CaptureBy::Value { move_kw: rustc_span::DUMMY_SP }, closure_id, - None, - body.span, + return_type_hint, + body_span, desugaring_kind, - hir::CoroutineSource::Fn, + coroutine_source, mkbody, ); let expr = hir::Expr { hir_id: self.lower_node_id(closure_id), kind: coroutine_expr, - span: self.lower_span(body.span), + span: self.lower_span(body_span), }; (self.arena.alloc_from_iter(parameters), expr) diff --git a/compiler/rustc_error_codes/src/error_codes/E0708.md b/compiler/rustc_error_codes/src/error_codes/E0708.md index 9287fc803d1de..61a853ac44606 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0708.md +++ b/compiler/rustc_error_codes/src/error_codes/E0708.md @@ -1,12 +1,14 @@ +#### Note: this error code is no longer emitted by the compiler. + `async` non-`move` closures with parameters are currently not supported. Erroneous code example: -```compile_fail,edition2018,E0708 +```edition2018 #![feature(async_closure)] fn main() { - let add_one = async |num: u8| { // error! + let add_one = async |num: u8| { num + 1 }; } diff --git a/tests/ui/async-await/async-borrowck-escaping-closure-error.rs b/tests/ui/async-await/async-borrowck-escaping-closure-error.rs index e667b72aee530..f8ff9186842b8 100644 --- a/tests/ui/async-await/async-borrowck-escaping-closure-error.rs +++ b/tests/ui/async-await/async-borrowck-escaping-closure-error.rs @@ -1,9 +1,10 @@ // edition:2018 +// check-pass + #![feature(async_closure)] fn foo() -> Box> { let x = 0u32; Box::new((async || x)()) - //~^ ERROR E0373 } fn main() { diff --git a/tests/ui/async-await/async-borrowck-escaping-closure-error.stderr b/tests/ui/async-await/async-borrowck-escaping-closure-error.stderr deleted file mode 100644 index 1d8d1c67bae16..0000000000000 --- a/tests/ui/async-await/async-borrowck-escaping-closure-error.stderr +++ /dev/null @@ -1,21 +0,0 @@ -error[E0373]: closure may outlive the current function, but it borrows `x`, which is owned by the current function - --> $DIR/async-borrowck-escaping-closure-error.rs:5:15 - | -LL | Box::new((async || x)()) - | ^^^^^^^^ - `x` is borrowed here - | | - | may outlive borrowed value `x` - | -note: closure is returned here - --> $DIR/async-borrowck-escaping-closure-error.rs:5:5 - | -LL | Box::new((async || x)()) - | ^^^^^^^^^^^^^^^^^^^^^^^^ -help: to force the closure to take ownership of `x` (and any other referenced variables), use the `move` keyword - | -LL | Box::new((async move || x)()) - | ++++ - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0373`. diff --git a/tests/ui/async-await/no-params-non-move-async-closure.rs b/tests/ui/async-await/no-params-non-move-async-closure.rs index 3b15f35c260dc..1440d918c50eb 100644 --- a/tests/ui/async-await/no-params-non-move-async-closure.rs +++ b/tests/ui/async-await/no-params-non-move-async-closure.rs @@ -1,8 +1,8 @@ // edition:2018 +// check-pass #![feature(async_closure)] fn main() { let _ = async |x: u8| {}; - //~^ ERROR `async` non-`move` closures with parameters are not currently supported } diff --git a/tests/ui/async-await/no-params-non-move-async-closure.stderr b/tests/ui/async-await/no-params-non-move-async-closure.stderr deleted file mode 100644 index d265955369900..0000000000000 --- a/tests/ui/async-await/no-params-non-move-async-closure.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error[E0708]: `async` non-`move` closures with parameters are not currently supported - --> $DIR/no-params-non-move-async-closure.rs:6:13 - | -LL | let _ = async |x: u8| {}; - | ^^^^^^^^^^^^^ - | - = help: consider using `let` statements to manually capture variables by reference before entering an `async move` closure - -error: aborting due to 1 previous error - -For more information about this error, try `rustc --explain E0708`. diff --git a/tests/ui/suggestions/suggest-on-bare-closure-call.stderr b/tests/ui/suggestions/suggest-on-bare-closure-call.stderr index e65a6eb4939d9..77631b36e0f30 100644 --- a/tests/ui/suggestions/suggest-on-bare-closure-call.stderr +++ b/tests/ui/suggestions/suggest-on-bare-closure-call.stderr @@ -18,11 +18,6 @@ LL | let _ = async ||{}(); | ^^-- | | | call expression requires function - | -help: if you meant to create this closure and immediately call it, surround the closure with parentheses - | -LL | let _ = (async ||{})(); - | + + error: aborting due to 2 previous errors From f4e35c60b04b381395150669f1e8656dccc5f94f Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 15 Jan 2024 19:24:03 +0000 Subject: [PATCH 13/14] Fix async closure call suggestion --- compiler/rustc_hir_typeck/src/callee.rs | 82 +++++++++++-------- .../suggest-on-bare-closure-call.stderr | 5 ++ 2 files changed, 51 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/callee.rs b/compiler/rustc_hir_typeck/src/callee.rs index 1a4e03d50cae0..d486f069989ee 100644 --- a/compiler/rustc_hir_typeck/src/callee.rs +++ b/compiler/rustc_hir_typeck/src/callee.rs @@ -293,49 +293,59 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { callee_node: &hir::ExprKind<'_>, callee_span: Span, ) { + let hir::ExprKind::Block(..) = callee_node else { + // Only calls on blocks suggested here. + return; + }; + let hir = self.tcx.hir(); - let parent_hir_id = hir.parent_id(hir_id); - let parent_node = self.tcx.hir_node(parent_hir_id); - if let ( - hir::Node::Expr(hir::Expr { - kind: hir::ExprKind::Closure(&hir::Closure { fn_decl_span, kind, .. }), + let fn_decl_span = if let hir::Node::Expr(hir::Expr { + kind: hir::ExprKind::Closure(&hir::Closure { fn_decl_span, .. }), + .. + }) = hir.get_parent(hir_id) + { + fn_decl_span + } else if let Some(( + _, + hir::Node::Expr(&hir::Expr { + hir_id: parent_hir_id, + kind: + hir::ExprKind::Closure(&hir::Closure { + kind: + hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared( + hir::CoroutineDesugaring::Async, + hir::CoroutineSource::Closure, + )), + .. + }), .. }), - hir::ExprKind::Block(..), - ) = (parent_node, callee_node) + )) = hir.parent_iter(hir_id).nth(3) { - let fn_decl_span = if matches!( - kind, - hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared( - hir::CoroutineDesugaring::Async, - hir::CoroutineSource::Closure - ),) - ) { - // Actually need to unwrap one more layer of HIR to get to - // the _real_ closure... - let async_closure = hir.parent_id(parent_hir_id); - if let hir::Node::Expr(hir::Expr { - kind: hir::ExprKind::Closure(&hir::Closure { fn_decl_span, .. }), - .. - }) = self.tcx.hir_node(async_closure) - { - fn_decl_span - } else { - return; - } - } else { + // Actually need to unwrap one more layer of HIR to get to + // the _real_ closure... + let async_closure = hir.parent_id(parent_hir_id); + if let hir::Node::Expr(hir::Expr { + kind: hir::ExprKind::Closure(&hir::Closure { fn_decl_span, .. }), + .. + }) = self.tcx.hir_node(async_closure) + { fn_decl_span - }; + } else { + return; + } + } else { + return; + }; - let start = fn_decl_span.shrink_to_lo(); - let end = callee_span.shrink_to_hi(); - err.multipart_suggestion( - "if you meant to create this closure and immediately call it, surround the \ + let start = fn_decl_span.shrink_to_lo(); + let end = callee_span.shrink_to_hi(); + err.multipart_suggestion( + "if you meant to create this closure and immediately call it, surround the \ closure with parentheses", - vec![(start, "(".to_string()), (end, ")".to_string())], - Applicability::MaybeIncorrect, - ); - } + vec![(start, "(".to_string()), (end, ")".to_string())], + Applicability::MaybeIncorrect, + ); } /// Give appropriate suggestion when encountering `[("a", 0) ("b", 1)]`, where the diff --git a/tests/ui/suggestions/suggest-on-bare-closure-call.stderr b/tests/ui/suggestions/suggest-on-bare-closure-call.stderr index 77631b36e0f30..e65a6eb4939d9 100644 --- a/tests/ui/suggestions/suggest-on-bare-closure-call.stderr +++ b/tests/ui/suggestions/suggest-on-bare-closure-call.stderr @@ -18,6 +18,11 @@ LL | let _ = async ||{}(); | ^^-- | | | call expression requires function + | +help: if you meant to create this closure and immediately call it, surround the closure with parentheses + | +LL | let _ = (async ||{})(); + | + + error: aborting due to 2 previous errors From 04a5ee6f1d6ab1270b95c6c91a718d900a762eb8 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 15 Jan 2024 19:38:53 +0000 Subject: [PATCH 14/14] Deal with additional wrapping of async closure body in clippy --- .../clippy_lints/src/async_yields_async.rs | 98 ++++++++++++------- .../src/redundant_closure_call.rs | 20 +++- .../clippy/tests/ui/author/blocks.stdout | 6 +- 3 files changed, 82 insertions(+), 42 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/async_yields_async.rs b/src/tools/clippy/clippy_lints/src/async_yields_async.rs index bb08ac7508bc2..eeaa3de3725fb 100644 --- a/src/tools/clippy/clippy_lints/src/async_yields_async.rs +++ b/src/tools/clippy/clippy_lints/src/async_yields_async.rs @@ -45,50 +45,72 @@ declare_lint_pass!(AsyncYieldsAsync => [ASYNC_YIELDS_ASYNC]); impl<'tcx> LateLintPass<'tcx> for AsyncYieldsAsync { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - // For functions, with explicitly defined types, don't warn. - // XXXkhuey maybe we should? - if let ExprKind::Closure(Closure { - kind: - ClosureKind::Coroutine(CoroutineKind::Desugared( - CoroutineDesugaring::Async, - CoroutineSource::Block | CoroutineSource::Closure, - )), + let ExprKind::Closure(Closure { + kind: ClosureKind::Coroutine(CoroutineKind::Desugared(CoroutineDesugaring::Async, kind)), body: body_id, .. }) = expr.kind - { - if let Some(future_trait_def_id) = cx.tcx.lang_items().future_trait() { - let typeck_results = cx.tcx.typeck_body(*body_id); - let body = cx.tcx.hir().body(*body_id); - let expr_ty = typeck_results.expr_ty(body.value); + else { + return; + }; - if implements_trait(cx, expr_ty, future_trait_def_id, &[]) { - let return_expr_span = match &body.value.kind { - // XXXkhuey there has to be a better way. - ExprKind::Block(block, _) => block.expr.map(|e| e.span), - ExprKind::Path(QPath::Resolved(_, path)) => Some(path.span), - _ => None, - }; - if let Some(return_expr_span) = return_expr_span { - span_lint_hir_and_then( - cx, - ASYNC_YIELDS_ASYNC, - body.value.hir_id, + let body_expr = match kind { + CoroutineSource::Fn => { + // For functions, with explicitly defined types, don't warn. + // XXXkhuey maybe we should? + return; + }, + CoroutineSource::Block => cx.tcx.hir().body(*body_id).value, + CoroutineSource::Closure => { + // Like `async fn`, async closures are wrapped in an additional block + // to move all of the closure's arguments into the future. + + let async_closure_body = cx.tcx.hir().body(*body_id).value; + let ExprKind::Block(block, _) = async_closure_body.kind else { + return; + }; + let Some(block_expr) = block.expr else { + return; + }; + let ExprKind::DropTemps(body_expr) = block_expr.kind else { + return; + }; + body_expr + }, + }; + + let Some(future_trait_def_id) = cx.tcx.lang_items().future_trait() else { + return; + }; + + let typeck_results = cx.tcx.typeck_body(*body_id); + let expr_ty = typeck_results.expr_ty(body_expr); + + if implements_trait(cx, expr_ty, future_trait_def_id, &[]) { + let return_expr_span = match &body_expr.kind { + // XXXkhuey there has to be a better way. + ExprKind::Block(block, _) => block.expr.map(|e| e.span), + ExprKind::Path(QPath::Resolved(_, path)) => Some(path.span), + _ => None, + }; + if let Some(return_expr_span) = return_expr_span { + span_lint_hir_and_then( + cx, + ASYNC_YIELDS_ASYNC, + body_expr.hir_id, + return_expr_span, + "an async construct yields a type which is itself awaitable", + |db| { + db.span_label(body_expr.span, "outer async construct"); + db.span_label(return_expr_span, "awaitable value not awaited"); + db.span_suggestion( return_expr_span, - "an async construct yields a type which is itself awaitable", - |db| { - db.span_label(body.value.span, "outer async construct"); - db.span_label(return_expr_span, "awaitable value not awaited"); - db.span_suggestion( - return_expr_span, - "consider awaiting this value", - format!("{}.await", snippet(cx, return_expr_span, "..")), - Applicability::MaybeIncorrect, - ); - }, + "consider awaiting this value", + format!("{}.await", snippet(cx, return_expr_span, "..")), + Applicability::MaybeIncorrect, ); - } - } + }, + ); } } } diff --git a/src/tools/clippy/clippy_lints/src/redundant_closure_call.rs b/src/tools/clippy/clippy_lints/src/redundant_closure_call.rs index cde08dfcc748d..334e6770ae407 100644 --- a/src/tools/clippy/clippy_lints/src/redundant_closure_call.rs +++ b/src/tools/clippy/clippy_lints/src/redundant_closure_call.rs @@ -5,7 +5,9 @@ use clippy_utils::sugg::Sugg; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::intravisit::{Visitor as HirVisitor, Visitor}; -use rustc_hir::{intravisit as hir_visit, ClosureKind, CoroutineDesugaring, CoroutineKind, CoroutineSource, Node}; +use rustc_hir::{ + intravisit as hir_visit, ClosureKind, CoroutineDesugaring, CoroutineKind, CoroutineSource, ExprKind, Node, +}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::nested_filter; use rustc_middle::lint::in_external_macro; @@ -166,10 +168,22 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClosureCall { if coroutine_kind.is_async() && let hir::ExprKind::Closure(closure) = body.kind { - let async_closure_body = cx.tcx.hir().body(closure.body); + // Like `async fn`, async closures are wrapped in an additional block + // to move all of the closure's arguments into the future. + + let async_closure_body = cx.tcx.hir().body(closure.body).value; + let ExprKind::Block(block, _) = async_closure_body.kind else { + return; + }; + let Some(block_expr) = block.expr else { + return; + }; + let ExprKind::DropTemps(body_expr) = block_expr.kind else { + return; + }; // `async x` is a syntax error, so it becomes `async { x }` - if !matches!(async_closure_body.value.kind, hir::ExprKind::Block(_, _)) { + if !matches!(body_expr.kind, hir::ExprKind::Block(_, _)) { hint = hint.blockify(); } diff --git a/src/tools/clippy/tests/ui/author/blocks.stdout b/src/tools/clippy/tests/ui/author/blocks.stdout index 62de661f8ff86..8c4d71e68f80f 100644 --- a/src/tools/clippy/tests/ui/author/blocks.stdout +++ b/src/tools/clippy/tests/ui/author/blocks.stdout @@ -48,7 +48,11 @@ if let ExprKind::Closure { capture_clause: CaptureBy::Value { .. }, fn_decl: fn_ && expr2 = &cx.tcx.hir().body(body_id1).value && let ExprKind::Block(block, None) = expr2.kind && block.stmts.is_empty() - && block.expr.is_none() + && let Some(trailing_expr) = block.expr + && let ExprKind::DropTemps(expr3) = trailing_expr.kind + && let ExprKind::Block(block1, None) = expr3.kind + && block1.stmts.is_empty() + && block1.expr.is_none() { // report your lint here }