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

[Experimental] <T as Into<T>>::into lint #129249

Open
wants to merge 2 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
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,7 @@ fn check_type_defn<'tcx>(
} else {
// Evaluate the constant proactively, to emit an error if the constant has
// an unconditional error. We only do so if the const has no type params.
let _ = tcx.const_eval_poly(def_id.into());
let _ = tcx.const_eval_poly(def_id);
}
}
let field_id = field.did.expect_local();
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 @@ -740,6 +740,8 @@ lint_reserved_prefix = prefix `{$prefix}` is unknown
lint_reserved_string = will be parsed as a guarded string in Rust 2024
.suggestion = insert whitespace here to avoid this being parsed as a guarded string in Rust 2024

lint_self_type_conversion = this conversion is useless `{$source}` to `{$target}`

Choose a reason for hiding this comment

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

Incorrect sentence structure...

Suggested change
lint_self_type_conversion = this conversion is useless `{$source}` to `{$target}`
lint_self_type_conversion = useless conversion to the same type: `{$target}`

(taken from https://github.com/rust-lang/rust-clippy/blob/e5a1ef0795a119a68cbd4cccb41b569206d74d78/clippy_lints/src/useless_conversion.rs#L187)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The lint itself will need to be iterated on. This is all a placeholder so that I can run crater :)


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
122 changes: 118 additions & 4 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ use crate::lints::{
BuiltinTrivialBounds, BuiltinTypeAliasBounds, BuiltinUngatedAsyncFnTrackCaller,
BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub,
BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub,
BuiltinWhileTrue, InvalidAsmLabel,
BuiltinWhileTrue, InvalidAsmLabel, SelfTypeConversionDiag,
};
use crate::nonstandard_style::{MethodLateContext, method_context};
use crate::{
Expand Down Expand Up @@ -1304,9 +1304,9 @@ impl UnreachablePub {
cx.effective_visibilities.effective_vis(def_id).map(|effective_vis| {
effective_vis.at_level(rustc_middle::middle::privacy::Level::Reachable)
})
&& let parent_parent = cx.tcx.parent_module_from_def_id(
cx.tcx.parent_module_from_def_id(def_id.into()).into(),
)
&& let parent_parent = cx
.tcx
.parent_module_from_def_id(cx.tcx.parent_module_from_def_id(def_id).into())
&& *restricted_did == parent_parent.to_local_def_id()
&& !restricted_did.to_def_id().is_crate_root()
{
Expand Down Expand Up @@ -1589,6 +1589,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 @@ -3063,3 +3064,116 @@ 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:?}");
tracing::info!("ignoring {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!("found stripped {name:#?}");
}
}
}
// FIXME: also look at conditional cfgd uses so to account for things like
// `use std::io::repr_bitpacked` and `std::io::repr_unpacked`.
}

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) {
tracing::info!("ignored");
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!(?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")
|| cx
.tcx
.sess
.source_map()
.span_to_embeddable_string(expr.span)
.contains("crates/crates-io")
|| cx.tcx.sess.source_map().span_to_embeddable_string(expr.span).contains("cargo/core")
|| cx
.tcx
.sess
.source_map()
.span_to_embeddable_string(expr.span)
.contains("cargo/sources")
|| cx.tcx.sess.source_map().span_to_embeddable_string(expr.span).contains("cargo/util")
{
// 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 @@ -188,7 +188,7 @@ early_lint_methods!(
late_lint_methods!(
declare_combined_late_lint_pass,
[
BuiltinCombinedModuleLateLintPass,
BuiltinCombinedModuleLateLintPass<'tcx>,
[
ForLoopsOverFallibles: ForLoopsOverFallibles,
DefaultCouldBeDerived: DefaultCouldBeDerived::default(),
Expand All @@ -206,6 +206,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 @@ -275,6 +276,7 @@ fn register_builtins(store: &mut LintStore) {
store.register_lints(&foreign_modules::get_lints());
store.register_lints(&HardwiredLints::lint_vec());

store.register_late_pass(move |_tcx| Box::new(SelfTypeConversion::new()));
add_lint_group!(
"nonstandard_style",
NON_CAMEL_CASE_TYPES,
Expand Down Expand Up @@ -305,6 +307,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 @@ -71,6 +71,13 @@ pub(crate) struct BuiltinNonShorthandFieldPatterns {
pub prefix: &'static str,
}

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

#[derive(LintDiagnostic)]
pub(crate) 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 @@ -95,13 +95,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 @@ -115,12 +115,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
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ impl Subdiagnostic for LocalLabel<'_> {
for dtor in self.destructors {
dtor.add_to_diag_with(diag, f);
}
let msg = f(diag, crate::fluent_generated::mir_transform_label_local_epilogue.into());
let msg = f(diag, crate::fluent_generated::mir_transform_label_local_epilogue);
diag.span_label(self.span, msg);
}
}
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_parse/src/parser/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2366,8 +2366,7 @@ fn string_to_tts_1() {
token::Ident(sym::i32, IdentIsRaw::No),
sp(8, 11),
),
])
.into(),
]),
),
TokenTree::Delimited(
DelimSpan::from_pair(sp(13, 14), sp(18, 19)),
Expand All @@ -2383,8 +2382,7 @@ fn string_to_tts_1() {
),
// `Alone` because the `;` is followed by whitespace.
TokenTree::token_alone(token::Semi, sp(16, 17)),
])
.into(),
]),
),
]);

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 @@ -1119,6 +1119,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
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ pub fn is_const_evaluatable<'tcx>(
Err(
EvaluateConstErr::EvaluationFailure(e)
| EvaluateConstErr::InvalidConstParamTy(e),
) => Err(NotConstEvaluatable::Error(e.into())),
) => Err(NotConstEvaluatable::Error(e)),
Ok(_) => Ok(()),
}
}
Expand Down Expand Up @@ -140,7 +140,7 @@ pub fn is_const_evaluatable<'tcx>(
}
Err(
EvaluateConstErr::EvaluationFailure(e) | EvaluateConstErr::InvalidConstParamTy(e),
) => Err(NotConstEvaluatable::Error(e.into())),
) => Err(NotConstEvaluatable::Error(e)),
Ok(_) => Ok(()),
}
}
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 @@ -1789,7 +1789,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
2 changes: 1 addition & 1 deletion library/alloc/src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2468,7 +2468,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 @@ -446,6 +446,7 @@ pub trait AsMut<T: ?Sized> {
#[doc(search_unbox)]
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
2 changes: 1 addition & 1 deletion library/std/src/os/fd/owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

#![stable(feature = "io_safety", since = "1.63.0")]
#![deny(unsafe_op_in_unsafe_fn)]

#![cfg_attr(not(bootstrap), allow(self_type_conversion))]
use super::raw::{AsRawFd, FromRawFd, IntoRawFd, RawFd};
use crate::marker::PhantomData;
use crate::mem::ManuallyDrop;
Expand Down
1 change: 1 addition & 0 deletions library/std/src/sys/pal/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ impl Command {
// have the drop glue anyway because this code never returns (the
// child will either exec() or invoke libc::exit)
#[cfg(not(any(target_os = "tvos", target_os = "watchos")))]
#[cfg_attr(not(bootstrap), allow(self_type_conversion))]
unsafe fn do_exec(
&mut self,
stdio: ChildPipes,
Expand Down
1 change: 1 addition & 0 deletions library/std/src/sys_common/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ impl fmt::Debug for CommandEnv {

impl CommandEnv {
// Capture the current environment with these changes applied
#[cfg_attr(not(bootstrap), allow(self_type_conversion))]
pub fn capture(&self) -> BTreeMap<EnvKey, OsString> {
let mut result = BTreeMap::<EnvKey, OsString>::new();
if !self.clear {
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/thread/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ impl Builder {

let id = ThreadId::new();
let my_thread = match name {
Some(name) => Thread::new(id, name.into()),
Some(name) => Thread::new(id, name),
None => Thread::new_unnamed(id),
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ fn main() -> Result<(), ()> {
return Ok::<(), ()>(());
Err(())?;
Ok::<(), ()>(());
return Err(().into());
return Err(());
external! {
return Err(())?;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ fn main() -> Result<(), ()> {
return Ok::<(), ()>(());
Err(())?;
Ok::<(), ()>(());
return Err(().into());
return Err(());
external! {
return Err(())?;
}
Expand Down
Loading
Loading