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

Don't inline integer literals when they overflow - new attempt #123935

Merged
merged 4 commits into from
Apr 19, 2024
Merged
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
194 changes: 117 additions & 77 deletions compiler/rustc_ast_lowering/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,126 @@ impl<'hir> LoweringContext<'_, 'hir> {
let mut fmt = Cow::Borrowed(fmt);
if self.tcx.sess.opts.unstable_opts.flatten_format_args {
fmt = flatten_format_args(fmt);
fmt = inline_literals(fmt);
fmt = self.inline_literals(fmt);
}
expand_format_args(self, sp, &fmt, allow_const)
}

/// Try to convert a literal into an interned string
fn try_inline_lit(&self, lit: token::Lit) -> Option<Symbol> {
match LitKind::from_token_lit(lit) {
Ok(LitKind::Str(s, _)) => Some(s),
Ok(LitKind::Int(n, ty)) => {
match ty {
// unsuffixed integer literals are assumed to be i32's
LitIntType::Unsuffixed => {
(n <= i32::MAX as u128).then_some(Symbol::intern(&n.to_string()))
}
LitIntType::Signed(int_ty) => {
let max_literal = self.int_ty_max(int_ty);
(n <= max_literal).then_some(Symbol::intern(&n.to_string()))
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
}
LitIntType::Unsigned(uint_ty) => {
let max_literal = self.uint_ty_max(uint_ty);
(n <= max_literal).then_some(Symbol::intern(&n.to_string()))
}
}
}
_ => None,
}
}

/// Get the maximum value of int_ty. It is platform-dependent due to the byte size of isize
fn int_ty_max(&self, int_ty: IntTy) -> u128 {
match int_ty {
IntTy::Isize => self.tcx.data_layout.pointer_size.signed_int_max() as u128,
IntTy::I8 => i8::MAX as u128,
IntTy::I16 => i16::MAX as u128,
IntTy::I32 => i32::MAX as u128,
IntTy::I64 => i64::MAX as u128,
IntTy::I128 => i128::MAX as u128,
}
}

/// Get the maximum value of uint_ty. It is platform-dependent due to the byte size of usize
fn uint_ty_max(&self, uint_ty: UintTy) -> u128 {
match uint_ty {
UintTy::Usize => self.tcx.data_layout.pointer_size.unsigned_int_max(),
UintTy::U8 => u8::MAX as u128,
UintTy::U16 => u16::MAX as u128,
UintTy::U32 => u32::MAX as u128,
UintTy::U64 => u64::MAX as u128,
UintTy::U128 => u128::MAX as u128,
}
}

/// Inline literals into the format string.
///
/// Turns
///
/// `format_args!("Hello, {}! {} {}", "World", 123, x)`
///
/// into
///
/// `format_args!("Hello, World! 123 {}", x)`.
fn inline_literals<'fmt>(&self, mut fmt: Cow<'fmt, FormatArgs>) -> Cow<'fmt, FormatArgs> {
let mut was_inlined = vec![false; fmt.arguments.all_args().len()];
let mut inlined_anything = false;

for i in 0..fmt.template.len() {
let FormatArgsPiece::Placeholder(placeholder) = &fmt.template[i] else { continue };
let Ok(arg_index) = placeholder.argument.index else { continue };

let mut literal = None;

if let FormatTrait::Display = placeholder.format_trait
&& placeholder.format_options == Default::default()
&& let arg = fmt.arguments.all_args()[arg_index].expr.peel_parens_and_refs()
&& let ExprKind::Lit(lit) = arg.kind
{
literal = self.try_inline_lit(lit);
}

if let Some(literal) = literal {
// Now we need to mutate the outer FormatArgs.
// If this is the first time, this clones the outer FormatArgs.
let fmt = fmt.to_mut();
// Replace the placeholder with the literal.
fmt.template[i] = FormatArgsPiece::Literal(literal);
was_inlined[arg_index] = true;
inlined_anything = true;
}
}

// Remove the arguments that were inlined.
if inlined_anything {
let fmt = fmt.to_mut();

let mut remove = was_inlined;

// Don't remove anything that's still used.
for_all_argument_indexes(&mut fmt.template, |index| remove[*index] = false);

// Drop all the arguments that are marked for removal.
let mut remove_it = remove.iter();
fmt.arguments.all_args_mut().retain(|_| remove_it.next() != Some(&true));

// Calculate the mapping of old to new indexes for the remaining arguments.
let index_map: Vec<usize> = remove
.into_iter()
.scan(0, |i, remove| {
let mapped = *i;
*i += !remove as usize;
Some(mapped)
})
.collect();

// Correct the indexes that refer to arguments that have shifted position.
for_all_argument_indexes(&mut fmt.template, |index| *index = index_map[*index]);
}

fmt
}
}

