Skip to content

Split elided_lifetime_in_paths into finer-grained lints #120808

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3889,6 +3889,7 @@ dependencies = [
name = "rustc_hir"
version = "0.0.0"
dependencies = [
"bitflags",
"odht",
"rustc_abi",
"rustc_arena",
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_ast_lowering/src/delegation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use std::iter;

use ast::visit::Visitor;
use hir::def::{DefKind, PartialRes, Res};
use hir::{BodyId, HirId};
use hir::{BodyId, HirId, PathFlags};
use rustc_abi::ExternAbi;
use rustc_ast::*;
use rustc_errors::ErrorGuaranteed;
Expand Down Expand Up @@ -264,7 +264,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
hir_id: self.next_id(),
res: Res::Local(param_id),
args: None,
infer_args: false,
flags: PathFlags::empty(),
}));

let path = self.arena.alloc(hir::Path { span, res: Res::Local(param_id), segments });
Expand Down Expand Up @@ -366,6 +366,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
GenericArgsMode::Err,
ImplTraitContext::Disallowed(ImplTraitPosition::Path),
None,
PathFlags::empty(),
);
let segment = self.arena.alloc(segment);

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,9 @@ use rustc_ast::ptr::P as AstP;
use rustc_ast::*;
use rustc_ast_pretty::pprust::expr_to_string;
use rustc_data_structures::stack::ensure_sufficient_stack;
use rustc_hir as hir;
use rustc_hir::attrs::AttributeKind;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{HirId, find_attr};
use rustc_hir::{self as hir, HirId, PathFlags, find_attr};
use rustc_middle::span_bug;
use rustc_middle::ty::TyCtxt;
use rustc_session::errors::report_lit_error;
Expand Down Expand Up @@ -126,6 +125,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
ImplTraitContext::Disallowed(ImplTraitPosition::Path),
// Method calls can't have bound modifiers
None,
PathFlags::empty(),
));
let receiver = self.lower_expr(receiver);
let args =
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ use rustc_hir::def_id::{CRATE_DEF_ID, LOCAL_CRATE, LocalDefId};
use rustc_hir::lints::DelayedLint;
use rustc_hir::{
self as hir, AngleBrackets, ConstArg, GenericArg, HirId, ItemLocalMap, LifetimeSource,
LifetimeSyntax, ParamName, TraitCandidate,
LifetimeSyntax, ParamName, PathFlags, TraitCandidate,
};
use rustc_index::{Idx, IndexSlice, IndexVec};
use rustc_macros::extension;
Expand Down Expand Up @@ -781,6 +781,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let def_id = self.tcx.require_lang_item(lang_item, span);
let def_kind = self.tcx.def_kind(def_id);
let res = Res::Def(def_kind, def_id);
let mut flags = PathFlags::empty();
flags.set(PathFlags::INFER_ARGS, args.is_none());
self.arena.alloc(hir::Path {
span,
res,
Expand All @@ -789,7 +791,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
hir_id: self.next_id(),
res,
args,
infer_args: args.is_none(),
flags,
}]),
})
}
Expand Down
23 changes: 18 additions & 5 deletions compiler/rustc_ast_lowering/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::sync::Arc;
use rustc_ast::{self as ast, *};
use rustc_hir::def::{DefKind, PartialRes, PerNS, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::{self as hir, GenericArg};
use rustc_hir::{self as hir, GenericArg, PathFlags};
use rustc_middle::{span_bug, ty};
use rustc_session::parse::add_feature_diagnostics;
use rustc_span::{BytePos, DUMMY_SP, DesugaringKind, Ident, Span, Symbol, sym};
Expand Down Expand Up @@ -135,6 +135,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
generic_args_mode,
itctx(i),
bound_modifier_allowed_features.clone(),
PathFlags::empty(),
)
},
)),
Expand All @@ -160,16 +161,23 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}

