Skip to content

Commit

Permalink
Auto merge of rust-lang#88175 - camsteffen:let-desugar-span, r=Manish…
Browse files Browse the repository at this point in the history
…earth

Add expansion to while desugar spans

In the same vein as rust-lang#88163, this reverts a change in Clippy behavior as a result of rust-lang#80357 (and reverts some `#[allow]`s): This changes `clippy::blocks_in_if_conditions` to not fire on `while` loops. Though we might actually want Clippy to lint those cases, we should introduce the change purposefully, with tests, and possibly under a different lint name.

The actual change here is to add a desugaring expansion to the spans when lowering a `while` loop.

r? `@Manishearth`
  • Loading branch information
bors committed Oct 3, 2021
2 parents 9dbb26e + 67ea84d commit e737694
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 24 deletions.
11 changes: 7 additions & 4 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
ExprKind::If(ref cond, ref then, ref else_opt) => {
self.lower_expr_if(cond, then, else_opt.as_deref())
}
ExprKind::While(ref cond, ref body, opt_label) => self
.with_loop_scope(e.id, |this| {
this.lower_expr_while_in_loop_scope(e.span, cond, body, opt_label)
}),
ExprKind::While(ref cond, ref body, opt_label) => {
self.with_loop_scope(e.id, |this| {
let span =
this.mark_span_with_reason(DesugaringKind::WhileLoop, e.span, None);
this.lower_expr_while_in_loop_scope(span, cond, body, opt_label)
})
}
ExprKind::Loop(ref body, opt_label) => self.with_loop_scope(e.id, |this| {
hir::ExprKind::Loop(
this.lower_block(body, false),
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,9 +389,9 @@ pub fn struct_lint_level<'s, 'd>(
pub fn in_external_macro(sess: &Session, span: Span) -> bool {
let expn_data = span.ctxt().outer_expn_data();
match expn_data.kind {
ExpnKind::Inlined | ExpnKind::Root | ExpnKind::Desugaring(DesugaringKind::ForLoop(_)) => {
false
}
ExpnKind::Inlined
| ExpnKind::Root
| ExpnKind::Desugaring(DesugaringKind::ForLoop(_) | DesugaringKind::WhileLoop) => false,
ExpnKind::AstPass(_) | ExpnKind::Desugaring(_) => true, // well, it's "external"
ExpnKind::Macro(MacroKind::Bang, _) => {
// Dummy span for the `def_site` means it's an external macro.
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_span/src/hygiene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,7 @@ pub enum DesugaringKind {
Await,
ForLoop(ForLoopLoc),
LetElse,
WhileLoop,
}

/// A location in the desugaring of a `for` loop
Expand All @@ -1122,6 +1123,7 @@ impl DesugaringKind {
DesugaringKind::OpaqueTy => "`impl Trait`",
DesugaringKind::ForLoop(_) => "`for` loop",
DesugaringKind::LetElse => "`let...else`",
DesugaringKind::WhileLoop => "`while` loop",
}
}
}
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_typeck/src/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, ToPredicate, Ty, TypeAndMut};
use rustc_session::parse::feature_err;
use rustc_span::symbol::sym;
use rustc_span::{self, BytePos, Span};
use rustc_span::{self, BytePos, DesugaringKind, Span};
use rustc_target::spec::abi::Abi;
use rustc_trait_selection::traits::error_reporting::InferCtxtExt;
use rustc_trait_selection::traits::{self, ObligationCause, ObligationCauseCode};
Expand Down Expand Up @@ -1536,8 +1536,10 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
// If the block is from an external macro or try (`?`) desugaring, then
// do not suggest adding a semicolon, because there's nowhere to put it.
// See issues #81943 and #87051.
if cond_expr.span.desugaring_kind().is_none()
&& !in_external_macro(fcx.tcx.sess, cond_expr.span)
if matches!(
cond_expr.span.desugaring_kind(),
None | Some(DesugaringKind::WhileLoop)
) && !in_external_macro(fcx.tcx.sess, cond_expr.span)
&& !matches!(
cond_expr.kind,
hir::ExprKind::Match(.., hir::MatchSource::TryDesugar)
Expand Down
1 change: 0 additions & 1 deletion src/tools/clippy/tests/ui/crashes/issues_loop_mut_cond.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#![allow(clippy::blocks_in_if_conditions)]
#![allow(dead_code)]

/// Issue: https://github.com/rust-lang/rust-clippy/issues/2596
Expand Down
2 changes: 0 additions & 2 deletions src/tools/clippy/tests/ui/infinite_loop.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
#![allow(clippy::blocks_in_if_conditions)]

fn fn_val(i: i32) -> i32 {
unimplemented!()
}
Expand Down
22 changes: 11 additions & 11 deletions src/tools/clippy/tests/ui/infinite_loop.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: variables in the condition are not mutated in the loop body
--> $DIR/infinite_loop.rs:22:11
--> $DIR/infinite_loop.rs:20:11
|
LL | while y < 10 {
| ^^^^^^
Expand All @@ -8,71 +8,71 @@ LL | while y < 10 {
= note: this may lead to an infinite or to a never running loop

error: variables in the condition are not mutated in the loop body
--> $DIR/infinite_loop.rs:27:11
--> $DIR/infinite_loop.rs:25:11
|
LL | while y < 10 && x < 3 {
| ^^^^^^^^^^^^^^^
|
= note: this may lead to an infinite or to a never running loop

error: variables in the condition are not mutated in the loop body
--> $DIR/infinite_loop.rs:34:11
--> $DIR/infinite_loop.rs:32:11
|
LL | while !cond {
| ^^^^^
|
= note: this may lead to an infinite or to a never running loop

error: variables in the condition are not mutated in the loop body
--> $DIR/infinite_loop.rs:78:11
--> $DIR/infinite_loop.rs:76:11
|
LL | while i < 3 {
| ^^^^^
|
= note: this may lead to an infinite or to a never running loop

error: variables in the condition are not mutated in the loop body
--> $DIR/infinite_loop.rs:83:11
--> $DIR/infinite_loop.rs:81:11
|
LL | while i < 3 && j > 0 {
| ^^^^^^^^^^^^^^
|
= note: this may lead to an infinite or to a never running loop

error: variables in the condition are not mutated in the loop body
--> $DIR/infinite_loop.rs:87:11
--> $DIR/infinite_loop.rs:85:11
|
LL | while i < 3 {
| ^^^^^
|
= note: this may lead to an infinite or to a never running loop

error: variables in the condition are not mutated in the loop body
--> $DIR/infinite_loop.rs:102:11
--> $DIR/infinite_loop.rs:100:11
|
LL | while i < 3 {
| ^^^^^
|
= note: this may lead to an infinite or to a never running loop

error: variables in the condition are not mutated in the loop body
--> $DIR/infinite_loop.rs:107:11
--> $DIR/infinite_loop.rs:105:11
|
LL | while i < 3 {
| ^^^^^
|
= note: this may lead to an infinite or to a never running loop

error: variables in the condition are not mutated in the loop body
--> $DIR/infinite_loop.rs:173:15
--> $DIR/infinite_loop.rs:171:15
|
LL | while self.count < n {
| ^^^^^^^^^^^^^^
|
= note: this may lead to an infinite or to a never running loop

error: variables in the condition are not mutated in the loop body
--> $DIR/infinite_loop.rs:181:11
--> $DIR/infinite_loop.rs:179:11
|
LL | while y < 10 {
| ^^^^^^
Expand All @@ -82,7 +82,7 @@ LL | while y < 10 {
= help: rewrite it as `if cond { loop { } }`

error: variables in the condition are not mutated in the loop body
--> $DIR/infinite_loop.rs:188:11
--> $DIR/infinite_loop.rs:186:11
|
LL | while y < 10 {
| ^^^^^^
Expand Down

0 comments on commit e737694

Please sign in to comment.