Skip to content

Commit

Permalink
Add #[allow_internal_unstable] to track stability for macros better.
Browse files Browse the repository at this point in the history
Unstable items used in a macro expansion will now always trigger
stability warnings, *unless* the unstable items are directly inside a
macro marked with `#[allow_internal_unstable]`. IOW, the compiler warns
unless the span of the unstable item is a subspan of the definition of a
macro marked with that attribute.

E.g.

    #[allow_internal_unstable]
    macro_rules! foo {
        ($e: expr) => {{
            $e;
            unstable(); // no warning
            only_called_by_foo!();
        }}
    }

    macro_rules! only_called_by_foo {
        () => { unstable() } // warning
    }

    foo!(unstable()) // warning

The unstable inside `foo` is fine, due to the attribute. But the
`unstable` inside `only_called_by_foo` is not, since that macro doesn't
have the attribute, and the `unstable` passed into `foo` is also not
fine since it isn't contained in the macro itself (that is, even though
it is only used directly in the macro).

In the process this makes the stability tracking much more precise,
e.g. previously `println!("{}", unstable())` got no warning, but now it
does. As such, this is a bug fix that may cause [breaking-change]s.

The attribute is definitely feature gated, since it explicitly allows
side-stepping the feature gating system.
  • Loading branch information
huonw committed Mar 5, 2015
1 parent 68740b4 commit 84b060c
Show file tree
Hide file tree
Showing 21 changed files with 328 additions and 58 deletions.
8 changes: 8 additions & 0 deletions src/doc/reference.md
Expand Up @@ -2555,6 +2555,14 @@ The currently implemented features of the reference compiler are:
types, e.g. as the return type of a public function.
This capability may be removed in the future.

* `allow_internal_unstable` - Allows `macro_rules!` macros to be tagged with the
`#[allow_internal_unstable]` attribute, designed
to allow `std` macros to call
`#[unstable]`/feature-gated functionality
internally without imposing on callers
(i.e. making them behave like function calls in
terms of encapsulation).