// Create the innermost type that we're projecting from.
let mut ty = if path.segments.is_empty() {
let (mut ty, flags) = if path.segments.is_empty() {
// If the base path is empty that means there exists a
// syntactical `Self`, e.g., `&i32` in `<&i32>::clone`.
qself.expect("missing QSelf for <T>::...")
(qself.expect("missing QSelf for <T>::..."), PathFlags::empty())
} else {
// Otherwise, the base path is an implicit `Self` type path,
// e.g., `Vec` in `Vec::new` or `<I as Iterator>::Item` in
// `<I as Iterator>::Item::default`.
let new_id = self.next_id();
self.arena.alloc(self.ty_path(new_id, path.span, hir::QPath::Resolved(qself, path)))
(
&*self.arena.alloc(self.ty_path(
new_id,
path.span,
hir::QPath::Resolved(qself, path),
)),
PathFlags::IMPLICIT_SELF,
)
};

// Anything after the base path are associated "extensions",
Expand Down Expand Up @@ -199,6 +207,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
generic_args_mode,
itctx(i),
None,
flags,
));
let qpath = hir::QPath::TypeRelative(ty, hir_segment);

Expand Down Expand Up @@ -241,6 +250,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
GenericArgsMode::Err,
ImplTraitContext::Disallowed(ImplTraitPosition::Path),
None,
PathFlags::empty(),
)
})),
span: self.lower_span(p.span),
Expand All @@ -258,6 +268,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
// This is passed down to the implicit associated type binding in
// parenthesized bounds.
bound_modifier_allowed_features: Option<Arc<[Symbol]>>,
mut flags: PathFlags,
) -> hir::PathSegment<'hir> {
debug!("path_span: {:?}, lower_path_segment(segment: {:?})", path_span, segment);
let (mut generic_args, infer_args) = if let Some(generic_args) = segment.args.as_deref() {
Expand Down Expand Up @@ -400,11 +411,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
segment.ident, segment.id, hir_id,
);

flags.set(PathFlags::INFER_ARGS, infer_args);

