Skip to content
Draft
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
69 changes: 62 additions & 7 deletions compiler/rustc_ast_lowering/src/format.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use std::borrow::Cow;

use rustc_ast::*;
use rustc_data_structures::fx::FxIndexMap;
use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
use rustc_hir as hir;
use rustc_session::config::FmtDebug;
use rustc_span::{ByteSymbol, DesugaringKind, Ident, Span, Symbol, sym};
use smallvec::SmallVec;

use super::LoweringContext;

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

let mut incomplete_lit = String::new();

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

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

let arguments = fmt.arguments.all_args();

for (i, piece) in template.iter().enumerate() {
match piece {
&FormatArgsPiece::Literal(sym) => {
Expand Down Expand Up @@ -370,6 +374,16 @@ fn expand_format_args<'hir>(
)
.0 as u64;

if matches!(p.argument.kind, FormatArgPositionKind::Named)
&& let Some(arg) = arguments.get(p.argument.index.unwrap_or(usize::MAX))
&& matches!(arg.kind, FormatArgumentKind::Captured(_))
{
let dups = might_be_const_capture_dups
.entry(p.argument.index.unwrap_or(usize::MAX))
.or_insert_with(|| (arg, SmallVec::<[_; 1]>::with_capacity(1)));
dups.1.push(p.span);
}

// This needs to match the constants in library/core/src/fmt/mod.rs.
let o = &p.format_options;
let align = match o.alignment {
Expand Down Expand Up @@ -428,8 +442,6 @@ fn expand_format_args<'hir>(
ctx.dcx().span_err(macsp, "too many format arguments");
}

let arguments = fmt.arguments.all_args();

let (let_statements, args) = if arguments.is_empty() {
// Generate:
// []
Expand All @@ -439,8 +451,10 @@ fn expand_format_args<'hir>(
// super let args = (&arg0, &arg1, &…);
let args_ident = Ident::new(sym::args, macsp);
let (args_pat, args_hir_id) = ctx.pat_ident(macsp, args_ident);
let mut arg_exprs = FxHashMap::default();
let elements = ctx.arena.alloc_from_iter(arguments.iter().map(|arg| {
let arg_expr = ctx.lower_expr(&arg.expr);
arg_exprs.insert(arg.expr.node_id(), arg_expr);
ctx.expr(
arg.expr.span.with_ctxt(macsp.ctxt()),
hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, arg_expr),
Expand Down Expand Up @@ -487,10 +501,51 @@ fn expand_format_args<'hir>(
let args = ctx.arena.alloc(ctx.expr(macsp, hir::ExprKind::Array(args)));
let (args_pat, args_hir_id) = ctx.pat_ident(macsp, args_ident);
let let_statement_2 = ctx.stmt_super_let_pat(macsp, args_pat, Some(args));
(
vec![let_statement_1, let_statement_2],
ctx.arena.alloc(ctx.expr_ident_mut(macsp, args_ident, args_hir_id)),
)

let mut let_statements = vec![let_statement_1, let_statement_2];

// Generate:
// let __issue_145739 = (&x, (), (), ..);
// as many times as the number of duplicated captured arguments.
// These will be scrutinized with late lints for crater run for #145739
if might_be_const_capture_dups.values().any(|v| v.1.len() > 1) {
for (arg, dups) in might_be_const_capture_dups.values().filter(|v| v.1.len() > 1) {
let Some(&arg_expr) = arg_exprs.get(&arg.expr.node_id()) else { continue };
// HACK: In general, this would cause weird ICEs with nested expressions but okay in
// here since the arg here is a simple path.
let arg_expr = ctx.arena.alloc(ctx.expr(arg_expr.span, arg_expr.kind.clone()));
let dups_ident = Ident::new(sym::__issue_145739, arg.expr.span);
let (dups_pat, _) = ctx.pat_ident(arg.expr.span, dups_ident);
let elements = ctx.arena.alloc_from_iter(
std::iter::once({
ctx.expr(
arg.expr.span.with_ctxt(macsp.ctxt()),
hir::ExprKind::AddrOf(
hir::BorrowKind::Ref,
hir::Mutability::Not,
arg_expr,
),
)
})
.chain(dups.iter().map(|sp| {
let sp = sp.unwrap_or(arg.expr.span).with_ctxt(macsp.ctxt());
ctx.expr(sp, hir::ExprKind::Tup(&[]))
})),
);
let dups_tuple =
ctx.arena.alloc(ctx.expr(arg.expr.span, hir::ExprKind::Tup(elements)));
let let_statement = ctx.stmt_let_pat(
None,
arg.expr.span,
Some(dups_tuple),
dups_pat,
hir::LocalSource::Normal,
);
let_statements.push(let_statement);
}
}

(let_statements, ctx.arena.alloc(ctx.expr_ident_mut(macsp, args_ident, args_hir_id)))
};

// Generate:
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,19 @@ lint_invalid_reference_casting_note_book = for more information, visit <https://

lint_invalid_reference_casting_note_ty_has_interior_mutability = even for types with interior mutability, the only legal way to obtain a mutable pointer from a shared reference is through `UnsafeCell::get`

lint_issue_145739 = `format_args!` is called with multiple parameters that capturing the same {$kind} violating issue 145739
.note1 = the type of this {$kind} is `{$ty}` and it has {$is_not_freeze ->
[true] {$needs_drop ->
[true] interior mutability and drop implementation
*[false] interior mutability
}
*[false] {$needs_drop ->
[true] drop implementation
*[false] (none)
}
}
.note2 = these are the duplicated parameters

lint_lintpass_by_hand = implementing `LintPass` by hand
.help = try using `declare_lint_pass!` or `impl_lint_pass!` instead

Expand Down
86 changes: 86 additions & 0 deletions compiler/rustc_lint/src/issue_145739.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
use rustc_errors::MultiSpan;
use rustc_hir as hir;
use rustc_middle::span_bug;
use rustc_session::{declare_lint, declare_lint_pass};
use rustc_span::symbol::sym;

use crate::lints::Issue145738Diag;
use crate::{LateContext, LateLintPass, LintContext};

declare_lint! {
/// A crater only lint that checks for multiple parameters capturing a single const/const ctor
/// inside a single `format_args!` macro expansion.
pub ISSUE_145739,
Deny,
"a violation for issue 145739 is caught",
}

declare_lint_pass!(
Issue145739 => [ISSUE_145739]
);

impl<'tcx> LateLintPass<'tcx> for Issue145739 {
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx hir::Stmt<'tcx>) {
let hir::StmtKind::Let(hir::LetStmt {
pat:
hir::Pat { kind: hir::PatKind::Binding(hir::BindingMode::NONE, _, ident, None), .. },
init: Some(hir::Expr { kind: hir::ExprKind::Tup(tup), .. }),
..
}) = stmt.kind
else {
return;
};

if ident.name != sym::__issue_145739 {
return;
}

if tup.len() < 3 {
span_bug!(stmt.span, "Duplicated tuple is too short");
}

let hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Not, arg_expr) =
tup[0].kind
else {
span_bug!(stmt.span, "Duplicated tuple first element is not a ref");
};

let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = arg_expr.kind else {
return;
};

let hir::def::Res::Def(def_kind, def_id) = path.res else {
return;
};

let tcx = cx.tcx;
let typing_env = cx.typing_env();
let (ty, kind) = match def_kind {
hir::def::DefKind::Const => (tcx.type_of(def_id).skip_binder(), "constant"),
hir::def::DefKind::Ctor(_, hir::def::CtorKind::Const) => {
(tcx.type_of(tcx.parent(def_id)).skip_binder(), "constant constructor")
}
_ => {
return;
}
};

let lint = |is_not_freeze, needs_drop| {
let diag = Issue145738Diag {
const_span: tcx.def_span(def_id),
kind,
ty,
is_not_freeze,
needs_drop,
duplicates: MultiSpan::from_spans(tup.iter().skip(1).map(|e| e.span).collect()),
};
cx.emit_span_lint(ISSUE_145739, stmt.span, diag);
};

if !ty.is_freeze(tcx, typing_env) {
lint(true, ty.needs_drop(tcx, typing_env));
} else if ty.needs_drop(tcx, typing_env) {
lint(false, true);
}
}
}
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ mod impl_trait_overcaptures;
mod interior_mutable_consts;
mod internal;
mod invalid_from_utf8;
mod issue_145739;
mod late;
mod let_underscore;
mod levels;
Expand Down Expand Up @@ -97,6 +98,7 @@ use impl_trait_overcaptures::ImplTraitOvercaptures;
use interior_mutable_consts::*;
use internal::*;
use invalid_from_utf8::*;
use issue_145739::Issue145739;
use let_underscore::*;
use lifetime_syntax::*;
use macro_expr_fragment_specifier_2024_migration::*;
Expand Down Expand Up @@ -249,6 +251,7 @@ late_lint_methods!(
FunctionCastsAsInteger: FunctionCastsAsInteger,
CheckTransmutes: CheckTransmutes,
LifetimeSyntax: LifetimeSyntax,
Issue145739: Issue145739,
]
]
);
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3248,3 +3248,17 @@ impl Subdiagnostic for MismatchedLifetimeSyntaxesSuggestion {
}
}
}

