Skip to content

Commit 04bd0f3

Browse files
committed
Implement a crater lint for #145739
1 parent 42ec52b commit 04bd0f3

File tree

8 files changed

+309
-7
lines changed

8 files changed

+309
-7
lines changed

compiler/rustc_ast_lowering/src/format.rs

Lines changed: 62 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
use std::borrow::Cow;
22

33
use rustc_ast::*;
4-
use rustc_data_structures::fx::FxIndexMap;
4+
use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
55
use rustc_hir as hir;
66
use rustc_session::config::FmtDebug;
77
use rustc_span::{ByteSymbol, DesugaringKind, Ident, Span, Symbol, sym};
8+
use smallvec::SmallVec;
89

910
use super::LoweringContext;
1011

@@ -290,6 +291,7 @@ fn expand_format_args<'hir>(
290291
// We use usize::MAX for arguments that don't exist, because that can never be a valid index
291292
// into the arguments array.
292293
let mut argmap = FxIndexMap::default();
294+
let mut might_be_const_capture_dups = FxIndexMap::default();
293295

294296
let mut incomplete_lit = String::new();
295297

@@ -307,6 +309,8 @@ fn expand_format_args<'hir>(
307309

308310
// See library/core/src/fmt/mod.rs for the format string encoding format.
309311