hir::PathSegment {
ident: self.lower_ident(segment.ident),
hir_id,
res: self.lower_res(res),
infer_args,
flags,
args: if generic_args.is_empty() && generic_args.span.is_empty() {
None
} else {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_baked_icu_data/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@
//! ```

// tidy-alphabetical-start
#![allow(elided_lifetimes_in_paths)]
#![allow(internal_features)]
#![allow(unreachable_pub)] // because this crate is mostly generated code
#![doc(rust_logo)]
#![feature(rustdoc_internals)]
// #![warn(unreachable_pub)] // don't use because this crate is mostly generated code
// tidy-alphabetical-end
#![cfg_attr(bootstrap, allow(elided_lifetimes_in_paths))]
#![cfg_attr(not(bootstrap), allow(hidden_lifetimes_in_paths))]

mod data {
include!("data/mod.rs");
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ edition = "2024"

[dependencies]
# tidy-alphabetical-start
bitflags = "2.4.1"
odht = { version = "0.3.1", features = ["nightly"] }
rustc_abi = { path = "../rustc_abi" }
rustc_arena = { path = "../rustc_arena" }
Expand Down
41 changes: 35 additions & 6 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub use rustc_ast::{
};
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::sorted_map::SortedMap;
use rustc_data_structures::stable_hasher::HashStable;
use rustc_data_structures::tagged_ptr::TaggedRef;
use rustc_index::IndexVec;
use rustc_macros::{Decodable, Encodable, HashStable_Generic};
Expand Down Expand Up @@ -374,17 +375,37 @@ pub struct PathSegment<'hir> {
/// distinction.
pub args: Option<&'hir GenericArgs<'hir>>,

/// Whether to infer remaining type parameters, if any.
/// This only applies to expression and pattern paths, and
/// out of those only the segments with no type parameters
/// to begin with, e.g., `Vec::new` is `<Vec<..>>::new::<..>`.
pub infer_args: bool,
pub flags: PathFlags,
}

bitflags::bitflags! {
#[derive(Debug, Clone, Copy)]
pub struct PathFlags: u8 {
/// Whether to infer remaining type parameters, if any.
/// This only applies to expression and pattern paths, and
/// out of those only the segments with no type parameters
/// to begin with, e.g., `Vec::new` is `<Vec<..>>::new::<..>`.
const INFER_ARGS = 1 << 0;

/// Whether this path is an implicit `Self` type path.
const IMPLICIT_SELF = 1 << 1;
}
}

impl<CTX> HashStable<CTX> for PathFlags {
fn hash_stable(
&self,
hcx: &mut CTX,
hasher: &mut rustc_data_structures::stable_hasher::StableHasher,
) {
self.bits().hash_stable(hcx, hasher)
}
}

impl<'hir> PathSegment<'hir> {
/// Converts an identifier to the corresponding segment.
pub fn new(ident: Ident, hir_id: HirId, res: Res) -> PathSegment<'hir> {
PathSegment { ident, hir_id, res, infer_args: true, args: None }
PathSegment { ident, hir_id, res, flags: PathFlags::INFER_ARGS, args: None }
}

pub fn invalid() -> Self {
Expand All @@ -399,6 +420,14 @@ impl<'hir> PathSegment<'hir> {
DUMMY
}
}

pub fn infer_args(&self) -> bool {
self.flags.contains(PathFlags::INFER_ARGS)
}

pub fn implicit_self(&self) -> bool {
self.flags.contains(PathFlags::IMPLICIT_SELF)
}
}

/// A constant that enters the type system, used for arguments to const generics (e.g. array lengths).
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1427,7 +1427,7 @@ pub fn walk_path_segment<'v, V: Visitor<'v>>(
visitor: &mut V,
segment: &'v PathSegment<'v>,
) -> V::Result {
let PathSegment { ident, hir_id, res: _, args, infer_args: _ } = segment;
let PathSegment { ident, hir_id, res: _, args, flags: _ } = segment;
try_visit!(visitor.visit_ident(*ident));
try_visit!(visitor.visit_id(*hir_id));
visit_opt!(visitor, visit_generic_args, *args);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_errors::struct_span_code_err;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{CRATE_DEF_ID, DefId, LocalDefId};
use rustc_hir::{AmbigArg, PolyTraitRef};
use rustc_hir::{AmbigArg, PathFlags, PolyTraitRef};
use rustc_middle::bug;
use rustc_middle::ty::{
self as ty, IsSuggestable, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt,
Expand Down Expand Up @@ -602,7 +602,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
hir_id: constraint.hir_id,
res: Res::Err,
args: Some(constraint.gen_args),
infer_args: false,
flags: PathFlags::empty(),
};

let alias_args = self.lower_generic_args_of_assoc_item(
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,8 +427,8 @@ pub(crate) fn check_generic_arg_count(
})
.count();
let named_const_param_count = param_counts.consts - synth_const_param_count;
let infer_lifetimes =
(gen_pos != GenericArgPosition::Type || seg.infer_args) && !gen_args.has_lifetime_params();
let infer_lifetimes = (gen_pos != GenericArgPosition::Type || seg.infer_args())
&& !gen_args.has_lifetime_params();

if gen_pos != GenericArgPosition::Type
&& let Some(c) = gen_args.constraints.first()
Expand Down Expand Up @@ -577,7 +577,7 @@ pub(crate) fn check_generic_arg_count(
};

let args_correct = {
let expected_min = if seg.infer_args {
let expected_min = if seg.infer_args() {
0
} else {
param_counts.consts + named_type_param_count
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
def_id,
span,
generic_args: segment.args(),
infer_args: segment.infer_args,
infer_args: segment.infer_args(),
incorrect_args: &arg_count.correct,
};
let args = lower_generic_args(
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1238,10 +1238,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if !self.infer_args_for_err.contains(&index) {
// Check whether the user has provided generic arguments.
if let Some(data) = self.segments[index].args {
return (Some(data), self.segments[index].infer_args);
return (Some(data), self.segments[index].infer_args());
}
}
return (None, self.segments[index].infer_args);
return (None, self.segments[index].infer_args());
}

(None, true)
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,11 @@ lint_hidden_glob_reexport = private item shadows public glob re-export
.note_glob_reexport = the name `{$name}` in the {$namespace} namespace is supposed to be publicly re-exported here
.note_private_item = but the private item here shadows it
lint_hidden_lifetime_parameters = hidden lifetime parameters in types are deprecated
lint_hidden_lifetime_in_path =
paths containing hidden lifetime parameters are deprecated
Comment on lines +295 to +296
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion [WORDING-STRENGTH 1/2]: I think I agree with @tmandry's comment earlier

hidden lifetime parameters in types are deprecated

I also think we should remove this wording at least until we upgrade some lint to warn-by-default.

where if this set of 3 lints are initially allow-by-default in this PR, we should not use "deprecated" in this PR, but instead add "deprecated" in the follow-up PR that bumps these three lints and the hidden_lifetimes_in_paths to warn-by-default, so that the deprecation is part of the follow-up decision to explicitly deprecate this form.

In this PR, I would reword this less strong initially, maybe something like

paths containing hidden lifetime parameters can be confusing

Copy link
Contributor

Choose a reason for hiding this comment

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

We're still staying "deprecated" here. While at some level I do think that's correct, I'd highlight the comment by @tmandry that @jieyouxu highlighted above; there'd need to be a reply to this to argue for why it should be kept. We would almost certainly get pushback on this language.

lint_hidden_lifetime_in_path_suggestion =
indicate the anonymous lifetime
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
indicate the anonymous lifetime
indicate the inferred lifetime

I'd probably say "inferred" here for a couple reasons. One, I'm planning to update the Reference to call this the inferred lifetime (as we call _ the inferred type/const). Two, the lifetime isn't necessarily anonymous. It could be inferred to be a named lifetime, though of course we lint against that separately.

lint_hidden_unicode_codepoints = unicode codepoint changing visible direction of text present in {$label}
.label = this {$label} contains {$count ->
Expand Down
20 changes: 20 additions & 0 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,26 @@ impl LintStore {
self.by_name.insert(old_name.to_string(), Renamed(new_name.to_string(), target));
}

#[track_caller]
pub fn register_renamed_group(&mut self, old_name: &'static str, new_name: &'static str) {
let prev_lint = self.lint_groups.insert(
old_name,
LintGroup {
lint_ids: vec![],
is_externally_loaded: false,
depr: Some(LintAlias { name: new_name, silent: false }),
},
);

if prev_lint.is_some() {
bug!("The lint group {old_name} has already been registered");
}

if !self.lint_groups.contains_key(new_name) {
bug!("The lint group {new_name} has not been registered");
}
}

pub fn register_removed(&mut self, name: &str, reason: &str) {
self.by_name.insert(name.into(), Removed(reason.into()));
}
Expand Down
17 changes: 1 addition & 16 deletions compiler/rustc_lint/src/early/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
use std::borrow::Cow;

use rustc_ast::util::unicode::TEXT_FLOW_CONTROL_CHARS;
use rustc_errors::{
Applicability, Diag, DiagArgValue, LintDiagnostic, elided_lifetime_in_path_suggestion,
};
use rustc_errors::{Applicability, Diag, DiagArgValue, LintDiagnostic};
use rustc_middle::middle::stability;
use rustc_middle::ty::TyCtxt;
use rustc_session::Session;
Expand Down Expand Up @@ -72,19 +70,6 @@ pub fn decorate_builtin_lint(
lints::MacroExpandedMacroExportsAccessedByAbsolutePaths { definition: span_def }
.decorate_lint(diag)
}

BuiltinLintDiag::ElidedLifetimesInPaths(n, path_span, incl_angl_brckt, insertion_span) => {
lints::ElidedLifetimesInPaths {
subdiag: elided_lifetime_in_path_suggestion(
sess.source_map(),
n,
path_span,
incl_angl_brckt,
insertion_span,
),
}
.decorate_lint(diag);
}
BuiltinLintDiag::UnknownCrateTypes { span, candidate } => {
let sugg = candidate.map(|candidate| lints::UnknownCrateTypesSub { span, candidate });
lints::UnknownCrateTypes { sugg }.decorate_lint(diag);
Expand Down
Loading
Loading