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

Rollup of 9 pull requests #108466

Closed
Closed
Changes from 6 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
41ff6a8
Update LLVM submodule
icedrocket Feb 10, 2023
313f04f
Add regression test for #105626
icedrocket Feb 22, 2023
a914f37
Add lint against `Iterator::map` receiving a callable that returns `()`
obeis Feb 16, 2023
ddd7d10
Add ui test for `map_unit_fn` lint
obeis Feb 16, 2023
a87443a
Emit `map_unit_fn` lint in closure case
obeis Feb 16, 2023
b93d545
Add ui test for `map_unit_fn` lint in closure case
obeis Feb 16, 2023
99344a8
Add ui test for `E0271` error
obeis Feb 20, 2023
37d4302
Add regression test for #107918
GuillaumeGomez Feb 24, 2023
dca52ac
make "proc macro panicked" translatable
tshepang Feb 24, 2023
60ac309
add `known-bug` test for unsoundness issue #108425
gburgessiv Feb 25, 2023
9f876cc
docs/test: add UI test and docs for `E0476`
Ezrashaw Feb 25, 2023
7d83d69
add `known-bug` test for unsoundness issue #107975
gburgessiv Feb 25, 2023
6adc76d
add `known-bug` test for unsoundness issue #105787
gburgessiv Feb 25, 2023
3b51e9f
fix: fix issue in macro
Ezrashaw Feb 25, 2023
ed34354
Do not lint unresolved trait for ineffective unstable trait impl
fee1-dead Feb 25, 2023
c934ee8
Don't run issue-107918.rs test on windows
GuillaumeGomez Feb 25, 2023
885f9e7
Complete migrating `ast_passes` to derive diagnostics
clubby789 Feb 25, 2023
3560e65
Treat `str` as containing `[u8]` for auto trait purposes
compiler-errors Feb 11, 2023
53fb433
Special note for str in auto traits
compiler-errors Feb 14, 2023
c961431
Rollup merge of #107879 - icedrocket:update-llvm, r=cuviper
compiler-errors Feb 25, 2023
ec961db
Rollup merge of #107890 - obeis:mapping-to-unit, r=cjgillot
compiler-errors Feb 25, 2023
dfcaa20
Rollup merge of #107941 - compiler-errors:str-has-u8-slice-for-auto, …
compiler-errors Feb 25, 2023
cf5c521
Rollup merge of #108431 - GuillaumeGomez:regression-test-for-107918, …
compiler-errors Feb 25, 2023
8fa308d
Rollup merge of #108436 - tshepang:translatable-proc-macro-panicked, …
compiler-errors Feb 25, 2023
cb58440
Rollup merge of #108444 - Ezrashaw:add-test+docs-for-e0476, r=Guillau…
compiler-errors Feb 25, 2023
4b8a3a6
Rollup merge of #108445 - gburgessiv:add-known-bug, r=compiler-errors
compiler-errors Feb 25, 2023
fea67c2
Rollup merge of #108449 - fee1-dead-contrib:do_not_lint_unresolved, r…
compiler-errors Feb 25, 2023
e05492a
Rollup merge of #108456 - clubby789:ast-passes-diag-migrate, r=compil…
compiler-errors Feb 25, 2023
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
7 changes: 7 additions & 0 deletions compiler/rustc_lint/locales/en-US.ftl
Original file line number Diff line number Diff line change
@@ -24,6 +24,13 @@ lint_for_loops_over_fallibles =
.use_while_let = to check pattern in a loop use `while let`
.use_question_mark = consider unwrapping the `Result` with `?` to iterate over its contents

lint_map_unit_fn = `Iterator::map` call that discard the iterator's values
.note = `Iterator::map`, like many of the methods on `Iterator`, gets executed lazily, meaning that its effects won't be visible until it is iterated
.function_label = this function returns `()`, which is likely not what you wanted
.argument_label = called `Iterator::map` with callable that returns `()`
.map_label = after this call to map, the resulting iterator is `impl Iterator<Item = ()>`, which means the only information carried by the iterator is the number of items
.suggestion = you might have meant to use `Iterator::for_each`

lint_non_binding_let_on_sync_lock =
non-binding let on a synchronization lock

6 changes: 5 additions & 1 deletion compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
@@ -63,6 +63,7 @@ mod late;
mod let_underscore;
mod levels;
mod lints;
mod map_unit_fn;
mod methods;
mod multiple_supertrait_upcastable;
mod non_ascii_idents;
@@ -100,6 +101,7 @@ use for_loops_over_fallibles::*;
use hidden_unicode_codepoints::*;
use internal::*;
use let_underscore::*;
use map_unit_fn::*;
use methods::*;
use multiple_supertrait_upcastable::*;
use non_ascii_idents::*;
@@ -239,6 +241,7 @@ late_lint_methods!(
NamedAsmLabels: NamedAsmLabels,
OpaqueHiddenInferredBound: OpaqueHiddenInferredBound,
MultipleSupertraitUpcastable: MultipleSupertraitUpcastable,
MapUnitFn: MapUnitFn,
]
]
);
@@ -298,7 +301,8 @@ fn register_builtins(store: &mut LintStore) {
UNUSED_LABELS,
UNUSED_PARENS,
UNUSED_BRACES,
REDUNDANT_SEMICOLONS
REDUNDANT_SEMICOLONS,
MAP_UNIT_FN
);

