diff --git a/src/libcore/macros.rs b/src/libcore/macros.rs index ece419af95172..d42b08b8c2d3b 100644 --- a/src/libcore/macros.rs +++ b/src/libcore/macros.rs @@ -8,6 +8,13 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// SNAP 5520801 +#[cfg(stage0)] +#[macro_export] +macro_rules! __unstable_rustc_ensure_not_fmt_string_literal { + ($name:expr, $e:expr) => { $e } +} + /// Entry point of task panic, for details, see std::macros #[macro_export] macro_rules! panic { @@ -15,7 +22,9 @@ macro_rules! panic { panic!("explicit panic") ); ($msg:expr) => ({ - static _MSG_FILE_LINE: (&'static str, &'static str, u32) = ($msg, file!(), line!()); + static _MSG_FILE_LINE: (&'static str, &'static str, u32) = + (__unstable_rustc_ensure_not_fmt_string_literal!("unary `panic!`", $msg), + file!(), line!()); ::core::panicking::panic(&_MSG_FILE_LINE) }); ($fmt:expr, $($arg:tt)*) => ({ @@ -56,7 +65,8 @@ macro_rules! panic { macro_rules! assert { ($cond:expr) => ( if !$cond { - panic!(concat!("assertion failed: ", stringify!($cond))) + const MSG: &'static str = concat!("assertion failed: ", stringify!($cond)); + panic!(MSG) } ); ($cond:expr, $($arg:tt)+) => ( diff --git a/src/libcoretest/nonzero.rs b/src/libcoretest/nonzero.rs index 7a367ddeec8d4..90304bbb94a9d 100644 --- a/src/libcoretest/nonzero.rs +++ b/src/libcoretest/nonzero.rs @@ -95,6 +95,6 @@ fn test_match_option_string() { let five = "Five".to_string(); match Some(five) { Some(s) => assert_eq!(s, "Five"), - None => panic!("unexpected None while matching on Some(String { ... })") + None => panic!(("unexpected None while matching on Some(String { ... })")) } } diff --git a/src/libstd/macros.rs b/src/libstd/macros.rs index f3e99a8541aaa..209a8b3d2a647 100644 --- a/src/libstd/macros.rs +++ b/src/libstd/macros.rs @@ -16,6 +16,14 @@ #![unstable(feature = "std_misc")] +// SNAP 5520801 +#[cfg(stage0)] +#[macro_export] +#[allow_internal_unstable] +macro_rules! __unstable_rustc_ensure_not_fmt_string_literal { + ($name:expr, $e:expr) => { $e } +} + /// The entry point for panic of Rust tasks. /// /// This macro is used to inject panic into a Rust task, causing the task to @@ -44,7 +52,8 @@ macro_rules! panic { panic!("explicit panic") }); ($msg:expr) => ({ - $crate::rt::begin_unwind($msg, { + $crate::rt::begin_unwind( + __unstable_rustc_ensure_not_fmt_string_literal!("unary `panic!`", $msg), { // static requires less code at runtime, more constant data static _FILE_LINE: (&'static str, usize) = (file!(), line!() as usize); &_FILE_LINE @@ -90,11 +99,12 @@ macro_rules! panic { panic!("explicit panic") }); ($msg:expr) => ({ - $crate::rt::begin_unwind($msg, { - // static requires less code at runtime, more constant data - static _FILE_LINE: (&'static str, u32) = (file!(), line!()); - &_FILE_LINE - }) + $crate::rt::begin_unwind( + __unstable_rustc_ensure_not_fmt_string_literal!("unary `panic!`", $msg), { + // static requires less code at runtime, more constant data + static _FILE_LINE: (&'static str, u32) = (file!(), line!()); + &_FILE_LINE + }) }); ($fmt:expr, $($arg:tt)+) => ({ $crate::rt::begin_unwind_fmt(format_args!($fmt, $($arg)+), { diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index 9994fad3e317b..dcffdab4b93a8 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -458,6 +458,14 @@ fn initial_syntax_expander_table<'feat>(ecfg: &expand::ExpansionConfig<'feat>) syntax_expanders.insert(intern("format_args"), // format_args uses `unstable` things internally. NormalTT(Box::new(ext::format::expand_format_args), None, true)); + + // ensure_not_fmt_string_literal is passed `unstable` things to + // use in its expansion. (Of course, that should not actually + // force us to pass `true` to allow the use of unstable things; + // i.e. this is working around a bug elsewhere.) + let tt = NormalTT(Box::new(ext::format::ensure_not_fmt_string_literal), None, true); + syntax_expanders.insert(intern("__unstable_rustc_ensure_not_fmt_string_literal"), tt); + syntax_expanders.insert(intern("env"), builtin_normal_expander( ext::env::expand_env)); @@ -748,19 +756,38 @@ impl<'a> ExtCtxt<'a> { } } +pub type ExprStringLitResult = + Result<(P, InternedString, ast::StrStyle), P>; + +/// Extract a string literal from macro expanded version of `expr`. +/// +/// if `expr` is not string literal, then Err with span for expanded +/// input, but does not emit any messages nor stop compilation. +pub fn expr_string_lit(cx: &mut ExtCtxt, expr: P) -> ExprStringLitResult +{ + // we want to be able to handle e.g. concat("foo", "bar") + let expr = cx.expander().fold_expr(expr); + let lit = match expr.node { + ast::ExprLit(ref l) => match l.node { + ast::LitStr(ref s, style) => Some(((*s).clone(), style)), + _ => None + }, + _ => None + }; + match lit { + Some(lit) => Ok((expr, lit.0, lit.1)), + None => Err(expr), + } +} + /// Extract a string literal from the macro expanded version of `expr`, /// emitting `err_msg` if `expr` is not a string literal. This does not stop /// compilation on error, merely emits a non-fatal error and returns None. pub fn expr_to_string(cx: &mut ExtCtxt, expr: P, err_msg: &str) -> Option<(InternedString, ast::StrStyle)> { - // we want to be able to handle e.g. concat("foo", "bar") - let expr = cx.expander().fold_expr(expr); - match expr.node { - ast::ExprLit(ref l) => match l.node { - ast::LitStr(ref s, style) => return Some(((*s).clone(), style)), - _ => cx.span_err(l.span, err_msg) - }, - _ => cx.span_err(expr.span, err_msg) + match expr_string_lit(cx, expr) { + Ok((_, s, style)) => return Some((s, style)), + Err(e) => cx.span_err(e.span, err_msg), } None } diff --git a/src/libsyntax/ext/format.rs b/src/libsyntax/ext/format.rs index 374f6fa50406f..9e04e4620eabe 100644 --- a/src/libsyntax/ext/format.rs +++ b/src/libsyntax/ext/format.rs @@ -632,6 +632,78 @@ impl<'a, 'b> Context<'a, 'b> { } } +/// Expands `ensure_not_fmt_string_literal!(, )` +/// into ``, but ensures that if `` is a string-literal, +/// then it is not a potential format string literal. +pub fn ensure_not_fmt_string_literal<'cx>(cx: &'cx mut ExtCtxt, + sp: Span, + tts: &[ast::TokenTree]) + -> Box { + use fold::Folder; + let takes_two_args = |cx: &ExtCtxt, rest| { + cx.span_err(sp, &format!("`ensure_not_fmt_string_literal!` \ + takes 2 arguments, {}", rest)); + DummyResult::expr(sp) + }; + let mut p = cx.new_parser_from_tts(tts); + if p.token == token::Eof { return takes_two_args(cx, "given 0"); } + let arg1 = cx.expander().fold_expr(p.parse_expr()); + if p.token == token::Eof { return takes_two_args(cx, "given 1"); } + if !panictry!(p.eat(&token::Comma)) { return takes_two_args(cx, "comma-separated"); } + if p.token == token::Eof { return takes_two_args(cx, "given 1"); } + let arg2 = cx.expander().fold_expr(p.parse_expr()); + if p.token != token::Eof { + takes_two_args(cx, "given too many"); + // (but do not return; handle two provided, nonetheless) + } + + // First argument is name of where this was invoked (for error messages). + let lit_str_with_extended_lifetime; + let name: &str = match expr_string_lit(cx, arg1) { + Ok((_, lit_str, _)) => { + lit_str_with_extended_lifetime = lit_str; + &lit_str_with_extended_lifetime + } + Err(expr) => { + let msg = "first argument to `ensure_not_fmt_string_literal!` \ + must be string literal"; + cx.span_err(expr.span, msg); + // Compile proceeds using "ensure_not_fmt_string_literal" + // as the name. + "ensure_not_fmt_string_literal!" + } + }; + + // Second argument is the expression we are checking. + let warning = |cx: &ExtCtxt, c: char| { + cx.span_warn(sp, &format!("{} literal argument contains `{}`", name, c)); + cx.span_note(sp, "Is it meant to be a `format!` string?"); + cx.span_help(sp, "You can wrap the argument in parentheses \ + to sidestep this warning"); + }; + + let expr = match expr_string_lit(cx, arg2) { + Err(expr) => { + // input was not a string literal; just ignore it. + expr + } + + Ok((expr, lit_str, _style)) => { + for c in lit_str.chars() { + if c == '{' || c == '}' { + warning(cx, c); + break; + } + } + // we still return the expr itself, to allow catching of + // further errors in the input. + expr + } + }; + + MacEager::expr(expr) +} + pub fn expand_format_args<'cx>(ecx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree]) -> Box { diff --git a/src/test/compile-fail/issue-22932.rs b/src/test/compile-fail/issue-22932.rs new file mode 100644 index 0000000000000..4d379c0801d62 --- /dev/null +++ b/src/test/compile-fail/issue-22932.rs @@ -0,0 +1,75 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(rustc_attrs)] + +// Issue 22932: `panic!("{}");` should not compile. + +pub fn f1() { panic!("this does not work {}"); + //~^ WARN unary `panic!` literal argument contains `{` + //~| NOTE Is it meant to be a `format!` string? + //~| HELP You can wrap the argument in parentheses to sidestep this warning +} + +pub fn workaround_1() { + panic!(("This *does* works {}")); +} + +pub fn workaround_2() { + const MSG: &'static str = "This *does* work {}"; + panic!(MSG); +} + +pub fn f2() { panic!("this does not work {"); + //~^ WARN unary `panic!` literal argument contains `{` + //~| NOTE Is it meant to be a `format!` string? + //~| HELP You can wrap the argument in parentheses to sidestep this warning +} + +pub fn f3() { panic!("nor this }"); + //~^ WARN unary `panic!` literal argument contains `}` + //~| NOTE Is it meant to be a `format!` string? + //~| HELP You can wrap the argument in parentheses to sidestep this warning +} + +pub fn f4() { panic!("nor this {{"); + //~^ WARN unary `panic!` literal argument contains `{` + //~| NOTE Is it meant to be a `format!` string? + //~| HELP You can wrap the argument in parentheses to sidestep this warning +} + +pub fn f5() { panic!("nor this }}"); + //~^ WARN unary `panic!` literal argument contains `}` + //~| NOTE Is it meant to be a `format!` string? + //~| HELP You can wrap the argument in parentheses to sidestep this warning +} + +pub fn f0_a() { + __unstable_rustc_ensure_not_fmt_string_literal!("`f0_a`", "this does not work {}"); + //~^ WARN `f0_a` literal argument contains `{` + //~| NOTE Is it meant to be a `format!` string? + //~| HELP You can wrap the argument in parentheses to sidestep this warning +} + +pub fn f0_b() { + __unstable_rustc_ensure_not_fmt_string_literal!("`f0_b`", "this does work"); +} + +pub fn f0_c() { + __unstable_rustc_ensure_not_fmt_string_literal!("`f0_c`", ("so does this {}")); +} + +// This test is just checking that we get all the right warnings; none +// of them are outright errors, so use the special `rustc_error` +// attribute to force a compile error. +#[rustc_error] +pub fn main() { + //~^ ERROR compilation successful +} diff --git a/src/test/run-fail/explicit-panic-msg.rs b/src/test/run-fail/explicit-panic-msg.rs index c9c04e5f2daab..ca53e022c8193 100644 --- a/src/test/run-fail/explicit-panic-msg.rs +++ b/src/test/run-fail/explicit-panic-msg.rs @@ -15,5 +15,6 @@ fn main() { let mut a = 1; if 1 == 1 { a = 2; } - panic!(format!("woooo{}", "o")); + let msg = format!("woooo{}", "o"); + panic!(msg); }