Skip to content

Commit

Permalink
Rollup merge of rust-lang#99249 - cjgillot:no-reparse-fn, r=fee1-dead
Browse files Browse the repository at this point in the history
Do not re-parse function signatures to suggest generics

This PR uses the existing resolution rib infrastructure to channel the correct span information to suggest generic parameters.  This allows to avoid re-parsing a function's source code.

Drive-by cleanup: this removes useless `FnItemRibKind` from late resolution ribs.  All the use cases are already covered by `ItemRibKind` and `AssocItemRibKind` which have more precise semantics.
  • Loading branch information
Dylan-DPC authored Aug 23, 2022
2 parents 38528d4 + dff4280 commit d3d8463
Show file tree
Hide file tree
Showing 18 changed files with 134 additions and 218 deletions.
33 changes: 15 additions & 18 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ impl<'a> Resolver<'a> {
err.span_label(span, "use of generic parameter from outer function");

let sm = self.session.source_map();
match outer_res {
let def_id = match outer_res {
Res::SelfTy { trait_: maybe_trait_defid, alias_to: maybe_impl_defid } => {
if let Some(impl_span) =
maybe_impl_defid.and_then(|(def_id, _)| self.opt_span(def_id))
Expand All @@ -536,40 +536,37 @@ impl<'a> Resolver<'a> {
if let Some(span) = self.opt_span(def_id) {
err.span_label(span, "type parameter from outer function");
}
def_id
}
Res::Def(DefKind::ConstParam, def_id) => {
if let Some(span) = self.opt_span(def_id) {
err.span_label(span, "const parameter from outer function");
}
def_id
}
_ => {
bug!(
"GenericParamsFromOuterFunction should only be used with Res::SelfTy, \
DefKind::TyParam or DefKind::ConstParam"
);
}
}
};

if has_generic_params == HasGenericParams::Yes {
if let HasGenericParams::Yes(span) = has_generic_params {
// Try to retrieve the span of the function signature and generate a new
// message with a local type or const parameter.
let sugg_msg = "try using a local generic parameter instead";
if let Some((sugg_span, snippet)) = sm.generate_local_type_param_snippet(span) {
// Suggest the modification to the user
err.span_suggestion(
sugg_span,
sugg_msg,
snippet,
Applicability::MachineApplicable,
);
} else if let Some(sp) = sm.generate_fn_name_span(span) {
err.span_label(
sp,
"try adding a local generic parameter in this method instead",
);
let name = self.opt_name(def_id).unwrap_or(sym::T);
let (span, snippet) = if span.is_empty() {
let snippet = format!("<{}>", name);
(span, snippet)
} else {
err.help("try using a local generic parameter instead");
}
let span = sm.span_through_char(span, '<').shrink_to_hi();
let snippet = format!("{}, ", name);
(span, snippet)
};
// Suggest the modification to the user
err.span_suggestion(span, sugg_msg, snippet, Applicability::MaybeIncorrect);
}

err
Expand Down
28 changes: 12 additions & 16 deletions compiler/rustc_resolve/src/ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ use rustc_span::{Span, DUMMY_SP};

use std::ptr;

use crate::late::{ConstantItemKind, HasGenericParams, PathSource, Rib, RibKind};
use crate::late::{
ConstantHasGenerics, ConstantItemKind, HasGenericParams, PathSource, Rib, RibKind,
};
use crate::macros::{sub_namespace_match, MacroRulesScope};
use crate::{AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, Determinacy, Finalize};
use crate::{ImportKind, LexicalScopeBinding, Module, ModuleKind, ModuleOrUniformRoot};
Expand Down Expand Up @@ -1103,7 +1105,7 @@ impl<'a> Resolver<'a> {
| ForwardGenericParamBanRibKind => {
// Nothing to do. Continue.
}
ItemRibKind(_) | FnItemRibKind | AssocItemRibKind => {
ItemRibKind(_) | AssocItemRibKind => {
// This was an attempt to access an upvar inside a
// named function item. This is not allowed, so we
// report an error.
Expand Down Expand Up @@ -1168,10 +1170,10 @@ impl<'a> Resolver<'a> {
let has_generic_params: HasGenericParams = match rib.kind {
NormalRibKind
| ClosureOrAsyncRibKind
| AssocItemRibKind
| ModuleRibKind(..)
| MacroDefinition(..)
| InlineAsmSymRibKind
| AssocItemRibKind
| ForwardGenericParamBanRibKind => {
// Nothing to do. Continue.
continue;
Expand All @@ -1180,7 +1182,9 @@ impl<'a> Resolver<'a> {
ConstantItemRibKind(trivial, _) => {
let features = self.session.features_untracked();
// HACK(min_const_generics): We currently only allow `N` or `{ N }`.
if !(trivial == HasGenericParams::Yes || features.generic_const_exprs) {
if !(trivial == ConstantHasGenerics::Yes
|| features.generic_const_exprs)
{
// HACK(min_const_generics): If we encounter `Self` in an anonymous constant
// we can't easily tell if it's generic at this stage, so we instead remember
// this and then enforce the self type to be concrete later on.
Expand All @@ -1207,7 +1211,6 @@ impl<'a> Resolver<'a> {

// This was an attempt to use a type parameter outside its scope.
ItemRibKind(has_generic_params) => has_generic_params,
FnItemRibKind => HasGenericParams::Yes,
ConstParamTyRibKind => {
if let Some(span) = finalize {
self.report_error(
Expand All @@ -1232,28 +1235,22 @@ impl<'a> Resolver<'a> {
}
}
Res::Def(DefKind::ConstParam, _) => {
let mut ribs = ribs.iter().peekable();
if let Some(Rib { kind: FnItemRibKind, .. }) = ribs.peek() {
// When declaring const parameters inside function signatures, the first rib
// is always a `FnItemRibKind`. In this case, we can skip it, to avoid it
// (spuriously) conflicting with the const param.
ribs.next();
}

for rib in ribs {
let has_generic_params = match rib.kind {
NormalRibKind
| ClosureOrAsyncRibKind
| AssocItemRibKind
| ModuleRibKind(..)
| MacroDefinition(..)
| InlineAsmSymRibKind
| AssocItemRibKind
| ForwardGenericParamBanRibKind => continue,

ConstantItemRibKind(trivial, _) => {
let features = self.session.features_untracked();
// HACK(min_const_generics): We currently only allow `N` or `{ N }`.
if !(trivial == HasGenericParams::Yes || features.generic_const_exprs) {
if !(trivial == ConstantHasGenerics::Yes
|| features.generic_const_exprs)
{
if let Some(span) = finalize {
self.report_error(
span,
Expand All @@ -1272,7 +1269,6 @@ impl<'a> Resolver<'a> {
}

ItemRibKind(has_generic_params) => has_generic_params,
FnItemRibKind => HasGenericParams::Yes,
ConstParamTyRibKind => {
if let Some(span) = finalize {
self.report_error(
Expand Down
Loading

0 comments on commit d3d8463

Please sign in to comment.