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

WIP: IntoIterator for Box<[T]> + method dispatch mitigation for editions < 2024 #116607

Closed
wants to merge 2 commits into from
Closed
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_ast/src/ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ impl<'a, T> IntoIterator for &'a P<[T]> {
type Item = &'a T;
type IntoIter = slice::Iter<'a, T>;
fn into_iter(self) -> Self::IntoIter {
self.ptr.into_iter()
self.ptr.iter()
}
}

Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,10 +892,10 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
"the `#[rustc_main]` attribute is used internally to specify test entry point function",
),
rustc_attr!(
rustc_skip_array_during_method_dispatch, Normal, template!(Word),
WarnFollowing, EncodeCrossCrate::No,
"the `#[rustc_skip_array_during_method_dispatch]` attribute is used to exclude a trait \
from method dispatch when the receiver is an array, for compatibility in editions < 2021."
rustc_skip_during_method_dispatch, Normal, template!(List: "array, boxed_slice, ..."), WarnFollowing,
"the `#[rustc_skip_during_method_dispatch]` attribute is used to exclude a trait \
from method dispatch when the receiver is of the following type, for compatibility in \
editions < 2021 (array) or editions < 2024 (boxed_slice)."
),
rustc_attr!(
rustc_must_implement_one_of, Normal, template!(List: "function1, function2, ..."),
Expand Down
21 changes: 19 additions & 2 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1110,8 +1110,24 @@ fn trait_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::TraitDef {

let is_marker = tcx.has_attr(def_id, sym::marker);
let rustc_coinductive = tcx.has_attr(def_id, sym::rustc_coinductive);
let skip_array_during_method_dispatch =
tcx.has_attr(def_id, sym::rustc_skip_array_during_method_dispatch);

// FIXME: We could probably do way better attribute validation here.
let mut skip_array_during_method_dispatch = false;
let mut skip_boxed_slice_during_method_dispatch = false;
for attr in tcx.get_attrs(def_id, sym::rustc_skip_during_method_dispatch) {
if let Some(lst) = attr.meta_item_list() {
for item in lst {
if let Some(ident) = item.ident() {
match ident.as_str() {
"array" => skip_array_during_method_dispatch = true,
"boxed_slice" => skip_boxed_slice_during_method_dispatch = true,
_ => (),
}
}
}
}
}

let specialization_kind = if tcx.has_attr(def_id, sym::rustc_unsafe_specialization_marker) {
ty::trait_def::TraitSpecializationKind::Marker
} else if tcx.has_attr(def_id, sym::rustc_specialization_trait) {
Expand Down Expand Up @@ -1246,6 +1262,7 @@ fn trait_def(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::TraitDef {
is_marker,
is_coinductive: rustc_coinductive || is_auto,
skip_array_during_method_dispatch,
skip_boxed_slice_during_method_dispatch,
specialization_kind,
must_implement_one_of,
implement_via_object,
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1592,6 +1592,18 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
return ProbeResult::NoMatch;
}
}

// Some trait methods are excluded for boxed slices before 2024.
// (`boxed_slice.into_iter()` wants a slice iterator for compatibility.)
if self_ty.is_box()
&& self_ty.boxed_ty().is_slice()
&& !method_name.span.at_least_rust_2024()
{
let trait_def = self.tcx.trait_def(trait_ref.def_id);
if trait_def.skip_boxed_slice_during_method_dispatch {
return ProbeResult::NoMatch;
}
}
}
let predicate = ty::Binder::dummy(trait_ref).to_predicate(self.tcx);
parent_pred = Some(predicate);
Expand Down
182 changes: 141 additions & 41 deletions compiler/rustc_lint/src/array_into_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ use crate::{
LateContext, LateLintPass, LintContext,
};
use rustc_hir as hir;
use rustc_middle::ty;
use rustc_middle::ty::adjustment::{Adjust, Adjustment};
use rustc_middle::ty::{self, Ty};
use rustc_session::lint::FutureIncompatibilityReason;
use rustc_span::edition::Edition;
use rustc_span::symbol::sym;
use rustc_span::Span;
use std::ops::ControlFlow;

declare_lint! {
/// The `array_into_iter` lint detects calling `into_iter` on arrays.
Expand Down Expand Up @@ -38,16 +39,119 @@ declare_lint! {
reference: "<https://doc.rust-lang.org/nightly/edition-guide/rust-2021/IntoIterator-for-arrays.html>",
};
}
declare_lint! {
/// The `boxed_slice_into_iter` lint detects calling `into_iter` on boxed slices.
///
/// ### Example
///
/// ```rust,edition2021
/// # #![allow(unused)]
/// vec![1, 2, 3].into_boxed_slice().into_iter().for_each(|n| { *n; });
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Since Rust 1.??, boxed slices implement `IntoIterator`. However, to avoid
/// breakage, `boxed_slice.into_iter()` in Rust 2015, 2018, and 2021 code will still
/// behave as `(&boxed_slice).into_iter()`, returning an iterator over
/// references, just like in Rust 1.?? and earlier.
/// This only applies to the method call syntax `boxed_slice.into_iter()`, not to
/// any other syntax such as `for _ in boxed_slice` or `IntoIterator::into_iter(boxed_slice)`.
pub BOXED_SLICE_INTO_ITER,
Warn,
"detects calling `into_iter` on boxed slices in Rust 2015, 2018, and 2021",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024),
};
}

