Skip to content

Commit 60eff20

Browse files
committed
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.
1 parent 04aaeb6 commit 60eff20

File tree

1 file changed

+42
-2
lines changed

1 file changed

+42
-2
lines changed

src/libsyntax/ext/format.rs

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -675,16 +675,23 @@ pub fn ensure_not_fmt_string_literal<'cx>(cx: &'cx mut ExtCtxt,
675675
};
676676

677677
// Second argument is the expression we are checking.
678+
let warning = |cx: &ExtCtxt, c: char| {
679+
cx.span_warn(sp, &format!("{} literal argument contains `{}`", name, c));
680+
cx.span_note(sp, "Is it meant to be a `format!` string?");
681+
cx.span_help(sp, "You can wrap the argument in parentheses \
682+
to sidestep this warning");
683+
};
684+
678685
let expr = match expr_string_lit(cx, arg2) {
679686
Err(expr) => {
680687
// input was not a string literal; just ignore it.
681688
expr
682689
}
690+
683691
Ok((expr, lit_str, _style)) => {
684-
let m = format!("{} input cannot be format string literal", name);
685692
for c in lit_str.chars() {
686693
if c == '{' || c == '}' {
687-
cx.span_err(sp, &m);
694+
warning(cx, c);
688695
break;
689696
}
690697
}
@@ -694,6 +701,39 @@ pub fn ensure_not_fmt_string_literal<'cx>(cx: &'cx mut ExtCtxt,
694701
}
695702
};
696703

704+
let unstable_marker = cx.expr_path(cx.path_global(sp, vec![
705+
cx.ident_of_std("core"),
706+
cx.ident_of("fmt"),
707+
cx.ident_of("ENSURE_NOT_FMT_STRING_LITERAL_IS_UNSTABLE")]));
708+
709+
// Total hack: We do not (yet) have hygienic-marking of stabilty.
710+
// Thus an unstable macro (like `ensure_not_fmt_string!`) can leak
711+
// through another macro (like `panic!`), where the latter is just
712+
// using the former as an implementation detail.
713+
//
714+
// The `#[allow_internal_unstable]` system does not suffice to
715+
// address this; it explicitly (as described on Issue #22899)
716+
// disallows the use of unstable functionality via a helper macro
717+
// like `ensure_not_fmt_string!`, by design.
718+
//
719+
// So, the hack: the `ensure_not_fmt_string!` macro has just one
720+
// stable client: `panic!`. So we give `panic!` a backdoor: we
721+
// allow its name literal string to give it stable access. Any
722+
// other argument that is passed in will cause us to emit the
723+
// unstable-marker, which will then be checked against the enabled
724+
// feature-set.
725+
//
726+
// This, combined with the awkward actual name of the unstable
727+
// macro (hint: the real name is far more awkward than the one
728+
// given in this comment) should suffice to ensure that people do
729+
// not accidentally commit to using it.
730+
let marker = if name == "unary `panic!`" {
731+
cx.expr_tuple(sp, vec![]) // i.e. `()`
732+
} else {
733+
unstable_marker
734+
};
735+
let expr = cx.expr_tuple(sp, vec![marker, expr]);
736+
697737
MacEager::expr(expr)
698738
}
699739

0 commit comments

Comments
 (0)