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

Make it crystal clear what lint type_alias_bounds actually signifies #126575

Merged
merged 9 commits into from
Jul 26, 2024
19 changes: 12 additions & 7 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,18 @@ lint_builtin_special_module_name_used_main = found module declaration for main.r

lint_builtin_trivial_bounds = {$predicate_kind_name} bound {$predicate} does not depend on any type or lifetime parameters

lint_builtin_type_alias_bounds_help = use fully disambiguated paths (i.e., `<T as Trait>::Assoc`) to refer to associated types in type aliases

lint_builtin_type_alias_generic_bounds = bounds on generic parameters are not enforced in type aliases
.suggestion = the bound will not be checked when the type alias is used, and should be removed

lint_builtin_type_alias_where_clause = where clauses are not enforced in type aliases
.suggestion = the clause will not be checked when the type alias is used, and should be removed
lint_builtin_type_alias_bounds_enable_feat_help = add `#![feature(lazy_type_alias)]` to the crate attributes to enable the desired semantics
lint_builtin_type_alias_bounds_label = will not be checked at usage sites of the type alias
lint_builtin_type_alias_bounds_limitation_note = this is a known limitation of the type checker that may be lifted in a future edition.
Copy link
Member Author

Choose a reason for hiding this comment

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

Type checker / type system (implementation vs specification), it's not as clear-cut in Rust.

Copy link
Contributor

Choose a reason for hiding this comment

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

could just leave out the of the type checker part

Copy link
Member

Choose a reason for hiding this comment

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

nit .

Copy link
Member

Choose a reason for hiding this comment

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

oh, well actually this has a newline... hm.... i don't know how to best render this then

see issue #112792 <https://github.com/rust-lang/rust/issues/112792> for more information
lint_builtin_type_alias_bounds_param_bounds = bounds on generic parameters in type aliases are not enforced
.suggestion = remove {$count ->
[one] this bound
*[other] these bounds
}
lint_builtin_type_alias_bounds_qualify_assoc_tys_sugg = fully qualify this associated type
lint_builtin_type_alias_bounds_where_clause = where clauses on type aliases are not enforced
Copy link
Member Author

@fmease fmease Jun 17, 2024

Choose a reason for hiding this comment

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

Moving "in[/on] type aliases" before "not enforced" was not a stylistic decision.
The previous wording could be read as "not enforced within the body or definition of type aliases
which — while not entirely untrue (lack of item wfchecking) — would miss the point and might confuse
users who realize that trait bounds can indeed define/enable shorthand projections
(and are thus "enforced" for some unusual interpretation of the word like "to give strength or force to").

.suggestion = remove this where clause

lint_builtin_unpermitted_type_init_label = this code causes undefined behavior when executed
lint_builtin_unpermitted_type_init_label_suggestion = help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done
Expand Down
92 changes: 53 additions & 39 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ use crate::{
BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures, BuiltinKeywordIdents,
BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc,
BuiltinMutablesTransmutes, BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns,
BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, BuiltinTypeAliasGenericBounds,
BuiltinTypeAliasGenericBoundsSuggestion, BuiltinTypeAliasWhereClause,
BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit,
BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub, BuiltinUnsafe,
BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
BuiltinWhileTrue, InvalidAsmLabel, SuggestChangingAssocTypes,
BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, BuiltinTypeAliasBounds,
BuiltinTypeAliasParamBoundsSuggestion, BuiltinUngatedAsyncFnTrackCaller,
BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub,
BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment,
BuiltinUnusedDocCommentSub, BuiltinWhileTrue, InvalidAsmLabel,
TypeAliasBoundsQualifyAssocTysSugg,
},
EarlyContext, EarlyLintPass, LateContext, LateLintPass, Level, LintContext,
};
Expand Down Expand Up @@ -1406,23 +1406,6 @@ declare_lint_pass!(
TypeAliasBounds => [TYPE_ALIAS_BOUNDS]
);

impl TypeAliasBounds {
pub(crate) fn is_type_variable_assoc(qpath: &hir::QPath<'_>) -> bool {
match *qpath {
hir::QPath::TypeRelative(ty, _) => {
// If this is a type variable, we found a `T::Assoc`.
match ty.kind {
hir::TyKind::Path(hir::QPath::Resolved(None, path)) => {
matches!(path.res, Res::Def(DefKind::TyParam, _))
}
_ => false,
}
}
hir::QPath::Resolved(..) | hir::QPath::LangItem(..) => false,
}
}
}

