Skip to content

Commit 2fe4066

Browse files
committed
Auto merge of #24370 - pnkfelix:fsk-unary-panic, r=<try>
Ensure a sole string-literal passed to `panic!` is not a fmt string. To accomplish this, adds `ensure_not_fmt_string_literal!` macro that will fail the compile if its expression argument is a fmt string literal. Since this is making a certain kind of use of `panic!` illegal, it is a: [breaking-change] In particular, a panic like this: ```rust panic!("Is it stringified code: { or is it a ill-formed fmt arg? }"); ``` must be rewritten; one easy rewrite is to add parentheses: ```rust panic!(("Is it stringified code: { or is it a ill-formed fmt arg? }")); ``` ---- Fix #22932.
2 parents b9ed9e2 + ac56d7f commit 2fe4066

File tree

6 files changed

+166
-12
lines changed

6 files changed

+166
-12
lines changed

src/libcore/macros.rs

+11-2
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,22 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11+
// SNAP 5520801
12+
#[cfg(stage0)]
13+
#[macro_export]
14+
macro_rules! ensure_not_fmt_string_literal {
15+
($name:expr, $e:expr) => { $e }
16+
}
17+
1118
/// Entry point of task panic, for details, see std::macros
1219
#[macro_export]
1320
macro_rules! panic {
1421
() => (
1522
panic!("explicit panic")
1623
);
1724
($msg:expr) => ({
18-
static _MSG_FILE_LINE: (&'static str, &'static str, u32) = ($msg, file!(), line!());
25+
static _MSG_FILE_LINE: (&'static str, &'static str, u32) =
26+
(ensure_not_fmt_string_literal!("panic!", $msg), file!(), line!());
1927
::core::panicking::panic(&_MSG_FILE_LINE)
2028
});
2129
($fmt:expr, $($arg:tt)*) => ({
@@ -56,7 +64,8 @@ macro_rules! panic {
5664
macro_rules! assert {
5765
($cond:expr) => (
5866
if !$cond {
59-
panic!(concat!("assertion failed: ", stringify!($cond)))
67+
const MSG: &'static str = concat!("assertion failed: ", stringify!($cond));
68+
panic!(MSG)
6069
}
6170
);
6271
($cond:expr, $($arg:tt)+) => (

src/libcoretest/nonzero.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,6 @@ fn test_match_option_string() {
9595
let five = "Five".to_string();
9696
match Some(five) {
9797
Some(s) => assert_eq!(s, "Five"),
98-
None => panic!("unexpected None while matching on Some(String { ... })")
98+
None => panic!("{}", "unexpected None while matching on Some(String { ... })")
9999
}
100100
}

src/libstd/macros.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@
1616
1717
#![unstable(feature = "std_misc")]
1818

19+
// SNAP 5520801
20+
#[cfg(stage0)]
21+
#[macro_export]
22+
macro_rules! ensure_not_fmt_string_literal {
23+
($name:expr, $e:expr) => { $e }
24+
}
25+
1926
/// The entry point for panic of Rust tasks.
2027
///
2128
/// This macro is used to inject panic into a Rust task, causing the task to
@@ -44,7 +51,7 @@ macro_rules! panic {
4451
panic!("explicit panic")
4552
});
4653
($msg:expr) => ({
47-
$crate::rt::begin_unwind($msg, {
54+
$crate::rt::begin_unwind(ensure_not_fmt_string_literal!("panic!", $msg), {
4855
// static requires less code at runtime, more constant data
4956
static _FILE_LINE: (&'static str, usize) = (file!(), line!() as usize);
5057
&_FILE_LINE

src/libsyntax/ext/base.rs

+30-8
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,9 @@ fn initial_syntax_expander_table<'feat>(ecfg: &expand::ExpansionConfig<'feat>)
458458
syntax_expanders.insert(intern("format_args"),
459459
// format_args uses `unstable` things internally.
460460
NormalTT(Box::new(ext::format::expand_format_args), None, true));
461+
syntax_expanders.insert(intern("ensure_not_fmt_string_literal"),
462+
builtin_normal_expander(
463+
ext::format::ensure_not_fmt_string_literal));
461464
syntax_expanders.insert(intern("env"),
462465
builtin_normal_expander(
463466
ext::env::expand_env));
@@ -748,19 +751,38 @@ impl<'a> ExtCtxt<'a> {
748751
}
749752
}
750753

754+
pub type ExprStringLitResult =
755+
Result<(P<ast::Expr>, InternedString, ast::StrStyle), P<ast::Expr>>;
756+
757+
/// Extract a string literal from macro expanded version of `expr`.
758+
///
759+
/// if `expr` is not string literal, then Err with span for expanded
760+
/// input, but does not emit any messages nor stop compilation.
761+
pub fn expr_string_lit(cx: &mut ExtCtxt, expr: P<ast::Expr>) -> ExprStringLitResult
762+
{
763+
// we want to be able to handle e.g. concat("foo", "bar")
764+
let expr = cx.expander().fold_expr(expr);
765+
let lit = match expr.node {
766+
ast::ExprLit(ref l) => match l.node {
767+
ast::LitStr(ref s, style) => Some(((*s).clone(), style)),
768+
_ => None
769+
},
770+
_ => None
771+
};
772+
match lit {
773+
Some(lit) => Ok((expr, lit.0, lit.1)),
774+
None => Err(expr),
775+
}
776+
}
777+
751778
/// Extract a string literal from the macro expanded version of `expr`,
752779
/// emitting `err_msg` if `expr` is not a string literal. This does not stop
753780
/// compilation on error, merely emits a non-fatal error and returns None.
754781
pub fn expr_to_string(cx: &mut ExtCtxt, expr: P<ast::Expr>, err_msg: &str)
755782
-> Option<(InternedString, ast::StrStyle)> {
756-
// we want to be able to handle e.g. concat("foo", "bar")
757-
let expr = cx.expander().fold_expr(expr);
758-
match expr.node {
759-
ast::ExprLit(ref l) => match l.node {
760-
ast::LitStr(ref s, style) => return Some(((*s).clone(), style)),
761-
_ => cx.span_err(l.span, err_msg)
762-
},
763-
_ => cx.span_err(expr.span, err_msg)
783+
match expr_string_lit(cx, expr) {
784+
Ok((_, s, style)) => return Some((s, style)),
785+
Err(e) => cx.span_err(e.span, err_msg),
764786
}
765787
None
766788
}

src/libsyntax/ext/format.rs

+65
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use codemap::{Span, respan};
1616
use ext::base::*;
1717
use ext::base;
1818
use ext::build::AstBuilder;
19+
use fold::Folder;
1920
use fmt_macros as parse;
2021
use fold::Folder;
2122
use parse::token::special_idents;
@@ -628,6 +629,70 @@ impl<'a, 'b> Context<'a, 'b> {
628629
}
629630
}
630631

632+
/// Expands `ensure_not_fmt_string_literal!(<where-literal>, <expr>)`
633+
/// into `<expr>`, but ensures that if `<expr>` is a string-literal,
634+
/// then it is not a potential format string literal.
635+
pub fn ensure_not_fmt_string_literal<'cx>(cx: &'cx mut ExtCtxt,
636+
sp: Span,
637+
tts: &[ast::TokenTree])
638+
-> Box<base::MacResult+'cx> {
639+
let takes_two_args = |cx: &ExtCtxt, rest| {
640+
cx.span_err(sp, &format!("`ensure_not_fmt_string_literal!` \
641+
takes 2 arguments, {}", rest));
642+
DummyResult::expr(sp)
643+
};
644+
let mut p = cx.new_parser_from_tts(tts);
645+
if p.token == token::Eof { return takes_two_args(cx, "given 0"); }
646+
let arg1 = cx.expander().fold_expr(p.parse_expr());
647+
if p.token != token::Comma { return takes_two_args(cx, "comma-separated"); }
648+
p.bump();
649+
if p.token == token::Eof { return takes_two_args(cx, "given 1"); }
650+
let arg2 = cx.expander().fold_expr(p.parse_expr());
651+
if p.token != token::Eof {
652+
takes_two_args(cx, "given too many");
653+
// (but do not return; handle two provided, nonetheless)
654+
}
655+
656+
// First argument is name of where this was invoked (for error messages).
657+
let lit_str_with_extended_lifetime;
658+
let name: &str = match expr_string_lit(cx, arg1) {
659+
Ok((_, lit_str, _)) => {
660+
lit_str_with_extended_lifetime = lit_str;
661+
&lit_str_with_extended_lifetime
662+
}
663+
Err(expr) => {
664+
let msg = "first argument to `ensure_not_fmt_string_literal!` \
665+
must be string literal";
666+
cx.span_err(expr.span, msg);
667+
// Compile proceeds using "ensure_not_fmt_string_literal"
668+
// as the name.
669+
"ensure_not_fmt_string_literal!"
670+
}
671+
};
672+
673+
// Second argument is the expression we are checking.
674+
let expr = match expr_string_lit(cx, arg2) {
675+
Err(expr) => {
676+
// input was not a string literal; just ignore it.
677+
expr
678+
}
679+
Ok((expr, lit_str, _style)) => {
680+
let m = format!("{} input cannot be format string literal", name);
681+
for c in lit_str.chars() {
682+
if c == '{' || c == '}' {
683+
cx.span_err(sp, &m);
684+
break;
685+
}
686+
}
687+
// we still return the expr itself, to allow catching of
688+
// further errors in the input.
689+
expr
690+
}
691+
};
692+
693+
MacEager::expr(expr)
694+
}
695+
631696
pub fn expand_format_args<'cx>(ecx: &'cx mut ExtCtxt, sp: Span,
632697
tts: &[ast::TokenTree])
633698
-> Box<base::MacResult+'cx> {

src/test/compile-fail/issue-22932.rs

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Issue 22932: `panic!("{}");` should not compile.
12+
13+
pub fn f1() { panic!("this does not work {}");
14+
//~^ ERROR panic! input cannot be format string literal
15+
}
16+
17+
pub fn workaround_1() {
18+
panic!(("This *does* works {}"));
19+
}
20+
21+
pub fn workaround_2() {
22+
const MSG: &'static str = "This *does* work {}";
23+
panic!(MSG);
24+
}
25+
26+
pub fn f2() { panic!("this does not work {");
27+
//~^ ERROR panic! input cannot be format string literal
28+
}
29+
30+
pub fn f3() { panic!("nor this }");
31+
//~^ ERROR panic! input cannot be format string literal
32+
}
33+
34+
pub fn f4() { panic!("nor this {{");
35+
//~^ ERROR panic! input cannot be format string literal
36+
}
37+
38+
pub fn f5() { panic!("nor this }}");
39+
//~^ ERROR panic! input cannot be format string literal
40+
}
41+
42+
pub fn f0_a() {
43+
ensure_not_fmt_string_literal!("`f0_a`", "this does not work {}");
44+
//~^ ERROR `f0_a` input cannot be format string literal
45+
}
46+
47+
pub fn f0_b() {
48+
println!(ensure_not_fmt_string_literal!("`f0_b`", "this does work"));
49+
}
50+
51+
pub fn main() {}

0 commit comments

Comments
 (0)