If a feature is promoted to a language feature, then all existing programs will
start to receive compilation warnings about #[feature] directives which enabled
the new feature (because the directive is no longer necessary). However, if a
Expand Down
2 changes: 2 additions & 0 deletions src/libcore/num/mod.rs
Expand Up @@ -1238,6 +1238,7 @@ pub fn from_f64<A: FromPrimitive>(n: f64) -> Option<A> {

macro_rules! impl_from_primitive {
($T:ty, $to_ty:ident) => (
#[allow(deprecated)]
impl FromPrimitive for $T {
#[inline] fn from_int(n: int) -> Option<$T> { n.$to_ty() }
#[inline] fn from_i8(n: i8) -> Option<$T> { n.$to_ty() }
Expand Down Expand Up @@ -1299,6 +1300,7 @@ macro_rules! impl_num_cast {
($T:ty, $conv:ident) => (
impl NumCast for $T {
#[inline]
#[allow(deprecated)]
fn from<N: ToPrimitive>(n: N) -> Option<$T> {
// `$conv` could be generated using `concat_idents!`, but that
// macro seems to be broken at the moment
Expand Down
1 change: 1 addition & 0 deletions src/librustc/metadata/creader.rs
Expand Up @@ -542,6 +542,7 @@ impl<'a> CrateReader<'a> {
// overridden in plugin/load.rs
export: false,
use_locally: false,
allow_internal_unstable: false,

body: body,
});
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/metadata/macro_import.rs
Expand Up @@ -166,6 +166,9 @@ impl<'a> MacroLoader<'a> {
Some(sel) => sel.contains_key(&name),
};
def.export = reexport.contains_key(&name);
def.allow_internal_unstable = attr::contains_name(&def.attrs,
"allow_internal_unstable");
debug!("load_macros: loaded: {:?}", def);
self.macros.push(def);
}

Expand Down
5 changes: 2 additions & 3 deletions src/librustc/middle/stability.rs
Expand Up @@ -362,8 +362,6 @@ pub fn check_item(tcx: &ty::ctxt, item: &ast::Item, warn_about_defns: bool,
/// Helper for discovering nodes to check for stability
pub fn check_expr(tcx: &ty::ctxt, e: &ast::Expr,
cb: &mut FnMut(ast::DefId, Span, &Option<Stability>)) {
if is_internal(tcx, e.span) { return; }

let span;
let id = match e.node {
ast::ExprMethodCall(i, _, _) => {
Expand Down Expand Up @@ -527,12 +525,13 @@ pub fn check_pat(tcx: &ty::ctxt, pat: &ast::Pat,
fn maybe_do_stability_check(tcx: &ty::ctxt, id: ast::DefId, span: Span,
cb: &mut FnMut(ast::DefId, Span, &Option<Stability>)) {
if !is_staged_api(tcx, id) { return }
if is_internal(tcx, span) { return }
let ref stability = lookup(tcx, id);
cb(id, span, stability);
}

fn is_internal(tcx: &ty::ctxt, span: Span) -> bool {
tcx.sess.codemap().span_is_internal(span)
tcx.sess.codemap().span_allows_unstable(span)
}

fn is_staged_api(tcx: &ty::ctxt, id: DefId) -> bool {
Expand Down
11 changes: 8 additions & 3 deletions src/librustc/plugin/registry.rs
Expand Up @@ -81,8 +81,12 @@ impl<'a> Registry<'a> {
/// This is the most general hook into `libsyntax`'s expansion behavior.
pub fn register_syntax_extension(&mut self, name: ast::Name, extension: SyntaxExtension) {
self.syntax_exts.push((name, match extension {
NormalTT(ext, _) => NormalTT(ext, Some(self.krate_span)),
IdentTT(ext, _) => IdentTT(ext, Some(self.krate_span)),
NormalTT(ext, _, allow_internal_unstable) => {
NormalTT(ext, Some(self.krate_span), allow_internal_unstable)
}
IdentTT(ext, _, allow_internal_unstable) => {
IdentTT(ext, Some(self.krate_span), allow_internal_unstable)
}
Decorator(ext) => Decorator(ext),
Modifier(ext) => Modifier(ext),
MultiModifier(ext) => MultiModifier(ext),
Expand All @@ -99,7 +103,8 @@ impl<'a> Registry<'a> {
/// It builds for you a `NormalTT` that calls `expander`,
/// and also takes care of interning the macro's name.
pub fn register_macro(&mut self, name: &str, expander: MacroExpanderFn) {
self.register_syntax_extension(token::intern(name), NormalTT(Box::new(expander), None));
self.register_syntax_extension(token::intern(name),
NormalTT(Box::new(expander), None, false));
}

/// Register a compiler lint pass.
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/ast.rs
Expand Up @@ -1743,6 +1743,7 @@ pub struct MacroDef {
pub imported_from: Option<Ident>,
pub export: bool,
pub use_locally: bool,
pub allow_internal_unstable: bool,
pub body: Vec<TokenTree>,
}

Expand Down
70 changes: 38 additions & 32 deletions src/libsyntax/codemap.rs
Expand Up @@ -243,6 +243,10 @@ pub struct NameAndSpan {
pub name: String,
/// The format with which the macro was invoked.
pub format: MacroFormat,
/// Whether the macro is allowed to use #[unstable]/feature-gated
/// features internally without forcing the whole crate to opt-in
/// to them.
pub allow_internal_unstable: bool,
/// The span of the macro definition itself. The macro may not
/// have a sensible definition span (e.g. something defined
/// completely inside libsyntax) in which case this is None.
Expand Down Expand Up @@ -830,41 +834,43 @@ impl CodeMap {
}
}

/// Check if a span is "internal" to a macro. This means that it is entirely generated by a
/// macro expansion and contains no code that was passed in as an argument.
pub fn span_is_internal(&self, span: Span) -> bool {
// first, check if the given expression was generated by a macro or not
// we need to go back the expn_info tree to check only the arguments
// of the initial macro call, not the nested ones.
let mut is_internal = false;
let mut expnid = span.expn_id;
while self.with_expn_info(expnid, |expninfo| {
match expninfo {
Some(ref info) => {
// save the parent expn_id for next loop iteration
expnid = info.call_site.expn_id;
if info.callee.name == "format_args" {
// This is a hack because the format_args builtin calls unstable APIs.
// I spent like 6 hours trying to solve this more generally but am stupid.
is_internal = true;
false
} else if info.callee.span.is_none() {
// it's a compiler built-in, we *really* don't want to mess with it
// so we skip it, unless it was called by a regular macro, in which case
// we will handle the caller macro next turn
is_internal = true;
true // continue looping
/// Check if a span is "internal" to a macro in which #[unstable]
/// items can be used (that is, a macro marked with
/// `#[allow_internal_unstable]`).
pub fn span_allows_unstable(&self, span: Span) -> bool {
debug!("span_allows_unstable(span = {:?})", span);
let mut allows_unstable = false;
let mut expn_id = span.expn_id;
loop {
let quit = self.with_expn_info(expn_id, |expninfo| {
debug!("span_allows_unstable: expninfo = {:?}", expninfo);
expninfo.map_or(/* hit the top level */ true, |info| {

let span_comes_from_this_expansion =
info.callee.span.map_or(span == info.call_site, |mac_span| {
mac_span.lo <= span.lo && span.hi < mac_span.hi
});

debug!("span_allows_unstable: from this expansion? {}, allows unstable? {}",
span_comes_from_this_expansion,
info.callee.allow_internal_unstable);
if span_comes_from_this_expansion {
allows_unstable = info.callee.allow_internal_unstable;
// we've found the right place, stop looking
true
} else {
// was this expression from the current macro arguments ?
is_internal = !( span.lo > info.call_site.lo &&
span.hi < info.call_site.hi );
true // continue looping
// not the right place, keep looking
expn_id = info.call_site.expn_id;
false
}
},
_ => false // stop looping
})
});
if quit {
break
}
}) { /* empty while loop body */ }
return is_internal;
}
debug!("span_allows_unstable? {}", allows_unstable);
allows_unstable
}
}

Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/ext/asm.rs
Expand Up @@ -214,6 +214,7 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree])
name: "asm".to_string(),
format: codemap::MacroBang,
span: None,
allow_internal_unstable: false,
},
});

Expand Down
13 changes: 8 additions & 5 deletions src/libsyntax/ext/base.rs
Expand Up @@ -430,12 +430,15 @@ pub enum SyntaxExtension {
/// A normal, function-like syntax extension.
///
/// `bytes!` is a `NormalTT`.
NormalTT(Box<TTMacroExpander + 'static>, Option<Span>),
///
/// The `bool` dictates whether the contents of the macro can
/// directly use `#[unstable]` things (true == yes).
NormalTT(Box<TTMacroExpander + 'static>, Option<Span>, bool),

/// A function-like syntax extension that has an extra ident before
/// the block.
///
IdentTT(Box<IdentMacroExpander + 'static>, Option<Span>),
IdentTT(Box<IdentMacroExpander + 'static>, Option<Span>, bool),

/// Represents `macro_rules!` itself.
MacroRulesTT,
Expand Down Expand Up @@ -465,14 +468,14 @@ fn initial_syntax_expander_table<'feat>(ecfg: &expand::ExpansionConfig<'feat>)
-> SyntaxEnv {
// utility function to simplify creating NormalTT syntax extensions
fn builtin_normal_expander(f: MacroExpanderFn) -> SyntaxExtension {
NormalTT(Box::new(f), None)
NormalTT(Box::new(f), None, false)
}

let mut syntax_expanders = SyntaxEnv::new();
syntax_expanders.insert(intern("macro_rules"), MacroRulesTT);
syntax_expanders.insert(intern("format_args"),
builtin_normal_expander(
ext::format::expand_format_args));
// format_args uses `unstable` things internally.
NormalTT(Box::new(ext::format::expand_format_args), None, true));
syntax_expanders.insert(intern("env"),
builtin_normal_expander(
ext::env::expand_env));
Expand Down
3 changes: 2 additions & 1 deletion src/libsyntax/ext/deriving/generic/mod.rs
Expand Up @@ -1216,7 +1216,8 @@ impl<'a> TraitDef<'a> {
callee: codemap::NameAndSpan {
name: format!("derive({})", trait_name),
format: codemap::MacroAttribute,
span: Some(self.span)
span: Some(self.span),
allow_internal_unstable: false,
}
});
to_set
Expand Down

0 comments on commit 84b060c

Please sign in to comment.