impl<'tcx> LateLintPass<'tcx> for TypeAliasBounds {
fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) {
let hir::ItemKind::TyAlias(hir_ty, generics) = &item.kind else { return };
Expand All @@ -1437,7 +1420,6 @@ impl<'tcx> LateLintPass<'tcx> for TypeAliasBounds {
return;
}


// FIXME(generic_const_exprs): Revisit this before stabilization.
// See also `tests/ui/const-generics/generic_const_exprs/type-alias-bounds.rs`.
let ty = cx.tcx.type_of(item.owner_id).instantiate_identity();
Expand All @@ -1455,6 +1437,8 @@ impl<'tcx> LateLintPass<'tcx> for TypeAliasBounds {
let mut where_spans = Vec::new();
let mut inline_spans = Vec::new();
let mut inline_sugg = Vec::new();
let mut affects_object_lifetime_defaults = false;

for p in generics.predicates {
let span = p.span();
if p.in_where_clause() {
Expand All @@ -1465,31 +1449,61 @@ impl<'tcx> LateLintPass<'tcx> for TypeAliasBounds {
}
inline_sugg.push((span, String::new()));
}

// FIXME(fmease): Move this into a "diagnostic decorator" for increased laziness
// Bounds of the form `T: 'a` where `T` is a type param of
// the type alias affect object lifetime defaults.
if !affects_object_lifetime_defaults
&& let hir::WherePredicate::BoundPredicate(pred) = p
&& pred.bounds.iter().any(|bound| matches!(bound, hir::GenericBound::Outlives(_)))
&& pred.bound_generic_params.is_empty()
&& let hir::TyKind::Path(hir::QPath::Resolved(None, path)) = pred.bounded_ty.kind
&& let Res::Def(DefKind::TyParam, _) = path.res
{
affects_object_lifetime_defaults = true;
}
}

let mut suggested_changing_assoc_types = false;
if !where_spans.is_empty() {
let sub = (!suggested_changing_assoc_types).then(|| {
suggested_changing_assoc_types = true;
SuggestChangingAssocTypes { ty: hir_ty }
});
// FIXME(fmease): Add a disclaimer (in the form of a multi-span note) that the removal of
// type-param-outlives-bounds affects OLDs and explicit object lifetime
// bounds might be required [...].
// FIXME(fmease): The applicability should also depend on the outcome of the HIR walker
// inside of `TypeAliasBoundsQualifyAssocTysSugg`: Whether it found a
// shorthand projection or not.
let applicability = if affects_object_lifetime_defaults {
Applicability::MaybeIncorrect
} else {
Applicability::MachineApplicable
};

let mut qualify_assoc_tys_sugg = Some(TypeAliasBoundsQualifyAssocTysSugg { ty: hir_ty });
let enable_feat_help = cx.tcx.sess.is_nightly_build().then_some(());

if let [.., label_sp] = *where_spans {
cx.emit_span_lint(
TYPE_ALIAS_BOUNDS,
where_spans,
BuiltinTypeAliasWhereClause { suggestion: generics.where_clause_span, sub },
BuiltinTypeAliasBounds::WhereClause {
label: label_sp,
enable_feat_help,
suggestion: (generics.where_clause_span, applicability),
qualify_assoc_tys_sugg: qualify_assoc_tys_sugg.take(),
},
);
}

if !inline_spans.is_empty() {
let suggestion = BuiltinTypeAliasGenericBoundsSuggestion { suggestions: inline_sugg };
let sub = (!suggested_changing_assoc_types).then(|| {
suggested_changing_assoc_types = true;
SuggestChangingAssocTypes { ty: hir_ty }
});
if let [.., label_sp] = *inline_spans {
cx.emit_span_lint(
TYPE_ALIAS_BOUNDS,
inline_spans,
BuiltinTypeAliasGenericBounds { suggestion, sub },
BuiltinTypeAliasBounds::ParamBounds {
label: label_sp,
enable_feat_help,
suggestion: BuiltinTypeAliasParamBoundsSuggestion {
suggestions: inline_sugg,
applicability,
},
qualify_assoc_tys_sugg,
},
);
}
}
Expand Down
148 changes: 83 additions & 65 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_errors::{
ElidedLifetimeInPathSubdiag, EmissionGuarantee, LintDiagnostic, MultiSpan, SubdiagMessageOp,
Subdiagnostic, SuggestionStyle,
};
use rustc_hir::{def::Namespace, def_id::DefId};
use rustc_hir::{self as hir, def::Namespace, def_id::DefId};
use rustc_macros::{LintDiagnostic, Subdiagnostic};
use rustc_middle::ty::{
inhabitedness::InhabitedPredicate, Clause, PolyExistentialTraitRef, Ty, TyCtxt,
Expand All @@ -22,9 +22,7 @@ use rustc_span::{
Span, Symbol,
};

use crate::{
builtin::InitError, builtin::TypeAliasBounds, errors::OverruledAttributeSub, LateContext,
};
use crate::{builtin::InitError, errors::OverruledAttributeSub, LateContext};

// array_into_iter.rs
#[derive(LintDiagnostic)]
Expand Down Expand Up @@ -263,84 +261,104 @@ pub struct BuiltinUnreachablePub<'a> {
pub help: Option<()>,
}

pub struct SuggestChangingAssocTypes<'a, 'b> {
pub ty: &'a rustc_hir::Ty<'b>,
#[derive(LintDiagnostic)]
#[diag(lint_macro_expr_fragment_specifier_2024_migration)]
pub struct MacroExprFragment2024 {
#[suggestion(code = "expr_2021", applicability = "machine-applicable")]
pub suggestion: Span,
}

#[derive(LintDiagnostic)]
pub enum BuiltinTypeAliasBounds<'a, 'hir> {
#[diag(lint_builtin_type_alias_bounds_where_clause)]
#[note(lint_builtin_type_alias_bounds_limitation_note)]
WhereClause {
#[label(lint_builtin_type_alias_bounds_label)]
label: Span,
#[help(lint_builtin_type_alias_bounds_enable_feat_help)]
enable_feat_help: Option<()>,
#[suggestion(code = "")]
suggestion: (Span, Applicability),
#[subdiagnostic]
qualify_assoc_tys_sugg: Option<TypeAliasBoundsQualifyAssocTysSugg<'a, 'hir>>,
},
#[diag(lint_builtin_type_alias_bounds_param_bounds)]
#[note(lint_builtin_type_alias_bounds_limitation_note)]
ParamBounds {
#[label(lint_builtin_type_alias_bounds_label)]
label: Span,
#[help(lint_builtin_type_alias_bounds_enable_feat_help)]
enable_feat_help: Option<()>,
#[subdiagnostic]
suggestion: BuiltinTypeAliasParamBoundsSuggestion,
#[subdiagnostic]
qualify_assoc_tys_sugg: Option<TypeAliasBoundsQualifyAssocTysSugg<'a, 'hir>>,
},
}

pub struct BuiltinTypeAliasParamBoundsSuggestion {
pub suggestions: Vec<(Span, String)>,
pub applicability: Applicability,
}

impl<'a, 'b> Subdiagnostic for SuggestChangingAssocTypes<'a, 'b> {
impl Subdiagnostic for BuiltinTypeAliasParamBoundsSuggestion {
fn add_to_diag_with<G: EmissionGuarantee, F: SubdiagMessageOp<G>>(
self,
diag: &mut Diag<'_, G>,
_f: &F,
) {
// Access to associates types should use `<T as Bound>::Assoc`, which does not need a
// bound. Let's see if this type does that.

// We use a HIR visitor to walk the type.
use rustc_hir::intravisit::{self, Visitor};
struct WalkAssocTypes<'a, 'b, G: EmissionGuarantee> {
err: &'a mut Diag<'b, G>,
}
impl<'a, 'b, G: EmissionGuarantee> Visitor<'_> for WalkAssocTypes<'a, 'b, G> {
fn visit_qpath(
&mut self,
qpath: &rustc_hir::QPath<'_>,
id: rustc_hir::HirId,
span: Span,
) {
if TypeAliasBounds::is_type_variable_assoc(qpath) {
self.err.span_help(span, fluent::lint_builtin_type_alias_bounds_help);
}
intravisit::walk_qpath(self, qpath, id)
}
}

// Let's go for a walk!
let mut visitor = WalkAssocTypes { err: diag };
visitor.visit_ty(self.ty);
diag.arg("count", self.suggestions.len());
diag.multipart_suggestion(fluent::lint_suggestion, self.suggestions, self.applicability);
}
}

#[derive(LintDiagnostic)]
#[diag(lint_builtin_type_alias_where_clause)]
pub struct BuiltinTypeAliasWhereClause<'a, 'b> {
#[suggestion(code = "", applicability = "machine-applicable")]
pub suggestion: Span,
#[subdiagnostic]
pub sub: Option<SuggestChangingAssocTypes<'a, 'b>>,
}

#[derive(LintDiagnostic)]
#[diag(lint_builtin_type_alias_generic_bounds)]
pub struct BuiltinTypeAliasGenericBounds<'a, 'b> {
#[subdiagnostic]
pub suggestion: BuiltinTypeAliasGenericBoundsSuggestion,
#[subdiagnostic]
pub sub: Option<SuggestChangingAssocTypes<'a, 'b>>,
}

#[derive(LintDiagnostic)]
#[diag(lint_macro_expr_fragment_specifier_2024_migration)]
pub struct MacroExprFragment2024 {
#[suggestion(code = "expr_2021", applicability = "machine-applicable")]
pub suggestion: Span,
pub struct TypeAliasBoundsQualifyAssocTysSugg<'a, 'hir> {
pub ty: &'a hir::Ty<'hir>,
}

pub struct BuiltinTypeAliasGenericBoundsSuggestion {
pub suggestions: Vec<(Span, String)>,
}

impl Subdiagnostic for BuiltinTypeAliasGenericBoundsSuggestion {
impl<'a, 'hir> Subdiagnostic for TypeAliasBoundsQualifyAssocTysSugg<'a, 'hir> {
fn add_to_diag_with<G: EmissionGuarantee, F: SubdiagMessageOp<G>>(
self,
diag: &mut Diag<'_, G>,
_f: &F,
) {
diag.multipart_suggestion(
fluent::lint_suggestion,
self.suggestions,
Applicability::MachineApplicable,
);
// We perform the walk in here instead of in `<TypeAliasBounds as LateLintPass>` to
// avoid doing throwaway work in case the lint ends up getting suppressed.

use hir::intravisit::Visitor;
struct ProbeShorthandAssocTys<'a, 'b, G: EmissionGuarantee> {
diag: &'a mut Diag<'b, G>,
}
impl<'a, 'b, G: EmissionGuarantee> Visitor<'_> for ProbeShorthandAssocTys<'a, 'b, G> {
fn visit_qpath(&mut self, qpath: &hir::QPath<'_>, id: hir::HirId, _: Span) {
// Look for "type-parameter shorthand-associated-types". I.e., paths of the
// form `T::Assoc` with `T` type param. These are reliant on trait bounds.
// Suggest fully qualifying them via `<T as /* Trait */>::Assoc`.
//
// Instead of attempting to figure out the necessary trait ref, just use a
// placeholder. Since we don't record type-dependent resolutions for non-body
// items like type aliases, we can't simply deduce the corresp. trait from
// the HIR path alone without rerunning parts of HIR ty lowering here
// (namely `probe_single_ty_param_bound_for_assoc_ty`) which is infeasible.
//
// (We could employ some simple heuristics but that's likely not worth it).
if let hir::QPath::TypeRelative(qself, _) = qpath
&& let hir::TyKind::Path(hir::QPath::Resolved(None, path)) = qself.kind
&& let hir::def::Res::Def(hir::def::DefKind::TyParam, _) = path.res
{
self.diag.multipart_suggestion(
fluent::lint_builtin_type_alias_bounds_qualify_assoc_tys_sugg,
vec![
(qself.span.shrink_to_lo(), "<".into()),
(qself.span.shrink_to_hi(), " as /* Trait */>".into()),
],
Applicability::HasPlaceholders,
);
}
hir::intravisit::walk_qpath(self, qpath, id)
}
}
ProbeShorthandAssocTys { diag }.visit_ty(self.ty);
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/ui/associated-inherent-types/type-alias-bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
// automatically lead to full wfchecking and lint TAB getting suppressed.

pub type Alias<T: Bound> = (Source<T>::Assoc,);
//~^ WARN bounds on generic parameters are not enforced in type aliases
//~^ WARN bounds on generic parameters in type aliases are not enforced

pub struct Source<T>(T);
pub trait Bound {}
Expand Down
15 changes: 8 additions & 7 deletions tests/ui/associated-inherent-types/type-alias-bounds.stderr
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
warning: bounds on generic parameters are not enforced in type aliases
warning: bounds on generic parameters in type aliases are not enforced
--> $DIR/type-alias-bounds.rs:21:19
|
LL | pub type Alias<T: Bound> = (Source<T>::Assoc,);
| ^^^^^
| --^^^^^
| | |
| | will not be checked at usage sites of the type alias
| help: remove this bound
|
= note: this is a known limitation of the type checker that may be lifted in a future edition.
see issue #112792 <https://github.com/rust-lang/rust/issues/112792> for more information
= help: add `#![feature(lazy_type_alias)]` to the crate attributes to enable the desired semantics
= note: `#[warn(type_alias_bounds)]` on by default
help: the bound will not be checked when the type alias is used, and should be removed
|
LL - pub type Alias<T: Bound> = (Source<T>::Assoc,);
LL + pub type Alias<T> = (Source<T>::Assoc,);
|

warning: 1 warning emitted

Loading