Skip to content

Commit 20c7d0f

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 94888a9 commit 20c7d0f

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
@@ -671,16 +671,23 @@ pub fn ensure_not_fmt_string_literal<'cx>(cx: &'cx mut ExtCtxt,
671671
};
672672

673673
// Second argument is the expression we are checking.
674+
let warning = |cx: &ExtCtxt, c: char| {
675+
cx.span_warn(sp, &format!("{} literal argument contains `{}`", name, c));
676+
cx.span_note(sp, "Is it meant to be a `format!` string?");
677+
cx.span_help(sp, "You can wrap the argument in parentheses \
678+
to sidestep this warning");
679+
};
680+
674681
let expr = match expr_string_lit(cx, arg2) {
675682
Err(expr) => {
676683
// input was not a string literal; just ignore it.
677684
expr
678685
}
686+
679687
Ok((expr, lit_str, _style)) => {
680-
let m = format!("{} input cannot be format string literal", name);
681688
for c in lit_str.chars() {
682689
if c == '{' || c == '}' {
683-
cx.span_err(sp, &m);
690+
warning(cx, c);
684691
break;
685692
}
686693
}
@@ -690,6 +697,39 @@ pub fn ensure_not_fmt_string_literal<'cx>(cx: &'cx mut ExtCtxt,
690697
}
691698
};
692699

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

0 commit comments

Comments
 (0)