Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

diagnostics: port more diagnostics to derive + add support for Vec fields #96760

Merged
merged 4 commits into from
May 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions compiler/rustc_error_messages/locales/en-US/typeck.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,14 @@ 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

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 <https://github.com/rust-lang/rust/issues/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
45 changes: 25 additions & 20 deletions compiler/rustc_macros/src/diagnostics/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -353,35 +353,40 @@ impl SessionDiagnosticDeriveBuilder {
info: FieldInfo<'_>,
) -> Result<TokenStream, SessionDiagnosticDeriveError> {
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<Span>` 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<TokenStream, SessionDiagnosticDeriveError> {
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();
Expand All @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
);
}
Expand Down
18 changes: 5 additions & 13 deletions compiler/rustc_macros/src/diagnostics/subdiagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -301,11 +301,11 @@ impl<'a> SessionSubdiagnosticDeriveBuilder<'a> {
) -> Result<TokenStream, SessionDiagnosticDeriveError> {
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(),
};

Expand Down Expand Up @@ -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<TokenStream, SessionDiagnosticDeriveError> {
Expand Down
61 changes: 55 additions & 6 deletions compiler/rustc_macros/src/diagnostics/utils.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ok for now, it's just inside rustc after all, but in the future we should probably invent a new trait that we can auto-impl for all subdiagnostics and manually impl for vec and option. The problem with the current approach is that it's dependent on exact names in the source, without actually tying them to the type we expect.

/// 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
Expand Down
18 changes: 16 additions & 2 deletions compiler/rustc_session/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,9 +413,21 @@ impl Session {
pub fn err(&self, msg: impl Into<DiagnosticMessage>) -> 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)
}
Expand Down
28 changes: 6 additions & 22 deletions compiler/rustc_typeck/src/astconv/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -636,29 +639,10 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
})
.collect::<Vec<_>>();

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 <https://github.com/rust-lang/rust/issues/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();
}

Expand Down
12 changes: 5 additions & 7 deletions compiler/rustc_typeck/src/collect/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -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()
}
}
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_typeck/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,3 +228,25 @@ 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,
}

#[derive(SessionDiagnostic)]
#[error(code = "E0632", slug = "typeck-explicit-generic-args-with-impl-trait")]
#[note]
pub struct ExplicitGenericArgsWithImplTrait {
#[primary_span]
#[label]
pub spans: Vec<Span>,
}

#[derive(SessionSubdiagnostic)]
#[help(slug = "typeck-explicit-generic-args-with-impl-trait-feature")]
pub struct ExplicitGenericArgsWithImplTraitFeature;
Loading