From 0bc20e18c6c9b246d0ea940bb5aced76ca69b779 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 13 Apr 2015 09:08:05 +0200 Subject: [PATCH 01/11] Prepare for change to unary `panic!` rejecting fmt string literals. --- src/libcore/macros.rs | 3 ++- src/libcoretest/nonzero.rs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libcore/macros.rs b/src/libcore/macros.rs index ece419af95172..35fb35a9a00c9 100644 --- a/src/libcore/macros.rs +++ b/src/libcore/macros.rs @@ -56,7 +56,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 { ... })")) } } From a4b02caad3ecd765985a4f64c175dda45fc831d0 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 13 Apr 2015 09:07:23 +0200 Subject: [PATCH 02/11] Ensure a sole string-literal passes to `panic!` is not a fmt string. To accomplish this, adds `ensure_not_fmt_string_literal!` macro that will fail the compile if its argument is a fmt string literal. `ensure_not_fmt_string_literal!` takes a name along with expr; this allows for better error messages at its usage sites (like `panic!`). ---- 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. --- src/libcore/macros.rs | 10 +++++- src/libstd/macros.rs | 9 ++++- src/libsyntax/ext/base.rs | 38 +++++++++++++++++----- src/libsyntax/ext/format.rs | 65 +++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 10 deletions(-) diff --git a/src/libcore/macros.rs b/src/libcore/macros.rs index 35fb35a9a00c9..fbf27b1363290 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! 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,8 @@ 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) = + (ensure_not_fmt_string_literal!("panic!", $msg), file!(), line!()); ::core::panicking::panic(&_MSG_FILE_LINE) }); ($fmt:expr, $($arg:tt)*) => ({ diff --git a/src/libstd/macros.rs b/src/libstd/macros.rs index f3e99a8541aaa..adb6bbcdd8b34 100644 --- a/src/libstd/macros.rs +++ b/src/libstd/macros.rs @@ -16,6 +16,13 @@ #![unstable(feature = "std_misc")] +// SNAP 5520801 +#[cfg(stage0)] +#[macro_export] +macro_rules! 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 +51,7 @@ macro_rules! panic { panic!("explicit panic") }); ($msg:expr) => ({ - $crate::rt::begin_unwind($msg, { + $crate::rt::begin_unwind(ensure_not_fmt_string_literal!("panic!", $msg), { // static requires less code at runtime, more constant data static _FILE_LINE: (&'static str, usize) = (file!(), line!() as usize); &_FILE_LINE diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index 9994fad3e317b..f1f5894cdb7dd 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -458,6 +458,9 @@ 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)); + syntax_expanders.insert(intern("ensure_not_fmt_string_literal"), + builtin_normal_expander( + ext::format::ensure_not_fmt_string_literal)); syntax_expanders.insert(intern("env"), builtin_normal_expander( ext::env::expand_env)); @@ -748,19 +751,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..3ad9460c4ad90 100644 --- a/src/libsyntax/ext/format.rs +++ b/src/libsyntax/ext/format.rs @@ -632,6 +632,71 @@ 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 expr = match expr_string_lit(cx, arg2) { + Err(expr) => { + // input was not a string literal; just ignore it. + expr + } + Ok((expr, lit_str, _style)) => { + let m = format!("{} input cannot be format string literal", name); + for c in lit_str.chars() { + if c == '{' || c == '}' { + cx.span_err(sp, &m); + 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 { From ec1af4a7ce24e870cfc883dad5198c2be3bd8bf3 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 15 Apr 2015 10:51:04 +0200 Subject: [PATCH 03/11] revise `ensure_not_fmt_string_literal` to warn rather than error. As a drive-by, attempt to treat the macro as unstable by having it emit a reference to an unstable constant in its expansion. We *do* allow such an expansion when it occurs within `panic!`, for discussion, see large comment explaining the hack in the code. --- src/libsyntax/ext/format.rs | 44 +++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/src/libsyntax/ext/format.rs b/src/libsyntax/ext/format.rs index 3ad9460c4ad90..8c690e772754c 100644 --- a/src/libsyntax/ext/format.rs +++ b/src/libsyntax/ext/format.rs @@ -675,16 +675,23 @@ pub fn ensure_not_fmt_string_literal<'cx>(cx: &'cx mut ExtCtxt, }; // 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)) => { - let m = format!("{} input cannot be format string literal", name); for c in lit_str.chars() { if c == '{' || c == '}' { - cx.span_err(sp, &m); + warning(cx, c); break; } } @@ -694,6 +701,39 @@ pub fn ensure_not_fmt_string_literal<'cx>(cx: &'cx mut ExtCtxt, } }; + let unstable_marker = cx.expr_path(cx.path_global(sp, vec![ + cx.ident_of_std("core"), + cx.ident_of("fmt"), + cx.ident_of("ENSURE_NOT_FMT_STRING_LITERAL_IS_UNSTABLE")])); + + // Total hack: We do not (yet) have hygienic-marking of stabilty. + // Thus an unstable macro (like `ensure_not_fmt_string!`) can leak + // through another macro (like `panic!`), where the latter is just + // using the former as an implementation detail. + // + // The `#[allow_internal_unstable]` system does not suffice to + // address this; it explicitly (as described on Issue #22899) + // disallows the use of unstable functionality via a helper macro + // like `ensure_not_fmt_string!`, by design. + // + // So, the hack: the `ensure_not_fmt_string!` macro has just one + // stable client: `panic!`. So we give `panic!` a backdoor: we + // allow its name literal string to give it stable access. Any + // other argument that is passed in will cause us to emit the + // unstable-marker, which will then be checked against the enabled + // feature-set. + // + // This, combined with the awkward actual name of the unstable + // macro (hint: the real name is far more awkward than the one + // given in this comment) should suffice to ensure that people do + // not accidentally commit to using it. + let marker = if name == "unary `panic!`" { + cx.expr_tuple(sp, vec![]) // i.e. `()` + } else { + unstable_marker + }; + let expr = cx.expr_tuple(sp, vec![marker, expr]); + MacEager::expr(expr) } From 3f858875092e9ae9b7e1cbbcbdec2f3cbd783ae1 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 15 Apr 2015 11:00:15 +0200 Subject: [PATCH 04/11] rename macro to make it less palatable for use outside stdlib. --- src/libcore/macros.rs | 7 ++++--- src/libstd/macros.rs | 18 ++++++++++-------- src/libsyntax/ext/base.rs | 2 +- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/libcore/macros.rs b/src/libcore/macros.rs index fbf27b1363290..66b345c503219 100644 --- a/src/libcore/macros.rs +++ b/src/libcore/macros.rs @@ -11,8 +11,8 @@ // SNAP 5520801 #[cfg(stage0)] #[macro_export] -macro_rules! ensure_not_fmt_string_literal { - ($name:expr, $e:expr) => { $e } +macro_rules! __unstable_rustc_ensure_not_fmt_string_literal { + ($name:expr, $e:expr) => { ((), $e) } } /// Entry point of task panic, for details, see std::macros @@ -23,7 +23,8 @@ macro_rules! panic { ); ($msg:expr) => ({ static _MSG_FILE_LINE: (&'static str, &'static str, u32) = - (ensure_not_fmt_string_literal!("panic!", $msg), file!(), line!()); + (__unstable_rustc_ensure_not_fmt_string_literal!("unary `panic!`", $msg).1, + file!(), line!()); ::core::panicking::panic(&_MSG_FILE_LINE) }); ($fmt:expr, $($arg:tt)*) => ({ diff --git a/src/libstd/macros.rs b/src/libstd/macros.rs index adb6bbcdd8b34..daeed2f9d5ac7 100644 --- a/src/libstd/macros.rs +++ b/src/libstd/macros.rs @@ -19,8 +19,8 @@ // SNAP 5520801 #[cfg(stage0)] #[macro_export] -macro_rules! ensure_not_fmt_string_literal { - ($name:expr, $e:expr) => { $e } +macro_rules! __unstable_rustc_ensure_not_fmt_string_literal { + ($name:expr, $e:expr) => { ((), $e) } } /// The entry point for panic of Rust tasks. @@ -51,7 +51,8 @@ macro_rules! panic { panic!("explicit panic") }); ($msg:expr) => ({ - $crate::rt::begin_unwind(ensure_not_fmt_string_literal!("panic!", $msg), { + $crate::rt::begin_unwind( + __unstable_rustc_ensure_not_fmt_string_literal!("unary `panic!`", $msg).1, { // static requires less code at runtime, more constant data static _FILE_LINE: (&'static str, usize) = (file!(), line!() as usize); &_FILE_LINE @@ -97,11 +98,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 f1f5894cdb7dd..ca874f0d62919 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -458,7 +458,7 @@ 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)); - syntax_expanders.insert(intern("ensure_not_fmt_string_literal"), + syntax_expanders.insert(intern("__unstable_rustc_ensure_not_fmt_string_literal"), builtin_normal_expander( ext::format::ensure_not_fmt_string_literal)); syntax_expanders.insert(intern("env"), From 840afc3a637123da641010425647ecf23f33e407 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 16 Apr 2015 14:44:13 +0200 Subject: [PATCH 05/11] bug fix. --- src/libstd/macros.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/macros.rs b/src/libstd/macros.rs index daeed2f9d5ac7..9fb68e205e314 100644 --- a/src/libstd/macros.rs +++ b/src/libstd/macros.rs @@ -99,7 +99,7 @@ macro_rules! panic { }); ($msg:expr) => ({ $crate::rt::begin_unwind( - __unstable_rustc_ensure_not_fmt_string_literal!("unary `panic!`", $msg), { + __unstable_rustc_ensure_not_fmt_string_literal!("unary `panic!`", $msg).1, { // static requires less code at runtime, more constant data static _FILE_LINE: (&'static str, u32) = (file!(), line!()); &_FILE_LINE From f77c62b470d11cb546676d954c5adfeee8955529 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Mon, 13 Apr 2015 12:29:41 +0200 Subject: [PATCH 06/11] Regression test. --- .../issue-22932-ensure-helper-unstable.rs | 25 ++++++ src/test/compile-fail/issue-22932.rs | 76 +++++++++++++++++++ 2 files changed, 101 insertions(+) create mode 100644 src/test/compile-fail/issue-22932-ensure-helper-unstable.rs create mode 100644 src/test/compile-fail/issue-22932.rs diff --git a/src/test/compile-fail/issue-22932-ensure-helper-unstable.rs b/src/test/compile-fail/issue-22932-ensure-helper-unstable.rs new file mode 100644 index 0000000000000..bb0bda95461cf --- /dev/null +++ b/src/test/compile-fail/issue-22932-ensure-helper-unstable.rs @@ -0,0 +1,25 @@ +// 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. + +// This test is ensuring that the `ensure_not_fmt_string_literal!` +// macro cannot be invoked (at least not in code whose expansion ends +// up in the expanded output) without the appropriate feature. + +pub fn f0() { + panic!("this should work"); +} + +pub fn main() { + __unstable_rustc_ensure_not_fmt_string_literal!( + "`main`", "this should work, but its unstable"); + //~^^ ERROR use of unstable library feature 'ensure_not_fmt_string_literal' + //~| HELP add #![feature(ensure_not_fmt_string_literal)] to the crate attributes to enable + f0(); +} diff --git a/src/test/compile-fail/issue-22932.rs b/src/test/compile-fail/issue-22932.rs new file mode 100644 index 0000000000000..61f669b28b05b --- /dev/null +++ b/src/test/compile-fail/issue-22932.rs @@ -0,0 +1,76 @@ +// 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)] +#![feature(ensure_not_fmt_string_literal)] + +// 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 +} From ee540bc2fc192748e9694ebb6172c3ba6eec0d85 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 17 Apr 2015 22:24:02 +0200 Subject: [PATCH 07/11] allow helper macro to expand to unstable things, if necessary. --- src/libstd/macros.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libstd/macros.rs b/src/libstd/macros.rs index 9fb68e205e314..49b63d80569a3 100644 --- a/src/libstd/macros.rs +++ b/src/libstd/macros.rs @@ -19,6 +19,7 @@ // SNAP 5520801 #[cfg(stage0)] #[macro_export] +#[allow_internal_unstable] macro_rules! __unstable_rustc_ensure_not_fmt_string_literal { ($name:expr, $e:expr) => { ((), $e) } } From 4efe1445f58a5b9b3ed69a170ed02641b9f52559 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 17 Apr 2015 22:25:44 +0200 Subject: [PATCH 08/11] attempt to register the helper macro so that it allows unstable things internally. Unfortunately, i am having issues getting this to actually work. I repeatedly see: ``` failures: [run-fail] run-fail/explicit-panic-msg.rs ``` Namely: ``` failures: ---- [run-fail] run-fail/explicit-panic-msg.rs stdout ---- error: compilation failed! status: exit code: 101 command: x86_64-apple-darwin/stage2/bin/rustc /Users/fklock/Dev/Mozilla/rust-panic/src/test/run-fail/explicit-panic-msg.rs -L x86_64-apple-darwin/test/run-fail/ --target=x86_64-apple-darwin -L x86_64-apple-darwin/test/run-fail/explicit-panic-msg.stage2-x86_64-apple-darwinlibaux -C prefer-dynamic -o x86_64-apple-darwin/test/run-fail/explicit-panic-msg.stage2-x86_64-apple-darwin --cfg rtopt --cfg debug -L x86_64-apple-darwin/rt stdout: ------------------------------------------ ------------------------------------------ stderr: ------------------------------------------ :2:26: 2:57 error: use of unstable library feature 'core': internal to format_args! :2 $ crate:: fmt:: format ( format_args ! ( $ ( $ arg ) * ) ) ) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ note: in expansion of __unstable_rustc_ensure_not_fmt_string_literal! :4:1: 4:78 note: expansion site :1:1: 12:23 note: in expansion of panic! /Users/fklock/Dev/Mozilla/rust-panic/src/test/run-fail/explicit-panic-msg.rs:18:5: 18:37 note: expansion site :2:26: 2:57 help: add #![feature(core)] to the crate attributes to enable :2:26: 2:57 error: use of unstable library feature 'core': internal to format_args! :2 $ crate:: fmt:: format ( format_args ! ( $ ( $ arg ) * ) ) ) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ note: in expansion of __unstable_rustc_ensure_not_fmt_string_literal! :4:1: 4:78 note: expansion site :1:1: 12:23 note: in expansion of panic! /Users/fklock/Dev/Mozilla/rust-panic/src/test/run-fail/explicit-panic-msg.rs:18:5: 18:37 note: expansion site :2:26: 2:57 help: add #![feature(core)] to the crate attributes to enable error: aborting due to 2 previous errors ------------------------------------------ ``` and I have not yet figured out why this happens even now, nor how to resolve it. --- src/libsyntax/ext/base.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index ca874f0d62919..dcffdab4b93a8 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -458,9 +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)); - syntax_expanders.insert(intern("__unstable_rustc_ensure_not_fmt_string_literal"), - builtin_normal_expander( - ext::format::ensure_not_fmt_string_literal)); + + // 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)); From 5575da2bfe1c9f9dd7041944997d8292c5fd7246 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sat, 18 Apr 2015 11:20:08 +0200 Subject: [PATCH 09/11] remove attempts to encode macro's instability from implementation. --- src/libcore/macros.rs | 4 ++-- src/libstd/macros.rs | 6 +++--- src/libsyntax/ext/format.rs | 33 --------------------------------- 3 files changed, 5 insertions(+), 38 deletions(-) diff --git a/src/libcore/macros.rs b/src/libcore/macros.rs index 66b345c503219..d42b08b8c2d3b 100644 --- a/src/libcore/macros.rs +++ b/src/libcore/macros.rs @@ -12,7 +12,7 @@ #[cfg(stage0)] #[macro_export] macro_rules! __unstable_rustc_ensure_not_fmt_string_literal { - ($name:expr, $e:expr) => { ((), $e) } + ($name:expr, $e:expr) => { $e } } /// Entry point of task panic, for details, see std::macros @@ -23,7 +23,7 @@ macro_rules! panic { ); ($msg:expr) => ({ static _MSG_FILE_LINE: (&'static str, &'static str, u32) = - (__unstable_rustc_ensure_not_fmt_string_literal!("unary `panic!`", $msg).1, + (__unstable_rustc_ensure_not_fmt_string_literal!("unary `panic!`", $msg), file!(), line!()); ::core::panicking::panic(&_MSG_FILE_LINE) }); diff --git a/src/libstd/macros.rs b/src/libstd/macros.rs index 49b63d80569a3..209a8b3d2a647 100644 --- a/src/libstd/macros.rs +++ b/src/libstd/macros.rs @@ -21,7 +21,7 @@ #[macro_export] #[allow_internal_unstable] macro_rules! __unstable_rustc_ensure_not_fmt_string_literal { - ($name:expr, $e:expr) => { ((), $e) } + ($name:expr, $e:expr) => { $e } } /// The entry point for panic of Rust tasks. @@ -53,7 +53,7 @@ macro_rules! panic { }); ($msg:expr) => ({ $crate::rt::begin_unwind( - __unstable_rustc_ensure_not_fmt_string_literal!("unary `panic!`", $msg).1, { + __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 @@ -100,7 +100,7 @@ macro_rules! panic { }); ($msg:expr) => ({ $crate::rt::begin_unwind( - __unstable_rustc_ensure_not_fmt_string_literal!("unary `panic!`", $msg).1, { + __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 diff --git a/src/libsyntax/ext/format.rs b/src/libsyntax/ext/format.rs index 8c690e772754c..9e04e4620eabe 100644 --- a/src/libsyntax/ext/format.rs +++ b/src/libsyntax/ext/format.rs @@ -701,39 +701,6 @@ pub fn ensure_not_fmt_string_literal<'cx>(cx: &'cx mut ExtCtxt, } }; - let unstable_marker = cx.expr_path(cx.path_global(sp, vec![ - cx.ident_of_std("core"), - cx.ident_of("fmt"), - cx.ident_of("ENSURE_NOT_FMT_STRING_LITERAL_IS_UNSTABLE")])); - - // Total hack: We do not (yet) have hygienic-marking of stabilty. - // Thus an unstable macro (like `ensure_not_fmt_string!`) can leak - // through another macro (like `panic!`), where the latter is just - // using the former as an implementation detail. - // - // The `#[allow_internal_unstable]` system does not suffice to - // address this; it explicitly (as described on Issue #22899) - // disallows the use of unstable functionality via a helper macro - // like `ensure_not_fmt_string!`, by design. - // - // So, the hack: the `ensure_not_fmt_string!` macro has just one - // stable client: `panic!`. So we give `panic!` a backdoor: we - // allow its name literal string to give it stable access. Any - // other argument that is passed in will cause us to emit the - // unstable-marker, which will then be checked against the enabled - // feature-set. - // - // This, combined with the awkward actual name of the unstable - // macro (hint: the real name is far more awkward than the one - // given in this comment) should suffice to ensure that people do - // not accidentally commit to using it. - let marker = if name == "unary `panic!`" { - cx.expr_tuple(sp, vec![]) // i.e. `()` - } else { - unstable_marker - }; - let expr = cx.expr_tuple(sp, vec![marker, expr]); - MacEager::expr(expr) } From 0a46037503011ed91dd93968b7a59337a06df856 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sat, 18 Apr 2015 11:20:30 +0200 Subject: [PATCH 10/11] update unit test for checking unary panic input. --- .../issue-22932-ensure-helper-unstable.rs | 25 ------------------- src/test/compile-fail/issue-22932.rs | 1 - 2 files changed, 26 deletions(-) delete mode 100644 src/test/compile-fail/issue-22932-ensure-helper-unstable.rs diff --git a/src/test/compile-fail/issue-22932-ensure-helper-unstable.rs b/src/test/compile-fail/issue-22932-ensure-helper-unstable.rs deleted file mode 100644 index bb0bda95461cf..0000000000000 --- a/src/test/compile-fail/issue-22932-ensure-helper-unstable.rs +++ /dev/null @@ -1,25 +0,0 @@ -// 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. - -// This test is ensuring that the `ensure_not_fmt_string_literal!` -// macro cannot be invoked (at least not in code whose expansion ends -// up in the expanded output) without the appropriate feature. - -pub fn f0() { - panic!("this should work"); -} - -pub fn main() { - __unstable_rustc_ensure_not_fmt_string_literal!( - "`main`", "this should work, but its unstable"); - //~^^ ERROR use of unstable library feature 'ensure_not_fmt_string_literal' - //~| HELP add #![feature(ensure_not_fmt_string_literal)] to the crate attributes to enable - f0(); -} diff --git a/src/test/compile-fail/issue-22932.rs b/src/test/compile-fail/issue-22932.rs index 61f669b28b05b..4d379c0801d62 100644 --- a/src/test/compile-fail/issue-22932.rs +++ b/src/test/compile-fail/issue-22932.rs @@ -9,7 +9,6 @@ // except according to those terms. #![feature(rustc_attrs)] -#![feature(ensure_not_fmt_string_literal)] // Issue 22932: `panic!("{}");` should not compile. From b58359af401d6a574afff7b4568ff8919a1f1c84 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Sat, 18 Apr 2015 12:05:07 +0200 Subject: [PATCH 11/11] workaround bug in macro expansion. --- src/test/run-fail/explicit-panic-msg.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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); }