312+
let arguments = fmt.arguments.all_args();
313+
310314
for (i, piece) in template.iter().enumerate() {
311315
match piece {
312316
&FormatArgsPiece::Literal(sym) => {
@@ -370,6 +374,16 @@ fn expand_format_args<'hir>(
370374
)
371375
.0 as u64;
372376

377+
if matches!(p.argument.kind, FormatArgPositionKind::Named)
378+
&& let Some(arg) = arguments.get(p.argument.index.unwrap_or(usize::MAX))
379+
&& matches!(arg.kind, FormatArgumentKind::Captured(_))
380+
{
381+
let dups = might_be_const_capture_dups
382+
.entry(p.argument.index.unwrap_or(usize::MAX))
383+
.or_insert_with(|| (arg, SmallVec::<[_; 1]>::with_capacity(1)));
384+
dups.1.push(p.span);
385+
}
386+
373387
// This needs to match the constants in library/core/src/fmt/mod.rs.
374388
let o = &p.format_options;
375389
let align = match o.alignment {
@@ -428,8 +442,6 @@ fn expand_format_args<'hir>(
428442
ctx.dcx().span_err(macsp, "too many format arguments");
429443
}
430444

431-
let arguments = fmt.arguments.all_args();
432-
433445
let (let_statements, args) = if arguments.is_empty() {
434446
// Generate:
435447
// []
@@ -439,8 +451,10 @@ fn expand_format_args<'hir>(
439451
// super let args = (&arg0, &arg1, &…);
440452
let args_ident = Ident::new(sym::args, macsp);
441453
let (args_pat, args_hir_id) = ctx.pat_ident(macsp, args_ident);
454+
let mut arg_exprs = FxHashMap::default();
442455
let elements = ctx.arena.alloc_from_iter(arguments.iter().map(|arg| {
443456
let arg_expr = ctx.lower_expr(&arg.expr);
457+
arg_exprs.insert(arg.expr.node_id(), arg_expr);
444458
ctx.expr(
445459
arg.expr.span.with_ctxt(macsp.ctxt()),
446460
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, arg_expr),
@@ -487,10 +501,51 @@ fn expand_format_args<'hir>(
487501
let args = ctx.arena.alloc(ctx.expr(macsp, hir::ExprKind::Array(args)));
488502
let (args_pat, args_hir_id) = ctx.pat_ident(macsp, args_ident);
489503
let let_statement_2 = ctx.stmt_super_let_pat(macsp, args_pat, Some(args));
490-
(
491-
vec![let_statement_1, let_statement_2],
492-
ctx.arena.alloc(ctx.expr_ident_mut(macsp, args_ident, args_hir_id)),
493-
)
504+
505+
let mut let_statements = vec![let_statement_1, let_statement_2];
506+
507+
// Generate:
508+
// let __issue_145739 = (&x, (), (), ..);
509+
// as many times as the number of duplicated captured arguments.
510+
// These will be scrutinized with late lints for crater run for #145739
511+
if might_be_const_capture_dups.values().any(|v| v.1.len() > 1) {
512+
for (arg, dups) in might_be_const_capture_dups.values().filter(|v| v.1.len() > 1) {
513+
let Some(&arg_expr) = arg_exprs.get(&arg.expr.node_id()) else { continue };
514+
// HACK: In general, this would cause weird ICEs with nested expressions but okay in
515+
// here since the arg here is a simple path.
516+
let arg_expr = ctx.arena.alloc(ctx.expr(arg_expr.span, arg_expr.kind.clone()));
517+
let dups_ident = Ident::new(sym::__issue_145739, arg.expr.span);
518+
let (dups_pat, _) = ctx.pat_ident(arg.expr.span, dups_ident);
519+
let elements = ctx.arena.alloc_from_iter(
520+
std::iter::once({
521+
ctx.expr(
522+
arg.expr.span.with_ctxt(macsp.ctxt()),
523+
hir::ExprKind::AddrOf(
524+
hir::BorrowKind::Ref,
525+
hir::Mutability::Not,
526+
arg_expr,
527+
),
528+
)
529+
})
530+
.chain(dups.iter().map(|sp| {
531+
let sp = sp.unwrap_or(arg.expr.span).with_ctxt(macsp.ctxt());
532+
ctx.expr(sp, hir::ExprKind::Tup(&[]))
533+
})),
534+
);
535+
let dups_tuple =
536+
ctx.arena.alloc(ctx.expr(arg.expr.span, hir::ExprKind::Tup(elements)));
537+
let let_statement = ctx.stmt_let_pat(
538+
None,
539+
arg.expr.span,
540+
Some(dups_tuple),
541+
dups_pat,
542+
hir::LocalSource::Normal,
543+
);
544+
let_statements.push(let_statement);
545+
}
546+
}
547+
548+
(let_statements, ctx.arena.alloc(ctx.expr_ident_mut(macsp, args_ident, args_hir_id)))
494549
};
495550

496551
// Generate:

compiler/rustc_lint/messages.ftl

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -985,3 +985,16 @@ lint_uses_power_alignment = repr(C) does not follow the power alignment rule. Th
985985
986986
lint_variant_size_differences =
987987
enum variant is more than three times larger ({$largest} bytes) than the next largest
988+
989+
lint_issue_145739 = `format_args!` is called with multiple parameters that capturing the same {$kind} violating issue 145739
990+
.note1 = the type of this {$kind} is `{$ty}` and it has {$is_not_freeze ->
991+
[true] {$needs_drop ->
992+
[true] interior mutability and drop implementation
993+
*[false] interior mutability
994+
}
995+
*[false] {$needs_drop ->
996+
[true] drop implementation
997+
*[false] (none)
998+
}
999+
}
1000+
.note2 = these are the duplicated parameters
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
use rustc_errors::MultiSpan;
2+
use rustc_hir as hir;
3+
use rustc_middle::span_bug;
4+
use rustc_session::{declare_lint, declare_lint_pass};
5+
use rustc_span::symbol::sym;
6+
7+
use crate::{LateContext, LateLintPass, LintContext, lints::Issue145738Diag};
8+
9+
declare_lint! {
10+
pub ISSUE_145739,
11+
Deny,
12+
"a violation for issue 145739 is caught",
13+
}
14+
15+
declare_lint_pass!(
16+
Issue145739 => [ISSUE_145739]
17+
);
18+
19+
impl<'tcx> LateLintPass<'tcx> for Issue145739 {
20+
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx hir::Stmt<'tcx>) {
21+
let hir::StmtKind::Let(hir::LetStmt {
22+
pat:
23+
hir::Pat { kind: hir::PatKind::Binding(hir::BindingMode::NONE, _, ident, None), .. },
24+
init: Some(hir::Expr { kind: hir::ExprKind::Tup(tup), .. }),
25+
..
26+
}) = stmt.kind
27+
else {
28+
return;
29+
};
30+
31+
if ident.name != sym::__issue_145739 {
32+
return;
33+
}
34+
35+
if tup.len() < 3 {
36+
span_bug!(stmt.span, "Duplicated tuple is too short");
37+
}
38+
39+
let hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, arg_expr) =
40+
tup[0].kind
41+
else {
42+
span_bug!(stmt.span, "Duplicated tuple first element is not a ref");
43+
};
44+
45+
let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = arg_expr.kind else {
46+
return;
47+
};
48+
49+
let hir::def::Res::Def(def_kind, def_id) = path.res else {
50+
return;
51+
};
52+
53+
let tcx = cx.tcx;
54+
let typing_env = cx.typing_env();
55+
let (ty, kind) = match def_kind {
56+
hir::def::DefKind::Const => (tcx.type_of(def_id).skip_binder(), "constant"),
57+
hir::def::DefKind::Ctor(_, hir::def::CtorKind::Const) => {
58+
(tcx.type_of(tcx.parent(def_id)).skip_binder(), "constant constructor")
59+
}
60+
_ => {
61+
return;
62+
}
63+
};
64+
65+
let lint = |is_not_freeze, needs_drop| {
66+
let diag = Issue145738Diag {
67+
const_span: tcx.def_span(def_id),
68+
kind,
69+
ty,
70+
is_not_freeze,
71+
needs_drop,
72+
duplicates: MultiSpan::from_spans(tup.iter().skip(1).map(|e| e.span).collect()),
73+
};
74+
cx.emit_span_lint(ISSUE_145739, stmt.span, diag);
75+
};
76+
77+
if !ty.is_freeze(tcx, typing_env) {
78+
lint(true, ty.needs_drop(tcx, typing_env));
79+
} else if ty.needs_drop(tcx, typing_env) {
80+
lint(false, true);
81+
}
82+
}
83+
}

compiler/rustc_lint/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ mod impl_trait_overcaptures;
5151
mod interior_mutable_consts;
5252
mod internal;
5353
mod invalid_from_utf8;
54+
mod issue_145739;
5455
mod late;
5556
mod let_underscore;
5657
mod levels;
@@ -97,6 +98,7 @@ use impl_trait_overcaptures::ImplTraitOvercaptures;
9798
use interior_mutable_consts::*;
9899
use internal::*;
99100
use invalid_from_utf8::*;
101+
use issue_145739::Issue145739;
100102
use let_underscore::*;
101103
use lifetime_syntax::*;
102104
use macro_expr_fragment_specifier_2024_migration::*;
@@ -249,6 +251,7 @@ late_lint_methods!(
249251
FunctionCastsAsInteger: FunctionCastsAsInteger,
250252
CheckTransmutes: CheckTransmutes,
251253
LifetimeSyntax: LifetimeSyntax,
254+
Issue145739: Issue145739,
252255
]
253256
]
254257
);

compiler/rustc_lint/src/lints.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3248,3 +3248,17 @@ impl Subdiagnostic for MismatchedLifetimeSyntaxesSuggestion {
32483248
}
32493249
}
32503250
}
3251+
3252+
// issue_145739.rs
3253+
#[derive(LintDiagnostic)]
3254+
#[diag(lint_issue_145739)]
3255+
pub(crate) struct Issue145738Diag<'a> {
3256+
#[note(lint_note1)]
3257+
pub const_span: Span,
3258+
pub kind: &'static str,
3259+
pub ty: Ty<'a>,
3260+
pub is_not_freeze: bool,
3261+
pub needs_drop: bool,
3262+
#[note(lint_note2)]
3263+
pub duplicates: MultiSpan,
3264+
}