/// Flattens nested `format_args!()` into one.
Expand Down Expand Up @@ -103,82 +219,6 @@ fn flatten_format_args(mut fmt: Cow<'_, FormatArgs>) -> Cow<'_, FormatArgs> {
fmt
}

/// Inline literals into the format string.
///
/// Turns
///
/// `format_args!("Hello, {}! {} {}", "World", 123, x)`
///
/// into
///
/// `format_args!("Hello, World! 123 {}", x)`.
fn inline_literals(mut fmt: Cow<'_, FormatArgs>) -> Cow<'_, FormatArgs> {
let mut was_inlined = vec![false; fmt.arguments.all_args().len()];
let mut inlined_anything = false;

for i in 0..fmt.template.len() {
let FormatArgsPiece::Placeholder(placeholder) = &fmt.template[i] else { continue };
let Ok(arg_index) = placeholder.argument.index else { continue };

let mut literal = None;

if let FormatTrait::Display = placeholder.format_trait
&& placeholder.format_options == Default::default()
&& let arg = fmt.arguments.all_args()[arg_index].expr.peel_parens_and_refs()
&& let ExprKind::Lit(lit) = arg.kind
{
if let token::LitKind::Str | token::LitKind::StrRaw(_) = lit.kind
&& let Ok(LitKind::Str(s, _)) = LitKind::from_token_lit(lit)
{
literal = Some(s);
} else if let token::LitKind::Integer = lit.kind
&& let Ok(LitKind::Int(n, _)) = LitKind::from_token_lit(lit)
{
literal = Some(Symbol::intern(&n.to_string()));
}
}

if let Some(literal) = literal {
// Now we need to mutate the outer FormatArgs.
// If this is the first time, this clones the outer FormatArgs.
let fmt = fmt.to_mut();
// Replace the placeholder with the literal.
fmt.template[i] = FormatArgsPiece::Literal(literal);
was_inlined[arg_index] = true;
inlined_anything = true;
}
}

// Remove the arguments that were inlined.
if inlined_anything {
let fmt = fmt.to_mut();

let mut remove = was_inlined;

// Don't remove anything that's still used.
for_all_argument_indexes(&mut fmt.template, |index| remove[*index] = false);

// Drop all the arguments that are marked for removal.
let mut remove_it = remove.iter();
fmt.arguments.all_args_mut().retain(|_| remove_it.next() != Some(&true));

// Calculate the mapping of old to new indexes for the remaining arguments.
let index_map: Vec<usize> = remove
.into_iter()
.scan(0, |i, remove| {
let mapped = *i;
*i += !remove as usize;
Some(mapped)
})
.collect();

// Correct the indexes that refer to arguments that have shifted position.
for_all_argument_indexes(&mut fmt.template, |index| *index = index_map[*index]);
}

fmt
}

#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
enum ArgumentType {
Format(FormatTrait),
Expand Down
14 changes: 14 additions & 0 deletions tests/ui/fmt/no-inline-literals-out-of-range.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//@ only-64bit

fn main() {
format_args!("{}", 0x8f_i8); // issue #115423
//~^ ERROR literal out of range for `i8`
format_args!("{}", 0xffff_ffff_u8); // issue #116633
//~^ ERROR literal out of range for `u8`
format_args!("{}", 0xffff_ffff_ffff_ffff_ffff_usize);
//~^ ERROR literal out of range for `usize`
format_args!("{}", 0x8000_0000_0000_0000_isize);
//~^ ERROR literal out of range for `isize`
format_args!("{}", 0xffff_ffff); // treat unsuffixed literals as i32
//~^ ERROR literal out of range for `i32`
}
56 changes: 56 additions & 0 deletions tests/ui/fmt/no-inline-literals-out-of-range.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
error: literal out of range for `i8`
--> $DIR/no-inline-literals-out-of-range.rs:4:24
|
LL | format_args!("{}", 0x8f_i8); // issue #115423
| ^^^^^^^
|
= note: the literal `0x8f_i8` (decimal `143`) does not fit into the type `i8` and will become `-113i8`
= note: `#[deny(overflowing_literals)]` on by default
help: consider using the type `u8` instead
|
LL | format_args!("{}", 0x8f_u8); // issue #115423
| ~~~~~~~
help: to use as a negative number (decimal `-113`), consider using the type `u8` for the literal and cast it to `i8`
|
LL | format_args!("{}", 0x8f_u8 as i8); // issue #115423
| ~~~~~~~~~~~~~

error: literal out of range for `u8`
--> $DIR/no-inline-literals-out-of-range.rs:6:24
|
LL | format_args!("{}", 0xffff_ffff_u8); // issue #116633
| ^^^^^^^^^^^^^^ help: consider using the type `u32` instead: `0xffff_ffff_u32`
|
= note: the literal `0xffff_ffff_u8` (decimal `4294967295`) does not fit into the type `u8` and will become `255u8`

error: literal out of range for `usize`
--> $DIR/no-inline-literals-out-of-range.rs:8:24
|
LL | format_args!("{}", 0xffff_ffff_ffff_ffff_ffff_usize);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: the literal `0xffff_ffff_ffff_ffff_ffff_usize` (decimal `1208925819614629174706175`) does not fit into the type `usize` and will become `18446744073709551615usize`

error: literal out of range for `isize`
--> $DIR/no-inline-literals-out-of-range.rs:10:24
|
LL | format_args!("{}", 0x8000_0000_0000_0000_isize);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: the literal `0x8000_0000_0000_0000_isize` (decimal `9223372036854775808`) does not fit into the type `isize` and will become `-9223372036854775808isize`

error: literal out of range for `i32`
--> $DIR/no-inline-literals-out-of-range.rs:12:24
|
LL | format_args!("{}", 0xffff_ffff); // treat unsuffixed literals as i32
| ^^^^^^^^^^^
|
= note: the literal `0xffff_ffff` (decimal `4294967295`) does not fit into the type `i32` and will become `-1i32`
= help: consider using the type `u32` instead
help: to use as a negative number (decimal `-1`), consider using the type `u32` for the literal and cast it to `i32`
|
LL | format_args!("{}", 0xffff_ffffu32 as i32); // treat unsuffixed literals as i32
| ~~~~~~~~~~~~~~~~~~~~~

error: aborting due to 5 previous errors

Loading