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 7 pull requests #101092

Closed
wants to merge 37 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
e540425
Add a `File::create_new` constructor
joshtriplett Jul 2, 2022
c04568b
Stabilize `std::io::read_to_string`
camelid Aug 9, 2022
7b4cd17
Start uplifting `clippy::for_loops_over_fallibles`
WaffleLapkin Jul 17, 2022
810cf60
Use structured suggestions for `for_loop_over_fallibles` lint
WaffleLapkin Jul 18, 2022
b661157
`for_loop_over_fallibles`: Suggest removing `.next()`
WaffleLapkin Jul 24, 2022
7cf94ad
`for_loop_over_fallibles`: suggest `while let` loop
WaffleLapkin Jul 24, 2022
14b8f24
`for_loop_over_fallibles`: suggest using `?` in some cases
WaffleLapkin Jul 24, 2022
2bf213b
`for_loop_over_fallibles`: remove duplication from the message
WaffleLapkin Jul 24, 2022
5128140
Add a test for the `for_loop_over_fallibles` lint
WaffleLapkin Jul 24, 2022
34815a9
`for_loop_over_fallibles`: fix suggestion for "remove `.next()`" case
WaffleLapkin Jul 24, 2022
c4ab59e
`for_loop_over_fallibles`: don't use `MachineApplicable`
WaffleLapkin Jul 24, 2022
41fccb1
allow or avoid for loops over option in compiler and tests
WaffleLapkin Jul 26, 2022
86360f4
allow `for_loop_over_fallibles` in a `core` test
WaffleLapkin Jul 26, 2022
d7b8a65
Edit documentation for `for_loop_over_fallibles` lint
WaffleLapkin Aug 14, 2022
aed1ae4
remove an infinite loop
WaffleLapkin Aug 15, 2022
71b8c89
fix `for_loop_over_fallibles` lint docs
WaffleLapkin Aug 18, 2022
36c42fa
Use `DisplayBuffer` for socket addresses.
reitermarkus Aug 16, 2022
63700a8
Add tests for `SockAddr` `Display`.
reitermarkus Aug 19, 2022
89c74e8
Move `net::parser` into `net::addr` module.
reitermarkus Aug 19, 2022
d61ecec
Flatten `net` module again.
reitermarkus Aug 24, 2022
4e97626
Call them constants instead of types
compiler-errors Aug 12, 2022
4ff5872
Note binding obligation causes for const equate errors
compiler-errors Aug 12, 2022
d464d3a
Add test for #100414
compiler-errors Aug 12, 2022
8189a45
Use ExprItemObligation and ExprBindingObligation too
compiler-errors Aug 23, 2022
252c65e
Fix clippy tests that trigger `for_loop_over_fallibles` lint
WaffleLapkin Aug 25, 2022
c3f568b
Do not report too many expr field candidates
compiler-errors Aug 23, 2022
3e567bc
Make invalid-value trigger on uninit primitives
5225225 Jul 4, 2022
57ddb2d
Creating uninitialized integers is UB
5225225 Jul 5, 2022
73a30f8
More tests for invalid_value lint
5225225 Jul 5, 2022
5e8f95b
Re-add some justification
5225225 Jul 5, 2022
db9a7fc
Rollup merge of #98801 - joshtriplett:file-create-new, r=thomcc
Dylan-DPC Aug 27, 2022
1c22c94
Rollup merge of #98919 - 5225225:stricter-invalid-value, r=RalfJung
Dylan-DPC Aug 27, 2022
dad93c1
Rollup merge of #99696 - WaffleLapkin:uplift, r=fee1-dead
Dylan-DPC Aug 27, 2022
b98848b
Rollup merge of #100337 - camelid:stabilize-io_read_to_string, r=John…
Dylan-DPC Aug 27, 2022
50bc88e
Rollup merge of #100437 - compiler-errors:better-const-mismatch-err, …
Dylan-DPC Aug 27, 2022
6185c88
Rollup merge of #100640 - reitermarkus:socket-display-buffer, r=thomcc
Dylan-DPC Aug 27, 2022
11905c6
Rollup merge of #100898 - compiler-errors:too-many-expr-fields, r=spa…
Dylan-DPC Aug 27, 2022
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
14 changes: 6 additions & 8 deletions compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,14 +244,12 @@ pub trait Visitor<'ast>: Sized {