// issue_145739.rs
#[derive(LintDiagnostic)]
#[diag(lint_issue_145739)]
pub(crate) struct Issue145738Diag<'a> {
#[note(lint_note1)]
pub const_span: Span,
pub kind: &'static str,
pub ty: Ty<'a>,
pub is_not_freeze: bool,
pub needs_drop: bool,
#[note(lint_note2)]
pub duplicates: MultiSpan,
}
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ symbols! {
__S,
__T,
__awaitee,
__issue_145739,
__try_var,
_t,
_task_context,
Expand Down
40 changes: 40 additions & 0 deletions tests/ui/lint/format-args-issue-145739.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
use std::cell::Cell;
use std::fmt::{self, Display, Formatter};

#[derive(Debug)]
struct NonFreeze(Cell<usize>);

impl Display for DropImpl {
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
write!(f, "DropImpl")
}
}

#[derive(Debug)]
struct DropImpl;

impl Drop for DropImpl {
fn drop(&mut self) {}
}

const NON_FREEZE: NonFreeze = NonFreeze(Cell::new(0));
const DROP_IMPL: DropImpl = DropImpl;
const FINE: &str = "fine";

fn main() {
// These are okay
format_args!("{FINE}{FINE}");
format_args!("{DROP_IMPL}{DropImpl}");
let _ = format!("{NON_FREEZE:?}{:?}", NON_FREEZE);

// These are not
print!("{DROP_IMPL}, {DROP_IMPL}");
//~^ ERROR `format_args!` is called with multiple parameters that capturing the same constant violating issue 145739 [issue_145739]
println!("{DropImpl}{DropImpl:?}");
//~^ ERROR `format_args!` is called with multiple parameters that capturing the same constant constructor violating issue 145739 [issue_145739]
format!("{NON_FREEZE:?}{:?}{NON_FREEZE:?}", NON_FREEZE);
//~^ ERROR `format_args!` is called with multiple parameters that capturing the same constant violating issue 145739 [issue_145739]
format!("{NON_FREEZE:?}{DropImpl}{NON_FREEZE:?}{DropImpl}");
//~^ ERROR `format_args!` is called with multiple parameters that capturing the same constant violating issue 145739 [issue_145739]
//~| ERROR `format_args!` is called with multiple parameters that capturing the same constant constructor violating issue 145739 [issue_145739]
}
Loading
Loading