#[derive(Copy, Clone, Default)]
pub struct ArrayIntoIter {
#[derive(Copy, Clone)]
pub struct CommonIntoIter<F, N> {
for_expr_span: Span,
filter: F,
namer: N,
}

#[derive(Copy, Clone)]
pub struct ArrayIntoIter(CommonIntoIter<ArrayFilter, ArrayNamer>);
impl Default for ArrayIntoIter {
fn default() -> ArrayIntoIter {
ArrayIntoIter(CommonIntoIter {
for_expr_span: Span::default(),
filter: array_filter,
namer: array_namer,
})
}
}

#[derive(Copy, Clone)]
pub struct BoxedSliceIntoIter(CommonIntoIter<BoxedSliceFilter, BoxedSliceNamer>);
impl Default for BoxedSliceIntoIter {
fn default() -> BoxedSliceIntoIter {
BoxedSliceIntoIter(CommonIntoIter {
for_expr_span: Span::default(),
filter: boxed_slice_filter,
namer: boxed_slice_namer,
})
}
}

impl_lint_pass!(ArrayIntoIter => [ARRAY_INTO_ITER]);
impl_lint_pass!(BoxedSliceIntoIter => [BOXED_SLICE_INTO_ITER]);

impl<'tcx> LateLintPass<'tcx> for ArrayIntoIter {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
type ArrayFilter = impl Copy + FnMut(Ty<'_>) -> ControlFlow<bool>;
type BoxedSliceFilter = impl Copy + FnMut(Ty<'_>) -> ControlFlow<bool>;
type ArrayNamer = impl Copy + FnMut(Ty<'_>) -> &'static str;
type BoxedSliceNamer = impl Copy + FnMut(Ty<'_>) -> &'static str;

fn array_filter(ty: Ty<'_>) -> ControlFlow<bool> {
match ty.kind() {
// If we run into a &[T; N] or &[T] first, there's nothing to warn about.
// It'll resolve to the reference version.
ty::Ref(_, inner_ty, _) if inner_ty.is_array() => ControlFlow::Break(false),
ty::Ref(_, inner_ty, _) if matches!(inner_ty.kind(), ty::Slice(..)) => {
ControlFlow::Break(false)
}
// Found an actual array type without matching a &[T; N] first.
// This is the problematic case.
ty::Array(..) => ControlFlow::Break(true),
_ => ControlFlow::Continue(()),
}
}

fn boxed_slice_filter(_ty: Ty<'_>) -> ControlFlow<bool> {
todo!()
}

fn array_namer(ty: Ty<'_>) -> &'static str {
match *ty.kind() {
ty::Ref(_, inner_ty, _) if inner_ty.is_array() => "[T; N]",
ty::Ref(_, inner_ty, _) if matches!(inner_ty.kind(), ty::Slice(..)) => "[T]",
// We know the original first argument type is an array type,
// we know that the first adjustment was an autoref coercion
// and we know that `IntoIterator` is the trait involved. The
// array cannot be coerced to something other than a reference
// to an array or to a slice.
_ => bug!("array type coerced to something other than array or slice"),
}
}

fn boxed_slice_namer(_ty: Ty<'_>) -> &'static str {
todo!()
}

impl<F, N> CommonIntoIter<F, N>
where
F: FnMut(Ty<'_>) -> ControlFlow<bool>,
N: FnMut(Ty<'_>) -> &'static str,
{
fn check_expr<'tcx>(
&mut self,
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'tcx>,
) -> Option<(Span, ArrayIntoIterDiag<'tcx>)> {
// Save the span of expressions in `for _ in expr` syntax,
// so we can give a better suggestion for those later.
if let hir::ExprKind::Match(arg, [_], hir::MatchSource::ForLoopDesugar) = &expr.kind {
Expand All @@ -65,61 +169,43 @@ impl<'tcx> LateLintPass<'tcx> for ArrayIntoIter {
// We only care about method call expressions.
if let hir::ExprKind::MethodCall(call, receiver_arg, ..) = &expr.kind {
if call.ident.name != sym::into_iter {
return;
return None;
}

// Check if the method call actually calls the libcore
// `IntoIterator::into_iter`.
let def_id = cx.typeck_results().type_dependent_def_id(expr.hir_id).unwrap();
match cx.tcx.trait_of_item(def_id) {
Some(trait_id) if cx.tcx.is_diagnostic_item(sym::IntoIterator, trait_id) => {}
_ => return,
_ => return None,
};

// As this is a method call expression, we have at least one argument.
let receiver_ty = cx.typeck_results().expr_ty(receiver_arg);
let adjustments = cx.typeck_results().expr_adjustments(receiver_arg);

let Some(Adjustment { kind: Adjust::Borrow(_), target }) = adjustments.last() else {
return;
return None;
};

let types =
std::iter::once(receiver_ty).chain(adjustments.iter().map(|adj| adj.target));

let mut found_array = false;

for ty in types {
match ty.kind() {
// If we run into a &[T; N] or &[T] first, there's nothing to warn about.
// It'll resolve to the reference version.
ty::Ref(_, inner_ty, _) if inner_ty.is_array() => return,
ty::Ref(_, inner_ty, _) if matches!(inner_ty.kind(), ty::Slice(..)) => return,
// Found an actual array type without matching a &[T; N] first.
// This is the problematic case.
ty::Array(..) => {
found_array = true;
break;
let found_it = 'outer: {
for ty in types {
match (self.filter)(ty) {
ControlFlow::Break(b) => break 'outer b,
ControlFlow::Continue(()) => (),
}
_ => {}
}
}

if !found_array {
return;
false
};
if !found_it {
return None;
}

// Emit lint diagnostic.
let target = match *target.kind() {
ty::Ref(_, inner_ty, _) if inner_ty.is_array() => "[T; N]",
ty::Ref(_, inner_ty, _) if matches!(inner_ty.kind(), ty::Slice(..)) => "[T]",
// We know the original first argument type is an array type,
// we know that the first adjustment was an autoref coercion
// and we know that `IntoIterator` is the trait involved. The
// array cannot be coerced to something other than a reference
// to an array or to a slice.
_ => bug!("array type coerced to something other than array or slice"),
};
let target = (self.namer)(*target);
let sub = if self.for_expr_span == expr.span {
Some(ArrayIntoIterDiagSub::RemoveIntoIter {
span: receiver_arg.span.shrink_to_hi().to(expr.span.shrink_to_hi()),
Expand All @@ -132,11 +218,25 @@ impl<'tcx> LateLintPass<'tcx> for ArrayIntoIter {
} else {
None
};
cx.emit_span_lint(
ARRAY_INTO_ITER,
call.ident.span,
ArrayIntoIterDiag { target, suggestion: call.ident.span, sub },
);

Some((call.ident.span, ArrayIntoIterDiag { target, suggestion: call.ident.span, sub }))
} else {
None
}
}
}

impl<'tcx> LateLintPass<'tcx> for ArrayIntoIter {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
if let Some((span, decorator)) = self.0.check_expr(cx, expr) {
cx.emit_spanned_lint(ARRAY_INTO_ITER, span, decorator);
}
}
}
impl<'tcx> LateLintPass<'tcx> for BoxedSliceIntoIter {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
if let Some((span, decorator)) = self.0.check_expr(cx, expr) {
cx.emit_spanned_lint(BOXED_SLICE_INTO_ITER, span, decorator);
}
}
}
4 changes: 4 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@
#![feature(let_chains)]
#![feature(trait_upcasting)]
#![feature(rustc_attrs)]
#![feature(type_alias_impl_trait)]
#![recursion_limit = "256"]
#![deny(rustc::untranslatable_diagnostic)]
#![deny(rustc::diagnostic_outside_of_impl)]
#![allow(internal_features)]

#[macro_use]
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_middle/src/ty/trait_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,16 @@ pub struct TraitDef {
/// also have already switched to the new trait solver.
pub is_coinductive: bool,

/// If `true`, then this trait has the `#[rustc_skip_array_during_method_dispatch]`
/// If `true`, then this trait has the `#[rustc_skip_during_method_dispatch(array)]`
/// attribute, indicating that editions before 2021 should not consider this trait
/// during method dispatch if the receiver is an array.
pub skip_array_during_method_dispatch: bool,

/// If `true`, then this trait has the `#[rustc_skip_during_method_dispatch(boxed_slice)]`
/// attribute, indicating that editions before 2021 should not consider this trait
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
/// during method dispatch if the receiver is a boxed slice.
pub skip_boxed_slice_during_method_dispatch: bool,

/// Used to determine whether the standard library is allowed to specialize
/// on this trait.
pub specialization_kind: TraitSpecializationKind,
Expand Down
Loading
Loading