From 3dac70fcc093a3145c4fa3315d43012090c7968b Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 4 May 2022 07:27:12 +0100 Subject: [PATCH 1/4] typeck: port "unconstrained opaque type" diag Port the "unconstrained opaque type" diagnostic to using the diagnostic derive. Signed-off-by: David Wood --- .../rustc_error_messages/locales/en-US/typeck.ftl | 3 +++ compiler/rustc_typeck/src/collect/type_of.rs | 12 +++++------- compiler/rustc_typeck/src/errors.rs | 9 +++++++++ 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_error_messages/locales/en-US/typeck.ftl b/compiler/rustc_error_messages/locales/en-US/typeck.ftl index 6a3235fc7728c..80eacd41add3c 100644 --- a/compiler/rustc_error_messages/locales/en-US/typeck.ftl +++ b/compiler/rustc_error_messages/locales/en-US/typeck.ftl @@ -90,3 +90,6 @@ typeck-add-return-type-missing-here = a return type might be missing here typeck-expected-default-return-type = expected `()` because of default return type typeck-expected-return-type = expected `{$expected}` because of return type + +typeck-unconstrained-opaque-type = unconstrained opaque type + .note = `{$name}` must be used in combination with a concrete type within the same module diff --git a/compiler/rustc_typeck/src/collect/type_of.rs b/compiler/rustc_typeck/src/collect/type_of.rs index 75ad584f41990..e37e0c91de46c 100644 --- a/compiler/rustc_typeck/src/collect/type_of.rs +++ b/compiler/rustc_typeck/src/collect/type_of.rs @@ -14,6 +14,7 @@ use rustc_span::{Span, DUMMY_SP}; use super::ItemCtxt; use super::{bad_placeholder, is_suggestable_infer_ty}; +use crate::errors::UnconstrainedOpaqueType; /// Computes the relevant generic parameter for a potential generic const argument. /// @@ -682,13 +683,10 @@ fn find_opaque_ty_constraints(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Ty<'_> { match locator.found { Some(hidden) => hidden.ty, None => { - let span = tcx.def_span(def_id); - let name = tcx.item_name(tcx.local_parent(def_id).to_def_id()); - let label = format!( - "`{}` must be used in combination with a concrete type within the same module", - name - ); - tcx.sess.struct_span_err(span, "unconstrained opaque type").note(&label).emit(); + tcx.sess.emit_err(UnconstrainedOpaqueType { + span: tcx.def_span(def_id), + name: tcx.item_name(tcx.local_parent(def_id).to_def_id()), + }); tcx.ty_error() } } diff --git a/compiler/rustc_typeck/src/errors.rs b/compiler/rustc_typeck/src/errors.rs index 3d2f93537e4e8..10c0fcd6bcec5 100644 --- a/compiler/rustc_typeck/src/errors.rs +++ b/compiler/rustc_typeck/src/errors.rs @@ -228,3 +228,12 @@ pub enum ExpectedReturnTypeLabel<'tcx> { expected: Ty<'tcx>, }, } + +#[derive(SessionDiagnostic)] +#[error(slug = "typeck-unconstrained-opaque-type")] +#[note] +pub struct UnconstrainedOpaqueType { + #[primary_span] + pub span: Span, + pub name: Symbol, +} From 859079ff127f8886c81152da7bb74b38f84b6797 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 6 May 2022 03:43:30 +0100 Subject: [PATCH 2/4] macros: allow `Vec` fields in diagnostic derive Diagnostics can have multiple primary spans, or have subdiagnostics repeated at multiple locations, so support `Vec<..>` fields in the diagnostic derive which become loops in the generated code. Signed-off-by: David Wood --- .../src/diagnostics/diagnostic.rs | 45 ++++++++------ .../src/diagnostics/subdiagnostic.rs | 18 ++---- .../rustc_macros/src/diagnostics/utils.rs | 61 +++++++++++++++++-- .../session-diagnostic/diagnostic-derive.rs | 8 +++ 4 files changed, 93 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_macros/src/diagnostics/diagnostic.rs b/compiler/rustc_macros/src/diagnostics/diagnostic.rs index f49166433faad..83fc7bcde8ab4 100644 --- a/compiler/rustc_macros/src/diagnostics/diagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/diagnostic.rs @@ -5,8 +5,8 @@ use crate::diagnostics::error::{ SessionDiagnosticDeriveError, }; use crate::diagnostics::utils::{ - option_inner_ty, report_error_if_not_applied_to_span, type_matches_path, Applicability, - FieldInfo, HasFieldMap, SetOnce, + report_error_if_not_applied_to_span, type_matches_path, Applicability, FieldInfo, FieldInnerTy, + HasFieldMap, SetOnce, }; use proc_macro2::TokenStream; use quote::{format_ident, quote}; @@ -353,35 +353,40 @@ impl SessionDiagnosticDeriveBuilder { info: FieldInfo<'_>, ) -> Result { let field_binding = &info.binding.binding; - let option_ty = option_inner_ty(&info.ty); - let generated_code = self.generate_non_option_field_code( + + let inner_ty = FieldInnerTy::from_type(&info.ty); + let name = attr.path.segments.last().unwrap().ident.to_string(); + let (binding, needs_destructure) = match (name.as_str(), &inner_ty) { + // `primary_span` can accept a `Vec` so don't destructure that. + ("primary_span", FieldInnerTy::Vec(_)) => (quote! { #field_binding.clone() }, false), + _ => (quote! { *#field_binding }, true), + }; + + let generated_code = self.generate_inner_field_code( attr, FieldInfo { vis: info.vis, binding: info.binding, - ty: option_ty.unwrap_or(&info.ty), + ty: inner_ty.inner_type().unwrap_or(&info.ty), span: info.span, }, + binding, )?; - if option_ty.is_none() { - Ok(quote! { #generated_code }) + if needs_destructure { + Ok(inner_ty.with(field_binding, generated_code)) } else { - Ok(quote! { - if let Some(#field_binding) = #field_binding { - #generated_code - } - }) + Ok(generated_code) } } - fn generate_non_option_field_code( + fn generate_inner_field_code( &mut self, attr: &Attribute, info: FieldInfo<'_>, + binding: TokenStream, ) -> Result { let diag = &self.diag; - let field_binding = &info.binding.binding; let name = attr.path.segments.last().unwrap().ident.to_string(); let name = name.as_str(); @@ -397,14 +402,14 @@ impl SessionDiagnosticDeriveBuilder { "primary_span" => { report_error_if_not_applied_to_span(attr, &info)?; Ok(quote! { - #diag.set_span(*#field_binding); + #diag.set_span(#binding); }) } "label" | "note" | "help" => { report_error_if_not_applied_to_span(attr, &info)?; - Ok(self.add_subdiagnostic(field_binding, name, name)) + Ok(self.add_subdiagnostic(binding, name, name)) } - "subdiagnostic" => Ok(quote! { #diag.subdiagnostic(*#field_binding); }), + "subdiagnostic" => Ok(quote! { #diag.subdiagnostic(#binding); }), _ => throw_invalid_attr!(attr, &meta, |diag| { diag .help("only `skip_arg`, `primary_span`, `label`, `note`, `help` and `subdiagnostic` are valid field attributes") @@ -413,7 +418,7 @@ impl SessionDiagnosticDeriveBuilder { Meta::NameValue(MetaNameValue { lit: syn::Lit::Str(ref s), .. }) => match name { "label" | "note" | "help" => { report_error_if_not_applied_to_span(attr, &info)?; - Ok(self.add_subdiagnostic(field_binding, name, &s.value())) + Ok(self.add_subdiagnostic(binding, name, &s.value())) } _ => throw_invalid_attr!(attr, &meta, |diag| { diag.help("only `label`, `note` and `help` are valid field attributes") @@ -509,7 +514,7 @@ impl SessionDiagnosticDeriveBuilder { /// `fluent_attr_identifier`. fn add_subdiagnostic( &self, - field_binding: &proc_macro2::Ident, + field_binding: TokenStream, kind: &str, fluent_attr_identifier: &str, ) -> TokenStream { @@ -520,7 +525,7 @@ impl SessionDiagnosticDeriveBuilder { let fn_name = format_ident!("span_{}", kind); quote! { #diag.#fn_name( - *#field_binding, + #field_binding, rustc_errors::DiagnosticMessage::fluent_attr(#slug, #fluent_attr_identifier) ); } diff --git a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs index 961b42f424fd1..65b1328682f82 100644 --- a/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs +++ b/compiler/rustc_macros/src/diagnostics/subdiagnostic.rs @@ -5,8 +5,8 @@ use crate::diagnostics::error::{ SessionDiagnosticDeriveError, }; use crate::diagnostics::utils::{ - option_inner_ty, report_error_if_not_applied_to_applicability, - report_error_if_not_applied_to_span, Applicability, FieldInfo, HasFieldMap, SetOnce, + report_error_if_not_applied_to_applicability, report_error_if_not_applied_to_span, + Applicability, FieldInfo, FieldInnerTy, HasFieldMap, SetOnce, }; use proc_macro2::TokenStream; use quote::{format_ident, quote}; @@ -301,11 +301,11 @@ impl<'a> SessionSubdiagnosticDeriveBuilder<'a> { ) -> Result { let ast = binding.ast(); - let option_ty = option_inner_ty(&ast.ty); + let inner_ty = FieldInnerTy::from_type(&ast.ty); let info = FieldInfo { vis: &ast.vis, binding: binding, - ty: option_ty.unwrap_or(&ast.ty), + ty: inner_ty.inner_type().unwrap_or(&ast.ty), span: &ast.span(), }; @@ -353,15 +353,7 @@ impl<'a> SessionSubdiagnosticDeriveBuilder<'a> { ); }; - if option_ty.is_none() { - Ok(quote! { #generated }) - } else { - Ok(quote! { - if let Some(#binding) = #binding { - #generated - } - }) - } + Ok(inner_ty.with(binding, generated)) } fn into_tokens(&mut self) -> Result { diff --git a/compiler/rustc_macros/src/diagnostics/utils.rs b/compiler/rustc_macros/src/diagnostics/utils.rs index 1f36af0a20bcd..aba861fc6aafa 100644 --- a/compiler/rustc_macros/src/diagnostics/utils.rs +++ b/compiler/rustc_macros/src/diagnostics/utils.rs @@ -1,7 +1,7 @@ use crate::diagnostics::error::{span_err, throw_span_err, SessionDiagnosticDeriveError}; use proc_macro::Span; use proc_macro2::TokenStream; -use quote::{format_ident, quote}; +use quote::{format_ident, quote, ToTokens}; use std::collections::BTreeSet; use std::str::FromStr; use syn::{spanned::Spanned, Attribute, Meta, Type, Visibility}; @@ -76,22 +76,71 @@ pub(crate) fn report_error_if_not_applied_to_span( report_error_if_not_applied_to_ty(attr, info, &["rustc_span", "Span"], "Span") } -/// If `ty` is an Option, returns `Some(inner type)`, otherwise returns `None`. -pub(crate) fn option_inner_ty(ty: &Type) -> Option<&Type> { - if type_matches_path(ty, &["std", "option", "Option"]) { +/// Inner type of a field and type of wrapper. +pub(crate) enum FieldInnerTy<'ty> { + /// Field is wrapped in a `Option<$inner>`. + Option(&'ty Type), + /// Field is wrapped in a `Vec<$inner>`. + Vec(&'ty Type), + /// Field isn't wrapped in an outer type. + None, +} + +impl<'ty> FieldInnerTy<'ty> { + /// Returns inner type for a field, if there is one. + /// + /// - If `ty` is an `Option`, returns `FieldInnerTy::Option { inner: (inner type) }`. + /// - If `ty` is a `Vec`, returns `FieldInnerTy::Vec { inner: (inner type) }`. + /// - Otherwise returns `None`. + pub(crate) fn from_type(ty: &'ty Type) -> Self { + let variant: &dyn Fn(&'ty Type) -> FieldInnerTy<'ty> = + if type_matches_path(ty, &["std", "option", "Option"]) { + &FieldInnerTy::Option + } else if type_matches_path(ty, &["std", "vec", "Vec"]) { + &FieldInnerTy::Vec + } else { + return FieldInnerTy::None; + }; + if let Type::Path(ty_path) = ty { let path = &ty_path.path; let ty = path.segments.iter().last().unwrap(); if let syn::PathArguments::AngleBracketed(bracketed) = &ty.arguments { if bracketed.args.len() == 1 { if let syn::GenericArgument::Type(ty) = &bracketed.args[0] { - return Some(ty); + return variant(ty); } } } } + + unreachable!(); + } + + /// Returns `Option` containing inner type if there is one. + pub(crate) fn inner_type(&self) -> Option<&'ty Type> { + match self { + FieldInnerTy::Option(inner) | FieldInnerTy::Vec(inner) => Some(inner), + FieldInnerTy::None => None, + } + } + + /// Surrounds `inner` with destructured wrapper type, exposing inner type as `binding`. + pub(crate) fn with(&self, binding: impl ToTokens, inner: impl ToTokens) -> TokenStream { + match self { + FieldInnerTy::Option(..) => quote! { + if let Some(#binding) = #binding { + #inner + } + }, + FieldInnerTy::Vec(..) => quote! { + for #binding in #binding { + #inner + } + }, + FieldInnerTy::None => quote! { #inner }, + } } - None } /// Field information passed to the builder. Deliberately omits attrs to discourage the diff --git a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs index efbf78ac87d73..c63410fa35bde 100644 --- a/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs +++ b/src/test/ui-fulldeps/session-diagnostic/diagnostic-derive.rs @@ -474,3 +474,11 @@ struct Subdiagnostic { #[subdiagnostic] note: Note, } + +#[derive(SessionDiagnostic)] +#[error(code = "E0123", slug = "foo")] +struct VecField { + #[primary_span] + #[label] + spans: Vec, +} From 3f413d2abb5cf5f72002bc7da5709bf6c4dab444 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 6 May 2022 03:44:41 +0100 Subject: [PATCH 3/4] sess: add `create_{err,warning}` Currently, the only API for creating errors from a diagnostic derive will emit it immediately. This makes it difficult to add subdiagnostics to diagnostics from the derive, so add `create_{err,warning}` functions that return the diagnostic without emitting it. Signed-off-by: David Wood --- compiler/rustc_session/src/parse.rs | 18 ++++++++++++++++-- compiler/rustc_session/src/session.rs | 12 ++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_session/src/parse.rs b/compiler/rustc_session/src/parse.rs index e933fe1cb2412..6fb87e15a3303 100644 --- a/compiler/rustc_session/src/parse.rs +++ b/compiler/rustc_session/src/parse.rs @@ -289,12 +289,26 @@ impl ParseSess { self.proc_macro_quoted_spans.lock().clone() } + pub fn create_err<'a>( + &'a self, + err: impl SessionDiagnostic<'a>, + ) -> DiagnosticBuilder<'a, ErrorGuaranteed> { + err.into_diagnostic(self) + } + pub fn emit_err<'a>(&'a self, err: impl SessionDiagnostic<'a>) -> ErrorGuaranteed { - err.into_diagnostic(self).emit() + self.create_err(err).emit() + } + + pub fn create_warning<'a>( + &'a self, + warning: impl SessionDiagnostic<'a, ()>, + ) -> DiagnosticBuilder<'a, ()> { + warning.into_diagnostic(self) } pub fn emit_warning<'a>(&'a self, warning: impl SessionDiagnostic<'a, ()>) { - warning.into_diagnostic(self).emit() + self.create_warning(warning).emit() } pub fn struct_err( diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index e8279f6fed24f..b2c23cda6aae5 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -413,9 +413,21 @@ impl Session { pub fn err(&self, msg: impl Into) -> ErrorGuaranteed { self.diagnostic().err(msg) } + pub fn create_err<'a>( + &'a self, + err: impl SessionDiagnostic<'a>, + ) -> DiagnosticBuilder<'a, ErrorGuaranteed> { + self.parse_sess.create_err(err) + } pub fn emit_err<'a>(&'a self, err: impl SessionDiagnostic<'a>) -> ErrorGuaranteed { self.parse_sess.emit_err(err) } + pub fn create_warning<'a>( + &'a self, + err: impl SessionDiagnostic<'a, ()>, + ) -> DiagnosticBuilder<'a, ()> { + self.parse_sess.create_warning(err) + } pub fn emit_warning<'a>(&'a self, warning: impl SessionDiagnostic<'a, ()>) { self.parse_sess.emit_warning(warning) } From af47257c0dfc5b38468417d36a465a613c675d6e Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 6 May 2022 03:46:12 +0100 Subject: [PATCH 4/4] typeck: port "explicit generic args w/ impl trait" Port the "explicit generic arguments with impl trait" diagnostic to using the diagnostic derive. Signed-off-by: David Wood --- .../locales/en-US/typeck.ftl | 8 ++++++ compiler/rustc_typeck/src/astconv/generics.rs | 28 ++++--------------- compiler/rustc_typeck/src/errors.rs | 13 +++++++++ 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_error_messages/locales/en-US/typeck.ftl b/compiler/rustc_error_messages/locales/en-US/typeck.ftl index 80eacd41add3c..aef18fcafaa05 100644 --- a/compiler/rustc_error_messages/locales/en-US/typeck.ftl +++ b/compiler/rustc_error_messages/locales/en-US/typeck.ftl @@ -93,3 +93,11 @@ typeck-expected-return-type = expected `{$expected}` because of return type typeck-unconstrained-opaque-type = unconstrained opaque type .note = `{$name}` must be used in combination with a concrete type within the same module + +typeck-explicit-generic-args-with-impl-trait = + cannot provide explicit generic arguments when `impl Trait` is used in argument position + .label = explicit generic argument not allowed + .note = see issue #83701 for more information + +typeck-explicit-generic-args-with-impl-trait-feature = + add `#![feature(explicit_generic_args_with_impl_trait)]` to the crate attributes to enable diff --git a/compiler/rustc_typeck/src/astconv/generics.rs b/compiler/rustc_typeck/src/astconv/generics.rs index 794e711b6c831..38c29d3874c9e 100644 --- a/compiler/rustc_typeck/src/astconv/generics.rs +++ b/compiler/rustc_typeck/src/astconv/generics.rs @@ -3,7 +3,10 @@ use crate::astconv::{ AstConv, CreateSubstsForGenericArgsCtxt, ExplicitLateBound, GenericArgCountMismatch, GenericArgCountResult, GenericArgPosition, }; -use crate::errors::AssocTypeBindingNotAllowed; +use crate::errors::{ + AssocTypeBindingNotAllowed, ExplicitGenericArgsWithImplTrait, + ExplicitGenericArgsWithImplTraitFeature, +}; use crate::structured_errors::{GenericArgsInfo, StructuredDiagnostic, WrongNumberOfGenericArgs}; use rustc_ast::ast::ParamKindOrd; use rustc_errors::{struct_span_err, Applicability, Diagnostic, MultiSpan}; @@ -636,29 +639,10 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o { }) .collect::>(); - let mut err = struct_span_err! { - tcx.sess, - spans.clone(), - E0632, - "cannot provide explicit generic arguments when `impl Trait` is \ - used in argument position" - }; - - for span in spans { - err.span_label(span, "explicit generic argument not allowed"); - } - - err.note( - "see issue #83701 \ - for more information", - ); + let mut err = tcx.sess.create_err(ExplicitGenericArgsWithImplTrait { spans }); if tcx.sess.is_nightly_build() { - err.help( - "add `#![feature(explicit_generic_args_with_impl_trait)]` \ - to the crate attributes to enable", - ); + err.subdiagnostic(ExplicitGenericArgsWithImplTraitFeature); } - err.emit(); } diff --git a/compiler/rustc_typeck/src/errors.rs b/compiler/rustc_typeck/src/errors.rs index 10c0fcd6bcec5..a3e7108caae00 100644 --- a/compiler/rustc_typeck/src/errors.rs +++ b/compiler/rustc_typeck/src/errors.rs @@ -237,3 +237,16 @@ pub struct UnconstrainedOpaqueType { pub span: Span, pub name: Symbol, } + +#[derive(SessionDiagnostic)] +#[error(code = "E0632", slug = "typeck-explicit-generic-args-with-impl-trait")] +#[note] +pub struct ExplicitGenericArgsWithImplTrait { + #[primary_span] + #[label] + pub spans: Vec, +} + +#[derive(SessionSubdiagnostic)] +#[help(slug = "typeck-explicit-generic-args-with-impl-trait-feature")] +pub struct ExplicitGenericArgsWithImplTraitFeature;