add_lint_group!("let_underscore", LET_UNDERSCORE_DROP, LET_UNDERSCORE_LOCK);
16 changes: 16 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
@@ -748,6 +748,22 @@ impl AddToDiagnostic for HiddenUnicodeCodepointsDiagSub {
}
}

// map_unit_fn.rs
#[derive(LintDiagnostic)]
#[diag(lint_map_unit_fn)]
#[note]
pub struct MappingToUnit {
#[label(lint_function_label)]
pub function_label: Span,
#[label(lint_argument_label)]
pub argument_label: Span,
#[label(lint_map_label)]
pub map_label: Span,
#[suggestion(style = "verbose", code = "{replace}", applicability = "maybe-incorrect")]
pub suggestion: Span,
pub replace: String,
}

// internal.rs
#[derive(LintDiagnostic)]
#[diag(lint_default_hash_types)]
120 changes: 120 additions & 0 deletions compiler/rustc_lint/src/map_unit_fn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
use crate::lints::MappingToUnit;
use crate::{LateContext, LateLintPass, LintContext};

use rustc_hir::{Expr, ExprKind, HirId, Stmt, StmtKind};
use rustc_middle::{
query::Key,
ty::{self, Ty},
};

declare_lint! {
/// The `map_unit_fn` lint checks for `Iterator::map` receive
/// a callable that returns `()`.
///
/// ### Example
///
/// ```rust
/// fn foo(items: &mut Vec<u8>) {
/// items.sort();
/// }
///
/// fn main() {
/// let mut x: Vec<Vec<u8>> = vec![
/// vec![0, 2, 1],
/// vec![5, 4, 3],
/// ];
/// x.iter_mut().map(foo);
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Mapping to `()` is almost always a mistake.
pub MAP_UNIT_FN,
Warn,
"`Iterator::map` call that discard the iterator's values"
}

declare_lint_pass!(MapUnitFn => [MAP_UNIT_FN]);

impl<'tcx> LateLintPass<'tcx> for MapUnitFn {
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &Stmt<'_>) {
if stmt.span.from_expansion() {
return;
}

if let StmtKind::Semi(expr) = stmt.kind {
if let ExprKind::MethodCall(path, receiver, args, span) = expr.kind {
if path.ident.name.as_str() == "map" {
if receiver.span.from_expansion()
|| args.iter().any(|e| e.span.from_expansion())
|| !is_impl_slice(cx, receiver)
|| !is_diagnostic_name(cx, expr.hir_id, "IteratorMap")
{
return;
}
let arg_ty = cx.typeck_results().expr_ty(&args[0]);
if let ty::FnDef(id, _) = arg_ty.kind() {
let fn_ty = cx.tcx.fn_sig(id).skip_binder();
let ret_ty = fn_ty.output().skip_binder();
if is_unit_type(ret_ty) {
cx.emit_spanned_lint(
MAP_UNIT_FN,
span,
MappingToUnit {
function_label: cx.tcx.span_of_impl(*id).unwrap(),
argument_label: args[0].span,
map_label: arg_ty.default_span(cx.tcx),
suggestion: path.ident.span,
replace: "for_each".to_string(),
},
)
}
} else if let ty::Closure(id, subs) = arg_ty.kind() {
let cl_ty = subs.as_closure().sig();
let ret_ty = cl_ty.output().skip_binder();
if is_unit_type(ret_ty) {
cx.emit_spanned_lint(
MAP_UNIT_FN,
span,
MappingToUnit {
function_label: cx.tcx.span_of_impl(*id).unwrap(),
argument_label: args[0].span,
map_label: arg_ty.default_span(cx.tcx),
suggestion: path.ident.span,
replace: "for_each".to_string(),
},
)
}
}
}
}
}
}
}

fn is_impl_slice(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
if let Some(method_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) {
if let Some(impl_id) = cx.tcx.impl_of_method(method_id) {
return cx.tcx.type_of(impl_id).skip_binder().is_slice();
}
}
false
}

fn is_unit_type(ty: Ty<'_>) -> bool {
ty.is_unit() || ty.is_never()
}