compiler/rustc_span/src/symbol.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,7 @@ symbols! {
398398
__S,
399399
__T,
400400
__awaitee,
401+
__issue_145739,
401402
__try_var,
402403
_t,
403404
_task_context,
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
use std::cell::Cell;
2+
use std::fmt::{self, Display, Formatter};
3+
4+
#[derive(Debug)]
5+
struct NonFreeze(Cell<usize>);
6+
7+
impl Display for DropImpl {
8+
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
9+
write!(f, "DropImpl")
10+
}
11+
}
12+
13+
#[derive(Debug)]
14+
struct DropImpl;
15+
16+
impl Drop for DropImpl {
17+
fn drop(&mut self) {}
18+
}
19+
20+
const NON_FREEZE: NonFreeze = NonFreeze(Cell::new(0));
21+
const DROP_IMPL: DropImpl = DropImpl;
22+
const FINE: &str = "fine";
23+
24+
fn main() {
25+
// These are okay
26+
format_args!("{FINE}{FINE}");
27+
format_args!("{DROP_IMPL}{DropImpl}");
28+
let _ = format!("{NON_FREEZE:?}{:?}", NON_FREEZE);
29+
30+
// These are not
31+
print!("{DROP_IMPL}, {DROP_IMPL}");
32+
//~^ ERROR `format_args!` is called with multiple parameters that capturing the same constant violating issue 145739 [issue_145739]
33+
println!("{DropImpl}{DropImpl:?}");
34+
//~^ ERROR `format_args!` is called with multiple parameters that capturing the same constant constructor violating issue 145739 [issue_145739]
35+
format!("{NON_FREEZE:?}{:?}{NON_FREEZE:?}", NON_FREEZE);
36+
//~^ ERROR `format_args!` is called with multiple parameters that capturing the same constant violating issue 145739 [issue_145739]
37+
format!("{NON_FREEZE:?}{DropImpl}{NON_FREEZE:?}{DropImpl}");
38+
//~^ ERROR `format_args!` is called with multiple parameters that capturing the same constant violating issue 145739 [issue_145739]
39+
//~| ERROR `format_args!` is called with multiple parameters that capturing the same constant constructor violating issue 145739 [issue_145739]
40+
}

0 commit comments

Comments
 (0)