Skip to content

Commit

Permalink
Auto merge of rust-lang#129249 - estebank:useless-into, r=<try>
Browse files Browse the repository at this point in the history
[Experimental] `<T as Into<T>>::into` lint

Running crater to see how common that pattern is. The Lint would have to be at most warn-by-default because there are a handful of cases detected that are actually perfectly reasonable (`type` aliases with per-platform `cfg`, or macros) which are now at best half-heartedly handled.

I've detected a handful of cases where we're calling `.into()` unnecessarily in the `rustc` codebase as well, and changed those.
  • Loading branch information
bors committed Aug 19, 2024
2 parents 4fe1e2b + bd05c40 commit dd5e794
Show file tree
Hide file tree
Showing 30 changed files with 224 additions and 37 deletions.
6 changes: 2 additions & 4 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3464,7 +3464,7 @@ impl From<ForeignItemKind> for ItemKind {
fn from(foreign_item_kind: ForeignItemKind) -> ItemKind {
match foreign_item_kind {
ForeignItemKind::Static(box static_foreign_item) => {
ItemKind::Static(Box::new(static_foreign_item.into()))
ItemKind::Static(Box::new(static_foreign_item))
}
ForeignItemKind::Fn(fn_kind) => ItemKind::Fn(fn_kind),
ForeignItemKind::TyAlias(ty_alias_kind) => ItemKind::TyAlias(ty_alias_kind),
Expand All @@ -3478,9 +3478,7 @@ impl TryFrom<ItemKind> for ForeignItemKind {

fn try_from(item_kind: ItemKind) -> Result<ForeignItemKind, ItemKind> {
Ok(match item_kind {
ItemKind::Static(box static_item) => {
ForeignItemKind::Static(Box::new(static_item.into()))
}
ItemKind::Static(box static_item) => ForeignItemKind::Static(Box::new(static_item)),
ItemKind::Fn(fn_kind) => ForeignItemKind::Fn(fn_kind),
ItemKind::TyAlias(ty_alias_kind) => ForeignItemKind::TyAlias(ty_alias_kind),
ItemKind::MacCall(a) => ForeignItemKind::MacCall(a),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ fn eval_body_using_ecx<'tcx, R: InterpretationResult<'tcx>>(
ecx.push_stack_frame_raw(
cid.instance,
body,
&ret.clone().into(),
&ret.clone(),
StackPopCleanup::Root { cleanup: false },
)?;
ecx.storage_live_for_always_live_locals()?;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/interpret/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
(Abi::Rust, fn_abi),
&[FnArg::Copy(arg.into())],
false,
&ret.into(),
&ret,
Some(target),
unwind,
)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2533,7 +2533,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
other_generic_param.name.ident() == generic_param.name.ident()
},
) {
idxs_matched.push(other_idx.into());
idxs_matched.push(other_idx);
}

if idxs_matched.is_empty() {
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,8 @@ lint_reserved_prefix = prefix `{$prefix}` is unknown
.label = unknown prefix
.suggestion = insert whitespace here to avoid this being parsed as a prefix in Rust 2021
lint_self_type_conversion = this conversion is useless `{$source}` to `{$target}`
lint_shadowed_into_iter =
this method call resolves to `<&{$target} as IntoIterator>::into_iter` (due to backwards compatibility), but will resolve to `<{$target} as IntoIterator>::into_iter` in Rust {$edition}
.use_iter_suggestion = use `.iter()` instead of `.into_iter()` to avoid ambiguity
Expand Down
100 changes: 99 additions & 1 deletion compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ use crate::lints::{
BuiltinTrivialBounds, BuiltinTypeAliasBounds, BuiltinUngatedAsyncFnTrackCaller,
BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub,
BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
BuiltinWhileTrue, InvalidAsmLabel,
BuiltinWhileTrue, InvalidAsmLabel, SelfTypeConversionDiag,
};
use crate::nonstandard_style::{method_context, MethodLateContext};
use crate::{
Expand Down Expand Up @@ -1604,6 +1604,7 @@ declare_lint_pass!(
SoftLints => [
WHILE_TRUE,
NON_SHORTHAND_FIELD_PATTERNS,
SELF_TYPE_CONVERSION,
UNSAFE_CODE,
MISSING_DOCS,
MISSING_COPY_IMPLEMENTATIONS,
Expand Down Expand Up @@ -3064,3 +3065,100 @@ impl EarlyLintPass for SpecialModuleName {
}
}
}

declare_lint! {
/// The `self_type_conversion` lint detects when a call to `.into()` does not have any effect.
///
/// ### Example
///
/// ```rust,compile_fail
/// fn main() {
/// let () = ().into();
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
pub SELF_TYPE_CONVERSION,
Deny,
"",
}

pub struct SelfTypeConversion<'tcx> {
ignored_types: Vec<Ty<'tcx>>,
}

impl_lint_pass!(SelfTypeConversion<'_> => [SELF_TYPE_CONVERSION]);

impl SelfTypeConversion<'_> {
pub fn new() -> Self {
Self { ignored_types: vec![] }
}
}

impl<'tcx> LateLintPass<'tcx> for SelfTypeConversion<'tcx> {
fn check_item_post(&mut self, cx: &LateContext<'tcx>, item: &hir::Item<'_>) {
let hir::ItemKind::Use(path, kind) = item.kind else { return };
tracing::info!("{:#?}", item);
tracing::info!(?path, ?kind);
for res in &path.res {
let Res::Def(DefKind::TyAlias, def_id) = res else { continue };
let ty = cx.tcx.type_of(def_id).instantiate_identity();
let name = cx.tcx.item_name(*def_id);
// println!("{ty:?} {name:?}");
self.ignored_types.push(ty);
for stripped in cx.tcx.stripped_cfg_items(def_id.krate) {
if stripped.name.name == name {
tracing::info!("{name:#?}");
}
}
}
}

fn check_expr_post(&mut self, cx: &LateContext<'tcx>, expr: &hir::Expr<'_>) {
let hir::ExprKind::MethodCall(_segment, rcvr, args, _) = expr.kind else { return };
if !args.is_empty() {
tracing::info!("non-empty args");
return;
}
let ty = cx.typeck_results().expr_ty(expr);
let rcvr_ty = cx.typeck_results().expr_ty(rcvr);
tracing::info!(?ty, ?rcvr_ty);

if ty != rcvr_ty {
tracing::info!("different types");
return;
}
if self.ignored_types.contains(&rcvr_ty) {
return;
}
let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) else {
tracing::info!("no type dependent def id");
return;
};
tracing::info!(?def_id);
if Some(def_id) != cx.tcx.get_diagnostic_item(sym::into_fn) {
tracing::info!("not into_fn {:?}", cx.tcx.get_diagnostic_item(sym::into_fn));
return;
}
tracing::info!(?def_id);
tracing::info!(?expr);
if expr.span.macro_backtrace().next().is_some() {
return;
}
if cx.tcx.sess.source_map().span_to_embeddable_string(expr.span).contains("symbolize/gimli")
{
// HACK
return;
}
// println!("{:#?}", self.ignored_types);
cx.emit_span_lint(
SELF_TYPE_CONVERSION,
expr.span,
SelfTypeConversionDiag { source: rcvr_ty, target: ty },
);
// bug!("asdf");
}
}
5 changes: 4 additions & 1 deletion compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ early_lint_methods!(
late_lint_methods!(
declare_combined_late_lint_pass,
[
BuiltinCombinedModuleLateLintPass,
BuiltinCombinedModuleLateLintPass<'tcx>,
[
ForLoopsOverFallibles: ForLoopsOverFallibles,
DerefIntoDynSupertrait: DerefIntoDynSupertrait,
Expand All @@ -203,6 +203,7 @@ late_lint_methods!(
UnitBindings: UnitBindings,
NonUpperCaseGlobals: NonUpperCaseGlobals,
NonShorthandFieldPatterns: NonShorthandFieldPatterns,
SelfTypeConversion<'tcx>: SelfTypeConversion::new(),
UnusedAllocation: UnusedAllocation,
// Depends on types used in type definitions
MissingCopyImplementations: MissingCopyImplementations,
Expand Down Expand Up @@ -268,6 +269,7 @@ fn register_builtins(store: &mut LintStore) {
store.register_lints(&BuiltinCombinedModuleLateLintPass::get_lints());
store.register_lints(&foreign_modules::get_lints());

store.register_late_pass(move |_tcx| Box::new(SelfTypeConversion::new()));
add_lint_group!(
"nonstandard_style",
NON_CAMEL_CASE_TYPES,
Expand Down Expand Up @@ -298,6 +300,7 @@ fn register_builtins(store: &mut LintStore) {
UNUSED_PARENS,
UNUSED_BRACES,
REDUNDANT_SEMICOLONS,
// SELF_TYPE_CONVERSION,
MAP_UNIT_FN
);

Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,13 @@ pub struct BuiltinNonShorthandFieldPatterns {
pub prefix: &'static str,
}

#[derive(LintDiagnostic)]
#[diag(lint_self_type_conversion)]
pub struct SelfTypeConversionDiag<'t> {
pub source: Ty<'t>,
pub target: Ty<'t>,
}

#[derive(LintDiagnostic)]
pub enum BuiltinUnsafe {
#[diag(lint_builtin_allow_internal_unsafe)]
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_lint/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,13 @@ macro_rules! expand_combined_late_lint_pass_methods {
/// runtime.
#[macro_export]
macro_rules! declare_combined_late_lint_pass {
([$v:vis $name:ident, [$($pass:ident: $constructor:expr,)*]], $methods:tt) => (
([$v:vis $name:ident$(<$lt:lifetime>)?, [$($pass:ident$(<$inner_lt:lifetime>)?: $constructor:expr,)*]], $methods:tt) => (
#[allow(non_snake_case)]
$v struct $name {
$($pass: $pass,)*
$v struct $name$(<$lt>)? {
$($pass: $pass$(<$inner_lt>)?,)*
}

impl $name {
impl$(<$lt>)? $name$(<$lt>)? {
$v fn new() -> Self {
Self {
$($pass: $constructor,)*
Expand All @@ -113,12 +113,12 @@ macro_rules! declare_combined_late_lint_pass {
}
}

impl<'tcx> $crate::LateLintPass<'tcx> for $name {
impl<'tcx> $crate::LateLintPass<'tcx> for $name$(<$lt>)? {
$crate::expand_combined_late_lint_pass_methods!([$($pass),*], $methods);
}

#[allow(rustc::lint_pass_impl_without_macro)]
impl $crate::LintPass for $name {
impl$(<$lt>)? $crate::LintPass for $name$(<$lt>)? {
fn name(&self) -> &'static str {
panic!()
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ impl<'tcx> Const<'tcx> {
Ok((tcx.type_of(unevaluated.def).instantiate(tcx, unevaluated.args), c))
}
Ok(Err(bad_ty)) => Err(Either::Left(bad_ty)),
Err(err) => Err(Either::Right(err.into())),
Err(err) => Err(Either::Right(err)),
}
}
ConstKind::Value(ty, val) => Ok((ty, val)),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1526,7 +1526,7 @@ pub trait PrettyPrinter<'tcx>: Printer<'tcx> + fmt::Write {

let precedence = |binop: rustc_middle::mir::BinOp| {
use rustc_ast::util::parser::AssocOp;
AssocOp::from_ast_binop(binop.to_hir_binop().into()).precedence()
AssocOp::from_ast_binop(binop.to_hir_binop()).precedence()
};
let op_precedence = precedence(op);
let formatted_op = op.to_hir_binop().as_str();
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1586,7 +1586,7 @@ impl<'a> Parser<'a> {
(thin_vec![], Recovered::Yes(guar))
}
};
VariantData::Struct { fields, recovered: recovered.into() }
VariantData::Struct { fields, recovered }
} else if this.check(&token::OpenDelim(Delimiter::Parenthesis)) {
let body = match this.parse_tuple_struct_body() {
Ok(body) => body,
Expand Down Expand Up @@ -1670,7 +1670,7 @@ impl<'a> Parser<'a> {
class_name.span,
generics.where_clause.has_where_token,
)?;
VariantData::Struct { fields, recovered: recovered.into() }
VariantData::Struct { fields, recovered }
}
// No `where` so: `struct Foo<T>;`
} else if self.eat(&token::Semi) {
Expand All @@ -1682,7 +1682,7 @@ impl<'a> Parser<'a> {
class_name.span,
generics.where_clause.has_where_token,
)?;
VariantData::Struct { fields, recovered: recovered.into() }
VariantData::Struct { fields, recovered }
// Tuple-style struct definition with optional where-clause.
} else if self.token == token::OpenDelim(Delimiter::Parenthesis) {
let body = VariantData::Tuple(self.parse_tuple_struct_body()?, DUMMY_NODE_ID);
Expand Down Expand Up @@ -1711,14 +1711,14 @@ impl<'a> Parser<'a> {
class_name.span,
generics.where_clause.has_where_token,
)?;
VariantData::Struct { fields, recovered: recovered.into() }
VariantData::Struct { fields, recovered }
} else if self.token == token::OpenDelim(Delimiter::Brace) {
let (fields, recovered) = self.parse_record_struct_body(
"union",
class_name.span,
generics.where_clause.has_where_token,
)?;
VariantData::Struct { fields, recovered: recovered.into() }
VariantData::Struct { fields, recovered }
} else {
let token_str = super::token_descr(&self.token);
let msg = format!("expected `where` or `{{` after union name, found {token_str}");
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1045,6 +1045,7 @@ symbols! {
integer_: "integer", // underscore to avoid clashing with the function `sym::integer` below
integral,
into_async_iter_into_iter,
into_fn,
into_future,
into_iter,
intra_doc_pointers,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_symbol_mangling/src/v0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ impl<'tcx> Printer<'tcx> for SymbolMangler<'tcx> {
let consts = [
start.unwrap_or(self.tcx.consts.unit),
end.unwrap_or(self.tcx.consts.unit),
ty::Const::from_bool(self.tcx, include_end).into(),
ty::Const::from_bool(self.tcx, include_end),
];
// HACK: Represent as tuple until we have something better.
// HACK: constants are used in arrays, even if the types don't match.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ pub(super) fn opt_normalize_projection_term<'a, 'b, 'tcx>(
debug!("opt_normalize_projection_type: found error");
let result = normalize_to_error(selcx, param_env, projection_term, cause, depth);
obligations.extend(result.obligations);
return Ok(Some(result.value.into()));
return Ok(Some(result.value));
}
}

Expand Down Expand Up @@ -476,7 +476,7 @@ pub(super) fn opt_normalize_projection_term<'a, 'b, 'tcx>(
}
let result = normalize_to_error(selcx, param_env, projection_term, cause, depth);
obligations.extend(result.obligations);
Ok(Some(result.value.into()))
Ok(Some(result.value))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion library/alloc/src/rc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1714,7 +1714,7 @@ impl<T: ?Sized, A: Allocator> Rc<T, A> {
/// let x: Rc<&str> = Rc::new("Hello, world!");
/// {
/// let s = String::from("Oh, no!");
/// let mut y: Rc<&str> = x.clone().into();
/// let mut y: Rc<&str> = x.clone();
/// unsafe {
/// // this is Undefined Behavior, because x's inner type
/// // is &'long str, not &'short str
Expand Down
4 changes: 2 additions & 2 deletions library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2392,7 +2392,7 @@ impl<T: ?Sized, A: Allocator> Arc<T, A> {
/// use std::sync::Arc;
///
/// let x: Arc<str> = Arc::from("Hello, world!");
/// let mut y: Arc<[u8]> = x.clone().into();
/// let mut y: Arc<[u8]> = x.clone();
/// unsafe {
/// // this is Undefined Behavior, because x's inner type is str, not [u8]
/// Arc::get_mut_unchecked(&mut y).fill(0xff); // 0xff is invalid in UTF-8
Expand All @@ -2408,7 +2408,7 @@ impl<T: ?Sized, A: Allocator> Arc<T, A> {
/// let x: Arc<&str> = Arc::new("Hello, world!");
/// {
/// let s = String::from("Oh, no!");
/// let mut y: Arc<&str> = x.clone().into();
/// let mut y: Arc<&str> = x.clone();
/// unsafe {
/// // this is Undefined Behavior, because x's inner type
/// // is &'long str, not &'short str
Expand Down
1 change: 1 addition & 0 deletions library/core/src/convert/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ pub trait AsMut<T: ?Sized> {
#[stable(feature = "rust1", since = "1.0.0")]
pub trait Into<T>: Sized {
/// Converts this type into the (usually inferred) input type.
#[rustc_diagnostic_item = "into_fn"]
#[must_use]
#[stable(feature = "rust1", since = "1.0.0")]
fn into(self) -> T;
Expand Down
Loading

0 comments on commit dd5e794

Please sign in to comment.