fn is_diagnostic_name(cx: &LateContext<'_>, id: HirId, name: &str) -> bool {
if let Some(def_id) = cx.typeck_results().type_dependent_def_id(id) {
if let Some(item) = cx.tcx.get_diagnostic_name(def_id) {
if item.as_str() == name {
return true;
}
}
}
false
}
1 change: 1 addition & 0 deletions library/core/src/iter/mod.rs
Original file line number Diff line number Diff line change
@@ -278,6 +278,7 @@
//!
//! ```
//! # #![allow(unused_must_use)]
//! # #![cfg_attr(not(bootstrap), allow(map_unit_fn))]
//! let v = vec![1, 2, 3, 4, 5];
//! v.iter().map(|x| println!("{x}"));
//! ```
1 change: 1 addition & 0 deletions library/core/src/iter/traits/iterator.rs
Original file line number Diff line number Diff line change
@@ -789,6 +789,7 @@ pub trait Iterator {
/// println!("{x}");
/// }
/// ```
#[rustc_diagnostic_item = "IteratorMap"]
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_do_not_const_check]
13 changes: 13 additions & 0 deletions tests/ui/lint/issue-106991.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
fn foo(items: &mut Vec<u8>) {
items.sort();
}

fn bar() -> impl Iterator<Item = i32> {
//~^ ERROR expected `foo` to be a fn item that returns `i32`, but it returns `()` [E0271]
let mut x: Vec<Vec<u8>> = vec![vec![0, 2, 1], vec![5, 4, 3]];
x.iter_mut().map(foo)
}

fn main() {
bar();
}
11 changes: 11 additions & 0 deletions tests/ui/lint/issue-106991.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0271]: expected `foo` to be a fn item that returns `i32`, but it returns `()`
--> $DIR/issue-106991.rs:5:13
|
LL | fn bar() -> impl Iterator<Item = i32> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `i32`
|
= note: required for `Map<std::slice::IterMut<'_, Vec<u8>>, for<'a> fn(&'a mut Vec<u8>) {foo}>` to implement `Iterator`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0271`.
20 changes: 20 additions & 0 deletions tests/ui/lint/lint_map_unit_fn.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#![deny(map_unit_fn)]

fn foo(items: &mut Vec<u8>) {
items.sort();
}

fn main() {
let mut x: Vec<Vec<u8>> = vec![vec![0, 2, 1], vec![5, 4, 3]];
x.iter_mut().map(foo);
//~^ ERROR `Iterator::map` call that discard the iterator's values
x.iter_mut().map(|items| {
//~^ ERROR `Iterator::map` call that discard the iterator's values
items.sort();
});
let f = |items: &mut Vec<u8>| {
items.sort();
};
x.iter_mut().map(f);
//~^ ERROR `Iterator::map` call that discard the iterator's values
}
66 changes: 66 additions & 0 deletions tests/ui/lint/lint_map_unit_fn.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
error: `Iterator::map` call that discard the iterator's values
--> $DIR/lint_map_unit_fn.rs:9:18
|
LL | fn foo(items: &mut Vec<u8>) {
| --------------------------- this function returns `()`, which is likely not what you wanted
...
LL | x.iter_mut().map(foo);
| ^^^^---^
| | |
| | called `Iterator::map` with callable that returns `()`
| after this call to map, the resulting iterator is `impl Iterator<Item = ()>`, which means the only information carried by the iterator is the number of items
|
= note: `Iterator::map`, like many of the methods on `Iterator`, gets executed lazily, meaning that its effects won't be visible until it is iterated
note: the lint level is defined here
--> $DIR/lint_map_unit_fn.rs:1:9
|
LL | #![deny(map_unit_fn)]
| ^^^^^^^^^^^
help: you might have meant to use `Iterator::for_each`
|
LL | x.iter_mut().for_each(foo);
| ~~~~~~~~

error: `Iterator::map` call that discard the iterator's values
--> $DIR/lint_map_unit_fn.rs:11:18
|
LL | x.iter_mut().map(|items| {
| ^ -------
| | |
| ____________________|___this function returns `()`, which is likely not what you wanted
| | __________________|
| | |
LL | | |
LL | | | items.sort();
LL | | | });
| | | -^ after this call to map, the resulting iterator is `impl Iterator<Item = ()>`, which means the only information carried by the iterator is the number of items
| | |_____||
| |_______|
| called `Iterator::map` with callable that returns `()`
|
= note: `Iterator::map`, like many of the methods on `Iterator`, gets executed lazily, meaning that its effects won't be visible until it is iterated
help: you might have meant to use `Iterator::for_each`
|
LL | x.iter_mut().for_each(|items| {
| ~~~~~~~~

error: `Iterator::map` call that discard the iterator's values
--> $DIR/lint_map_unit_fn.rs:18:18
|
LL | let f = |items: &mut Vec<u8>| {
| --------------------- this function returns `()`, which is likely not what you wanted
...
LL | x.iter_mut().map(f);
| ^^^^-^
| | |
| | called `Iterator::map` with callable that returns `()`
| after this call to map, the resulting iterator is `impl Iterator<Item = ()>`, which means the only information carried by the iterator is the number of items
|
= note: `Iterator::map`, like many of the methods on `Iterator`, gets executed lazily, meaning that its effects won't be visible until it is iterated
help: you might have meant to use `Iterator::for_each`
|
LL | x.iter_mut().for_each(f);
| ~~~~~~~~

error: aborting due to 3 previous errors