#[macro_export]
macro_rules! walk_list {
($visitor: expr, $method: ident, $list: expr) => {
for elem in $list {
$visitor.$method(elem)
}
};
($visitor: expr, $method: ident, $list: expr, $($extra_args: expr),*) => {
for elem in $list {
$visitor.$method(elem, $($extra_args,)*)
($visitor: expr, $method: ident, $list: expr $(, $($extra_args: expr),* )?) => {
{
#[cfg_attr(not(bootstrap), allow(for_loop_over_fallibles))]
for elem in $list {
$visitor.$method(elem $(, $($extra_args,)* )?)
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/type_check/input_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {

debug!("{:?} normalized to {:?}", t, norm_ty);

for data in constraints {
if let Some(data) = constraints {
ConstraintConversion::new(
self.infcx,
&self.borrowck_context.universal_regions,
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1588,9 +1588,14 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
Mismatch::Variable(infer::ExpectedFound { expected, found }),
)
}
ValuePairs::Terms(infer::ExpectedFound {
expected: ty::Term::Const(_),
found: ty::Term::Const(_),
}) => (false, Mismatch::Fixed("constant")),
ValuePairs::TraitRefs(_) | ValuePairs::PolyTraitRefs(_) => {
(false, Mismatch::Fixed("trait"))
}
ValuePairs::Regions(_) => (false, Mismatch::Fixed("lifetime")),
_ => (false, Mismatch::Fixed("type")),
};
let vals = match self.values_str(values) {
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2462,6 +2462,15 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
Char if init == InitKind::Uninit => {
Some(("characters must be a valid Unicode codepoint".to_string(), None))
}
Int(_) | Uint(_) if init == InitKind::Uninit => {
Some(("integers must not be uninitialized".to_string(), None))
}
Float(_) if init == InitKind::Uninit => {
Some(("floats must not be uninitialized".to_string(), None))
}
RawPtr(_) if init == InitKind::Uninit => {
Some(("raw pointers must not be uninitialized".to_string(), None))
}
// Recurse and checks for some compound types.
Adt(adt_def, substs) if !adt_def.is_union() => {
// First check if this ADT has a layout attribute (like `NonNull` and friends).
Expand Down
188 changes: 188 additions & 0 deletions compiler/rustc_lint/src/for_loop_over_fallibles.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
use crate::{LateContext, LateLintPass, LintContext};

use hir::{Expr, Pat};
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_infer::traits::TraitEngine;
use rustc_infer::{infer::TyCtxtInferExt, traits::ObligationCause};
use rustc_middle::ty::{self, List};
use rustc_span::{sym, Span};
use rustc_trait_selection::traits::TraitEngineExt;

declare_lint! {
/// The `for_loop_over_fallibles` lint checks for `for` loops over `Option` or `Result` values.
///
/// ### Example
///
/// ```rust
/// let opt = Some(1);
/// for x in opt { /* ... */}
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Both `Option` and `Result` implement `IntoIterator` trait, which allows using them in a `for` loop.
/// `for` loop over `Option` or `Result` will iterate either 0 (if the value is `None`/`Err(_)`)
/// or 1 time (if the value is `Some(_)`/`Ok(_)`). This is not very useful and is more clearly expressed
/// via `if let`.
///
/// `for` loop can also be accidentally written with the intention to call a function multiple times,
/// while the function returns `Some(_)`, in these cases `while let` loop should be used instead.
///
/// The "intended" use of `IntoIterator` implementations for `Option` and `Result` is passing them to
/// generic code that expects something implementing `IntoIterator`. For example using `.chain(option)`
/// to optionally add a value to an iterator.
pub FOR_LOOP_OVER_FALLIBLES,
Warn,
"for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`"
}

declare_lint_pass!(ForLoopOverFallibles => [FOR_LOOP_OVER_FALLIBLES]);

impl<'tcx> LateLintPass<'tcx> for ForLoopOverFallibles {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
let Some((pat, arg)) = extract_for_loop(expr) else { return };

let ty = cx.typeck_results().expr_ty(arg);

let &ty::Adt(adt, substs) = ty.kind() else { return };

let (article, ty, var) = match adt.did() {
did if cx.tcx.is_diagnostic_item(sym::Option, did) => ("an", "Option", "Some"),
did if cx.tcx.is_diagnostic_item(sym::Result, did) => ("a", "Result", "Ok"),
_ => return,
};

let msg = format!(
"for loop over {article} `{ty}`. This is more readably written as an `if let` statement",
);

cx.struct_span_lint(FOR_LOOP_OVER_FALLIBLES, arg.span, |diag| {
let mut warn = diag.build(msg);

if let Some(recv) = extract_iterator_next_call(cx, arg)
&& let Ok(recv_snip) = cx.sess().source_map().span_to_snippet(recv.span)
{
warn.span_suggestion(
recv.span.between(arg.span.shrink_to_hi()),
format!("to iterate over `{recv_snip}` remove the call to `next`"),
".by_ref()",
Applicability::MaybeIncorrect
);
} else {
warn.multipart_suggestion_verbose(
format!("to check pattern in a loop use `while let`"),
vec![
// NB can't use `until` here because `expr.span` and `pat.span` have different syntax contexts
(expr.span.with_hi(pat.span.lo()), format!("while let {var}(")),
(pat.span.between(arg.span), format!(") = ")),
],
Applicability::MaybeIncorrect
);
}

if suggest_question_mark(cx, adt, substs, expr.span) {
warn.span_suggestion(
arg.span.shrink_to_hi(),
"consider unwrapping the `Result` with `?` to iterate over its contents",
"?",
Applicability::MaybeIncorrect,
);
}

warn.multipart_suggestion_verbose(
"consider using `if let` to clear intent",
vec![
// NB can't use `until` here because `expr.span` and `pat.span` have different syntax contexts
(expr.span.with_hi(pat.span.lo()), format!("if let {var}(")),
(pat.span.between(arg.span), format!(") = ")),
],
Applicability::MaybeIncorrect,
);

warn.emit()
})
}
}

fn extract_for_loop<'tcx>(expr: &Expr<'tcx>) -> Option<(&'tcx Pat<'tcx>, &'tcx Expr<'tcx>)> {
if let hir::ExprKind::DropTemps(e) = expr.kind
&& let hir::ExprKind::Match(iterexpr, [arm], hir::MatchSource::ForLoopDesugar) = e.kind
&& let hir::ExprKind::Call(_, [arg]) = iterexpr.kind
&& let hir::ExprKind::Loop(block, ..) = arm.body.kind
&& let [stmt] = block.stmts
&& let hir::StmtKind::Expr(e) = stmt.kind
&& let hir::ExprKind::Match(_, [_, some_arm], _) = e.kind
&& let hir::PatKind::Struct(_, [field], _) = some_arm.pat.kind
{
Some((field.pat, arg))
} else {
None
}
}

fn extract_iterator_next_call<'tcx>(
cx: &LateContext<'_>,
expr: &Expr<'tcx>,
) -> Option<&'tcx Expr<'tcx>> {
// This won't work for `Iterator::next(iter)`, is this an issue?
if let hir::ExprKind::MethodCall(_, [recv], _) = expr.kind
&& cx.typeck_results().type_dependent_def_id(expr.hir_id) == cx.tcx.lang_items().next_fn()
{
Some(recv)
} else {
return None
}
}

fn suggest_question_mark<'tcx>(
cx: &LateContext<'tcx>,
adt: ty::AdtDef<'tcx>,
substs: &List<ty::GenericArg<'tcx>>,
span: Span,
) -> bool {
let Some(body_id) = cx.enclosing_body else { return false };
let Some(into_iterator_did) = cx.tcx.get_diagnostic_item(sym::IntoIterator) else { return false };

if !cx.tcx.is_diagnostic_item(sym::Result, adt.did()) {
return false;
}

// Check that the function/closure/constant we are in has a `Result` type.
// Otherwise suggesting using `?` may not be a good idea.
{
let ty = cx.typeck_results().expr_ty(&cx.tcx.hir().body(body_id).value);
let ty::Adt(ret_adt, ..) = ty.kind() else { return false };
if !cx.tcx.is_diagnostic_item(sym::Result, ret_adt.did()) {
return false;
}
}

let ty = substs.type_at(0);
let is_iterator = cx.tcx.infer_ctxt().enter(|infcx| {
let mut fulfill_cx = <dyn TraitEngine<'_>>::new(infcx.tcx);

let cause = ObligationCause::new(
span,
body_id.hir_id,
rustc_infer::traits::ObligationCauseCode::MiscObligation,
);
fulfill_cx.register_bound(
&infcx,
ty::ParamEnv::empty(),
// Erase any region vids from the type, which may not be resolved
infcx.tcx.erase_regions(ty),
into_iterator_did,
cause,
);

// Select all, including ambiguous predicates
let errors = fulfill_cx.select_all_or_error(&infcx);

errors.is_empty()
});

is_iterator
}
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ mod early;
mod enum_intrinsics_non_enums;
mod errors;
mod expect;
mod for_loop_over_fallibles;
pub mod hidden_unicode_codepoints;
mod internal;
mod late;
Expand Down Expand Up @@ -81,6 +82,7 @@ use rustc_span::Span;
use array_into_iter::ArrayIntoIter;
use builtin::*;
use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
use for_loop_over_fallibles::*;
use hidden_unicode_codepoints::*;
use internal::*;
use methods::*;
Expand Down Expand Up @@ -179,6 +181,7 @@ macro_rules! late_lint_mod_passes {
$macro!(
$args,
[
ForLoopOverFallibles: ForLoopOverFallibles,
HardwiredLints: HardwiredLints,
ImproperCTypesDeclarations: ImproperCTypesDeclarations,
ImproperCTypesDefinitions: ImproperCTypesDefinitions,
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,13 @@ impl<'tcx> ObligationCauseCode<'tcx> {
_ => None,
}
}

pub fn peel_match_impls(&self) -> &Self {
match self {
MatchImpl(cause, _) => cause.code(),
_ => self,
}
}
}

// `ObligationCauseCode` is used a lot. Make sure it doesn't unintentionally get bigger.
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 @@ -157,6 +157,7 @@ symbols! {
BTreeSet,
BinaryHeap,
Borrow,
BorrowMut,
Break,
C,
CStr,
Expand Down
21 changes: 18 additions & 3 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1506,13 +1506,28 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
.emit();
}
FulfillmentErrorCode::CodeConstEquateError(ref expected_found, ref err) => {
self.report_mismatched_consts(
let mut diag = self.report_mismatched_consts(
&error.obligation.cause,
expected_found.expected,
expected_found.found,
err.clone(),
)
.emit();
);
let code = error.obligation.cause.code().peel_derives().peel_match_impls();
if let ObligationCauseCode::BindingObligation(..)
| ObligationCauseCode::ItemObligation(..)
| ObligationCauseCode::ExprBindingObligation(..)
| ObligationCauseCode::ExprItemObligation(..) = code
{
self.note_obligation_cause_code(
&mut diag,
&error.obligation.predicate,
error.obligation.param_env,
code,
&mut vec![],
&mut Default::default(),
);
}
diag.emit();
}
}
}
Expand Down
51 changes: 29 additions & 22 deletions compiler/rustc_typeck/src/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2605,32 +2605,39 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if let Some((fields, substs)) =
self.get_field_candidates_considering_privacy(span, expr_t, mod_id)
{
for candidate_field in fields {
if let Some(mut field_path) = self.check_for_nested_field_satisfying(
span,
&|candidate_field, _| candidate_field.ident(self.tcx()) == field,
candidate_field,
substs,
vec![],
mod_id,
) {
// field_path includes `field` that we're looking for, so pop it.
let candidate_fields: Vec<_> = fields
.filter_map(|candidate_field| {
self.check_for_nested_field_satisfying(
span,
&|candidate_field, _| candidate_field.ident(self.tcx()) == field,
candidate_field,
substs,
vec![],
mod_id,
)
})
.map(|mut field_path| {
field_path.pop();

let field_path_str = field_path
field_path
.iter()
.map(|id| id.name.to_ident_string())
.collect::<Vec<String>>()
.join(".");
debug!("field_path_str: {:?}", field_path_str);

err.span_suggestion_verbose(
field.span.shrink_to_lo(),
"one of the expressions' fields has a field of the same name",
format!("{field_path_str}."),
Applicability::MaybeIncorrect,
);
}
.join(".")
})
.collect::<Vec<_>>();

let len = candidate_fields.len();
if len > 0 {
err.span_suggestions(
field.span.shrink_to_lo(),
format!(
"{} of the expressions' fields {} a field of the same name",
if len > 1 { "some" } else { "one" },
if len > 1 { "have" } else { "has" },
),
candidate_fields.iter().map(|path| format!("{path}.")),
Applicability::MaybeIncorrect,
);
}
}
err
Expand Down
Loading