From 84b060ce29bf7dd65fc23e855ad7c5a8748d806c Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Sun, 1 Mar 2015 14:09:28 +1100 Subject: [PATCH 1/4] Add #[allow_internal_unstable] to track stability for macros better. 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. --- src/doc/reference.md | 8 +++ src/libcore/num/mod.rs | 2 + src/librustc/metadata/creader.rs | 1 + src/librustc/metadata/macro_import.rs | 3 + src/librustc/middle/stability.rs | 5 +- src/librustc/plugin/registry.rs | 11 ++- src/libsyntax/ast.rs | 1 + src/libsyntax/codemap.rs | 70 ++++++++++--------- src/libsyntax/ext/asm.rs | 1 + src/libsyntax/ext/base.rs | 13 ++-- src/libsyntax/ext/deriving/generic/mod.rs | 3 +- src/libsyntax/ext/expand.rs | 58 ++++++++++++--- src/libsyntax/ext/tt/macro_rules.rs | 2 +- src/libsyntax/feature_gate.rs | 20 +++++- src/libsyntax/test.rs | 6 +- src/test/auxiliary/internal_unstable.rs | 60 ++++++++++++++++ src/test/auxiliary/plugin_args.rs | 2 +- ...te-allow-internal-unstable-nested-macro.rs | 23 ++++++ .../feature-gate-allow-internal-unstable.rs | 16 +++++ .../compile-fail/internal-unstable-noallow.rs | 30 ++++++++ src/test/compile-fail/internal-unstable.rs | 51 ++++++++++++++ 21 files changed, 328 insertions(+), 58 deletions(-) create mode 100644 src/test/auxiliary/internal_unstable.rs create mode 100644 src/test/compile-fail/feature-gate-allow-internal-unstable-nested-macro.rs create mode 100644 src/test/compile-fail/feature-gate-allow-internal-unstable.rs create mode 100644 src/test/compile-fail/internal-unstable-noallow.rs create mode 100755 src/test/compile-fail/internal-unstable.rs diff --git a/src/doc/reference.md b/src/doc/reference.md index fa4986816aa1c..95ccc71532a34 100644 --- a/src/doc/reference.md +++ b/src/doc/reference.md @@ -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 diff --git a/src/libcore/num/mod.rs b/src/libcore/num/mod.rs index 92cdd84160b7a..76edec80896bb 100644 --- a/src/libcore/num/mod.rs +++ b/src/libcore/num/mod.rs @@ -1238,6 +1238,7 @@ pub fn from_f64(n: f64) -> Option { 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() } @@ -1299,6 +1300,7 @@ macro_rules! impl_num_cast { ($T:ty, $conv:ident) => ( impl NumCast for $T { #[inline] + #[allow(deprecated)] fn from(n: N) -> Option<$T> { // `$conv` could be generated using `concat_idents!`, but that // macro seems to be broken at the moment diff --git a/src/librustc/metadata/creader.rs b/src/librustc/metadata/creader.rs index a19ccceb05715..515e85410d9fd 100644 --- a/src/librustc/metadata/creader.rs +++ b/src/librustc/metadata/creader.rs @@ -542,6 +542,7 @@ impl<'a> CrateReader<'a> { // overridden in plugin/load.rs export: false, use_locally: false, + allow_internal_unstable: false, body: body, }); diff --git a/src/librustc/metadata/macro_import.rs b/src/librustc/metadata/macro_import.rs index d25dc4f58a5df..336ebb55e0c42 100644 --- a/src/librustc/metadata/macro_import.rs +++ b/src/librustc/metadata/macro_import.rs @@ -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); } diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index ddac6cc75143b..3599ba5a0f779 100644 --- a/src/librustc/middle/stability.rs +++ b/src/librustc/middle/stability.rs @@ -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)) { - if is_internal(tcx, e.span) { return; } - let span; let id = match e.node { ast::ExprMethodCall(i, _, _) => { @@ -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)) { 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 { diff --git a/src/librustc/plugin/registry.rs b/src/librustc/plugin/registry.rs index 6f98b79e782c0..78f7b3b91ddf7 100644 --- a/src/librustc/plugin/registry.rs +++ b/src/librustc/plugin/registry.rs @@ -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), @@ -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. diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 05348ee77e81f..b41eb05ded695 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -1743,6 +1743,7 @@ pub struct MacroDef { pub imported_from: Option, pub export: bool, pub use_locally: bool, + pub allow_internal_unstable: bool, pub body: Vec, } diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index 162da2ac54c12..44df8a6d3daec 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -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. @@ -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 } } diff --git a/src/libsyntax/ext/asm.rs b/src/libsyntax/ext/asm.rs index e58a3de41c05c..d256698b88598 100644 --- a/src/libsyntax/ext/asm.rs +++ b/src/libsyntax/ext/asm.rs @@ -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, }, }); diff --git a/src/libsyntax/ext/base.rs b/src/libsyntax/ext/base.rs index f6bdd693cfa21..80b86507f6ea7 100644 --- a/src/libsyntax/ext/base.rs +++ b/src/libsyntax/ext/base.rs @@ -430,12 +430,15 @@ pub enum SyntaxExtension { /// A normal, function-like syntax extension. /// /// `bytes!` is a `NormalTT`. - NormalTT(Box, Option), + /// + /// The `bool` dictates whether the contents of the macro can + /// directly use `#[unstable]` things (true == yes). + NormalTT(Box, Option, bool), /// A function-like syntax extension that has an extra ident before /// the block. /// - IdentTT(Box, Option), + IdentTT(Box, Option, bool), /// Represents `macro_rules!` itself. MacroRulesTT, @@ -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)); diff --git a/src/libsyntax/ext/deriving/generic/mod.rs b/src/libsyntax/ext/deriving/generic/mod.rs index a36d3a155b8d8..9cd965a8138e0 100644 --- a/src/libsyntax/ext/deriving/generic/mod.rs +++ b/src/libsyntax/ext/deriving/generic/mod.rs @@ -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 diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index bea57ae14e4af..8953689a0c282 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -22,7 +22,7 @@ use attr::AttrMetaMethods; use codemap; use codemap::{Span, Spanned, ExpnInfo, NameAndSpan, MacroBang, MacroAttribute}; use ext::base::*; -use feature_gate::{Features}; +use feature_gate::{self, Features}; use fold; use fold::*; use parse; @@ -395,13 +395,14 @@ fn expand_mac_invoc(mac: ast::Mac, span: codemap::Span, None } Some(rc) => match *rc { - NormalTT(ref expandfun, exp_span) => { + NormalTT(ref expandfun, exp_span, allow_internal_unstable) => { fld.cx.bt_push(ExpnInfo { call_site: span, callee: NameAndSpan { name: extnamestr.to_string(), format: MacroBang, span: exp_span, + allow_internal_unstable: allow_internal_unstable, }, }); let fm = fresh_mark(); @@ -530,6 +531,9 @@ fn expand_item_modifiers(mut it: P, fld: &mut MacroExpander) name: mname.to_string(), format: MacroAttribute, span: None, + // attributes can do whatever they like, + // for now + allow_internal_unstable: true, } }); it = mac.expand(fld.cx, attr.span, &*attr.node.value, it); @@ -614,7 +618,7 @@ pub fn expand_item_mac(it: P, } Some(rc) => match *rc { - NormalTT(ref expander, span) => { + NormalTT(ref expander, span, allow_internal_unstable) => { if it.ident.name != parse::token::special_idents::invalid.name { fld.cx .span_err(path_span, @@ -628,14 +632,15 @@ pub fn expand_item_mac(it: P, callee: NameAndSpan { name: extnamestr.to_string(), format: MacroBang, - span: span + span: span, + allow_internal_unstable: allow_internal_unstable, } }); // mark before expansion: let marked_before = mark_tts(&tts[..], fm); expander.expand(fld.cx, it.span, &marked_before[..]) } - IdentTT(ref expander, span) => { + IdentTT(ref expander, span, allow_internal_unstable) => { if it.ident.name == parse::token::special_idents::invalid.name { fld.cx.span_err(path_span, &format!("macro {}! expects an ident argument", @@ -647,7 +652,8 @@ pub fn expand_item_mac(it: P, callee: NameAndSpan { name: extnamestr.to_string(), format: MacroBang, - span: span + span: span, + allow_internal_unstable: allow_internal_unstable, } }); // mark before expansion: @@ -661,16 +667,35 @@ pub fn expand_item_mac(it: P, ); return SmallVector::zero(); } + fld.cx.bt_push(ExpnInfo { call_site: it.span, callee: NameAndSpan { name: extnamestr.to_string(), format: MacroBang, span: None, + // `macro_rules!` doesn't directly allow + // unstable (this is orthogonal to whether + // the macro it creates allows it) + allow_internal_unstable: false, } }); // DON'T mark before expansion. + let allow_internal_unstable = attr::contains_name(&it.attrs, + "allow_internal_unstable"); + + // ensure any #[allow_internal_unstable]s are + // detected (including nested macro definitions + // etc.) + if allow_internal_unstable && !fld.cx.ecfg.enable_allow_internal_unstable() { + feature_gate::emit_feature_err( + &fld.cx.parse_sess.span_diagnostic, + "allow_internal_unstable", + it.span, + feature_gate::EXPLAIN_ALLOW_INTERNAL_UNSTABLE) + } + let def = ast::MacroDef { ident: it.ident, attrs: it.attrs.clone(), @@ -679,6 +704,7 @@ pub fn expand_item_mac(it: P, imported_from: None, export: attr::contains_name(&it.attrs, "macro_export"), use_locally: true, + allow_internal_unstable: allow_internal_unstable, body: tts, }; fld.cx.insert_macro(def); @@ -959,13 +985,14 @@ fn expand_pat(p: P, fld: &mut MacroExpander) -> P { } Some(rc) => match *rc { - NormalTT(ref expander, tt_span) => { + NormalTT(ref expander, tt_span, allow_internal_unstable) => { fld.cx.bt_push(ExpnInfo { call_site: span, callee: NameAndSpan { name: extnamestr.to_string(), format: MacroBang, - span: tt_span + span: tt_span, + allow_internal_unstable: allow_internal_unstable, } }); @@ -1094,7 +1121,10 @@ fn expand_annotatable(a: Annotatable, callee: NameAndSpan { name: mname.to_string(), format: MacroAttribute, - span: None + span: None, + // attributes can do whatever they like, + // for now. + allow_internal_unstable: true, } }); @@ -1244,6 +1274,9 @@ fn expand_item_multi_modifier(mut it: Annotatable, name: mname.to_string(), format: MacroAttribute, span: None, + // attributes can do whatever they like, + // for now + allow_internal_unstable: true, } }); it = mac.expand(fld.cx, attr.span, &*attr.node.value, it); @@ -1457,6 +1490,13 @@ impl<'feat> ExpansionConfig<'feat> { _ => false, } } + + pub fn enable_allow_internal_unstable(&self) -> bool { + match self.features { + Some(&Features { allow_internal_unstable: true, .. }) => true, + _ => false + } + } } pub fn expand_crate<'feat>(parse_sess: &parse::ParseSess, diff --git a/src/libsyntax/ext/tt/macro_rules.rs b/src/libsyntax/ext/tt/macro_rules.rs index d15eb3df6394b..644c6cd7e2833 100644 --- a/src/libsyntax/ext/tt/macro_rules.rs +++ b/src/libsyntax/ext/tt/macro_rules.rs @@ -267,7 +267,7 @@ pub fn compile<'cx>(cx: &'cx mut ExtCtxt, rhses: rhses, }; - NormalTT(exp, Some(def.span)) + NormalTT(exp, Some(def.span), def.allow_internal_unstable) } fn check_lhs_nt_follows(cx: &mut ExtCtxt, lhs: &NamedMatch, sp: Span) { diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 18d3f85f4b58e..3b2d86cffe22c 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -142,6 +142,12 @@ const KNOWN_FEATURES: &'static [(&'static str, &'static str, Status)] = &[ // Allows the use of `static_assert` ("static_assert", "1.0.0", Active), + + // Allows the use of #[allow_internal_unstable]. This is an + // attribute on macro_rules! and can't use the attribute handling + // below (it has to be checked before expansion possibly makes + // macros disappear). + ("allow_internal_unstable", "1.0.0", Active), ]; // (changing above list without updating src/doc/reference.md makes @cmr sad) @@ -308,6 +314,7 @@ pub struct Features { pub allow_log_syntax: bool, pub allow_concat_idents: bool, pub allow_trace_macros: bool, + pub allow_internal_unstable: bool, pub old_orphan_check: bool, pub simd_ffi: bool, pub unmarked_api: bool, @@ -328,6 +335,7 @@ impl Features { allow_log_syntax: false, allow_concat_idents: false, allow_trace_macros: false, + allow_internal_unstable: false, old_orphan_check: false, simd_ffi: false, unmarked_api: false, @@ -387,6 +395,8 @@ pub const EXPLAIN_CONCAT_IDENTS: &'static str = pub const EXPLAIN_TRACE_MACROS: &'static str = "`trace_macros` is not stable enough for use and is subject to change"; +pub const EXPLAIN_ALLOW_INTERNAL_UNSTABLE: &'static str = + "allow_internal_unstable side-steps feature gating and stability checks"; struct MacroVisitor<'a> { context: &'a Context<'a> @@ -421,6 +431,13 @@ impl<'a, 'v> Visitor<'v> for MacroVisitor<'a> { self.context.gate_feature("concat_idents", path.span, EXPLAIN_CONCAT_IDENTS); } } + + fn visit_attribute(&mut self, attr: &'v ast::Attribute) { + if attr.name() == "allow_internal_unstable" { + self.context.gate_feature("allow_internal_unstable", attr.span, + EXPLAIN_ALLOW_INTERNAL_UNSTABLE) + } + } } struct PostExpansionVisitor<'a> { @@ -429,7 +446,7 @@ struct PostExpansionVisitor<'a> { impl<'a> PostExpansionVisitor<'a> { fn gate_feature(&self, feature: &str, span: Span, explain: &str) { - if !self.context.cm.span_is_internal(span) { + if !self.context.cm.span_allows_unstable(span) { self.context.gate_feature(feature, span, explain) } } @@ -754,6 +771,7 @@ fn check_crate_inner(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::C allow_log_syntax: cx.has_feature("log_syntax"), allow_concat_idents: cx.has_feature("concat_idents"), allow_trace_macros: cx.has_feature("trace_macros"), + allow_internal_unstable: cx.has_feature("allow_internal_unstable"), old_orphan_check: cx.has_feature("old_orphan_check"), simd_ffi: cx.has_feature("simd_ffi"), unmarked_api: cx.has_feature("unmarked_api"), diff --git a/src/libsyntax/test.rs b/src/libsyntax/test.rs index 5bada41badfd8..d889453a5f977 100644 --- a/src/libsyntax/test.rs +++ b/src/libsyntax/test.rs @@ -256,7 +256,8 @@ fn generate_test_harness(sess: &ParseSess, callee: NameAndSpan { name: "test".to_string(), format: MacroAttribute, - span: None + span: None, + allow_internal_unstable: false, } }); @@ -288,7 +289,8 @@ fn ignored_span(cx: &TestCtxt, sp: Span) -> Span { callee: NameAndSpan { name: "test".to_string(), format: MacroAttribute, - span: None + span: None, + allow_internal_unstable: true, } }; let expn_id = cx.sess.span_diagnostic.cm.record_expansion(info); diff --git a/src/test/auxiliary/internal_unstable.rs b/src/test/auxiliary/internal_unstable.rs new file mode 100644 index 0000000000000..3d59b8e900992 --- /dev/null +++ b/src/test/auxiliary/internal_unstable.rs @@ -0,0 +1,60 @@ +// 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(staged_api, allow_internal_unstable)] +#![staged_api] +#![stable(feature = "stable", since = "1.0.0")] + +#[unstable(feature = "function")] +pub fn unstable() {} + + +#[stable(feature = "stable", since = "1.0.0")] +pub struct Foo { + #[unstable(feature = "struct_field")] + pub x: u8 +} + +#[allow_internal_unstable] +#[macro_export] +macro_rules! call_unstable_allow { + () => { $crate::unstable() } +} + +#[allow_internal_unstable] +#[macro_export] +macro_rules! construct_unstable_allow { + ($e: expr) => { + $crate::Foo { x: $e } + } +} + +#[allow_internal_unstable] +#[macro_export] +macro_rules! pass_through_allow { + ($e: expr) => { $e } +} + +#[macro_export] +macro_rules! call_unstable_noallow { + () => { $crate::unstable() } +} + +#[macro_export] +macro_rules! construct_unstable_noallow { + ($e: expr) => { + $crate::Foo { x: $e } + } +} + +#[macro_export] +macro_rules! pass_through_noallow { + ($e: expr) => { $e } +} diff --git a/src/test/auxiliary/plugin_args.rs b/src/test/auxiliary/plugin_args.rs index 20c84c4ba5b4a..30b18a3618ffa 100644 --- a/src/test/auxiliary/plugin_args.rs +++ b/src/test/auxiliary/plugin_args.rs @@ -47,5 +47,5 @@ pub fn plugin_registrar(reg: &mut Registry) { let args = reg.args().clone(); reg.register_syntax_extension(token::intern("plugin_args"), // FIXME (#22405): Replace `Box::new` with `box` here when/if possible. - NormalTT(Box::new(Expander { args: args, }), None)); + NormalTT(Box::new(Expander { args: args, }), None, false)); } diff --git a/src/test/compile-fail/feature-gate-allow-internal-unstable-nested-macro.rs b/src/test/compile-fail/feature-gate-allow-internal-unstable-nested-macro.rs new file mode 100644 index 0000000000000..c9251c925cc40 --- /dev/null +++ b/src/test/compile-fail/feature-gate-allow-internal-unstable-nested-macro.rs @@ -0,0 +1,23 @@ +// 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. + +macro_rules! bar { + () => { + // more layers don't help: + #[allow_internal_unstable] + macro_rules! baz { //~ ERROR allow_internal_unstable side-steps + () => {} + } + } +} + +bar!(); + +fn main() {} diff --git a/src/test/compile-fail/feature-gate-allow-internal-unstable.rs b/src/test/compile-fail/feature-gate-allow-internal-unstable.rs new file mode 100644 index 0000000000000..8a2d8dddac074 --- /dev/null +++ b/src/test/compile-fail/feature-gate-allow-internal-unstable.rs @@ -0,0 +1,16 @@ +// 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. + +#[allow_internal_unstable] //~ ERROR allow_internal_unstable side-steps +macro_rules! foo { + () => {} +} + +fn main() {} diff --git a/src/test/compile-fail/internal-unstable-noallow.rs b/src/test/compile-fail/internal-unstable-noallow.rs new file mode 100644 index 0000000000000..4e296198be8e4 --- /dev/null +++ b/src/test/compile-fail/internal-unstable-noallow.rs @@ -0,0 +1,30 @@ +// 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 has to be separate to internal-unstable.rs because these tests +// have error messages pointing deep into the internals of the +// cross-crate macros, and hence need to use error-pattern instead of +// the // ~ form. + +// aux-build:internal_unstable.rs +// error-pattern:use of unstable library feature 'function' +// error-pattern:use of unstable library feature 'struct_field' +// error-pattern:compilation successful +#![feature(rustc_attrs)] + +#[macro_use] +extern crate internal_unstable; + +#[rustc_error] +fn main() { + call_unstable_noallow!(); + + construct_unstable_noallow!(0); +} diff --git a/src/test/compile-fail/internal-unstable.rs b/src/test/compile-fail/internal-unstable.rs new file mode 100755 index 0000000000000..8674e8ab5b2cd --- /dev/null +++ b/src/test/compile-fail/internal-unstable.rs @@ -0,0 +1,51 @@ +// 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. + +// aux-build:internal_unstable.rs + +#![feature(rustc_attrs, allow_internal_unstable)] + +#[macro_use] +extern crate internal_unstable; + +macro_rules! foo { + ($e: expr, $f: expr) => {{ + $e; + $f; + internal_unstable::unstable(); //~ WARN use of unstable + }} +} + +#[allow_internal_unstable] +macro_rules! bar { + ($e: expr) => {{ + foo!($e, + internal_unstable::unstable()); + internal_unstable::unstable(); + }} +} + +#[rustc_error] +fn main() { //~ ERROR + // ok, the instability is contained. + call_unstable_allow!(); + construct_unstable_allow!(0); + + // bad. + pass_through_allow!(internal_unstable::unstable()); //~ WARN use of unstable + + pass_through_noallow!(internal_unstable::unstable()); //~ WARN use of unstable + + + + println!("{:?}", internal_unstable::unstable()); //~ WARN use of unstable + + bar!(internal_unstable::unstable()); //~ WARN use of unstable +} From ab7ef7402bfab1c767b8be80f7e46947494f6d21 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Sun, 1 Mar 2015 14:09:42 +1100 Subject: [PATCH 2/4] Use `#[allow_internal_unstable]` for `thread_local!` This destabilises all the implementation details of `thread_local!`, since they do not *need* to be stable with the new attribute. --- src/libstd/lib.rs | 1 + src/libstd/sys/common/thread_local.rs | 6 +---- src/libstd/thread_local/mod.rs | 22 ++++++++++-------- src/libstd/thread_local/scoped.rs | 2 ++ .../internal-unstable-thread-local.rs | 23 +++++++++++++++++++ 5 files changed, 39 insertions(+), 15 deletions(-) create mode 100644 src/test/compile-fail/internal-unstable-thread-local.rs diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 569906047aab3..c0db163e0874d 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -125,6 +125,7 @@ #![feature(hash)] #![feature(int_uint)] #![feature(unique)] +#![feature(allow_internal_unstable)] #![cfg_attr(test, feature(test, rustc_private))] // Don't link to std. We are std. diff --git a/src/libstd/sys/common/thread_local.rs b/src/libstd/sys/common/thread_local.rs index 27b8784e3943a..91de2662883f8 100644 --- a/src/libstd/sys/common/thread_local.rs +++ b/src/libstd/sys/common/thread_local.rs @@ -55,6 +55,7 @@ //! ``` #![allow(non_camel_case_types)] +#![unstable(feature = "thread_local_internals")] use prelude::v1::*; @@ -84,17 +85,14 @@ use sys::thread_local as imp; /// KEY.set(1 as *mut u8); /// } /// ``` -#[stable(feature = "rust1", since = "1.0.0")] pub struct StaticKey { /// Inner static TLS key (internals), created with by `INIT_INNER` in this /// module. - #[stable(feature = "rust1", since = "1.0.0")] pub inner: StaticKeyInner, /// Destructor for the TLS value. /// /// See `Key::new` for information about when the destructor runs and how /// it runs. - #[stable(feature = "rust1", since = "1.0.0")] pub dtor: Option, } @@ -131,7 +129,6 @@ pub struct Key { /// Constant initialization value for static TLS keys. /// /// This value specifies no destructor by default. -#[stable(feature = "rust1", since = "1.0.0")] pub const INIT: StaticKey = StaticKey { inner: INIT_INNER, dtor: None, @@ -140,7 +137,6 @@ pub const INIT: StaticKey = StaticKey { /// Constant initialization value for the inner part of static TLS keys. /// /// This value allows specific configuration of the destructor for a TLS key. -#[stable(feature = "rust1", since = "1.0.0")] pub const INIT_INNER: StaticKeyInner = StaticKeyInner { key: atomic::ATOMIC_USIZE_INIT, }; diff --git a/src/libstd/thread_local/mod.rs b/src/libstd/thread_local/mod.rs index 764c7d730cb0e..6bba73420d851 100644 --- a/src/libstd/thread_local/mod.rs +++ b/src/libstd/thread_local/mod.rs @@ -45,7 +45,7 @@ pub mod scoped; // Sure wish we had macro hygiene, no? #[doc(hidden)] -#[stable(feature = "rust1", since = "1.0.0")] +#[unstable(feature = "thread_local_internals")] pub mod __impl { pub use super::imp::Key as KeyInner; pub use super::imp::destroy_value; @@ -117,6 +117,7 @@ pub struct Key { /// Declare a new thread local storage key of type `std::thread_local::Key`. #[macro_export] #[stable(feature = "rust1", since = "1.0.0")] +#[allow_internal_unstable] macro_rules! thread_local { (static $name:ident: $t:ty = $init:expr) => ( static $name: ::std::thread_local::Key<$t> = { @@ -176,6 +177,7 @@ macro_rules! thread_local { #[macro_export] #[doc(hidden)] +#[allow_internal_unstable] macro_rules! __thread_local_inner { (static $name:ident: $t:ty = $init:expr) => ( #[cfg_attr(all(any(target_os = "macos", target_os = "linux"), @@ -337,7 +339,7 @@ mod imp { use ptr; #[doc(hidden)] - #[stable(since = "1.0.0", feature = "rust1")] + #[unstable(feature = "thread_local_internals")] pub struct Key { // Place the inner bits in an `UnsafeCell` to currently get around the // "only Sync statics" restriction. This allows any type to be placed in @@ -345,14 +347,14 @@ mod imp { // // Note that all access requires `T: 'static` so it can't be a type with // any borrowed pointers still. - #[stable(since = "1.0.0", feature = "rust1")] + #[unstable(feature = "thread_local_internals")] pub inner: UnsafeCell, // Metadata to keep track of the state of the destructor. Remember that // these variables are thread-local, not global. - #[stable(since = "1.0.0", feature = "rust1")] + #[unstable(feature = "thread_local_internals")] pub dtor_registered: UnsafeCell, // should be Cell - #[stable(since = "1.0.0", feature = "rust1")] + #[unstable(feature = "thread_local_internals")] pub dtor_running: UnsafeCell, // should be Cell } @@ -455,7 +457,7 @@ mod imp { } #[doc(hidden)] - #[stable(feature = "rust1", since = "1.0.0")] + #[unstable(feature = "thread_local_internals")] pub unsafe extern fn destroy_value(ptr: *mut u8) { let ptr = ptr as *mut Key; // Right before we run the user destructor be sure to flag the @@ -477,15 +479,15 @@ mod imp { use sys_common::thread_local::StaticKey as OsStaticKey; #[doc(hidden)] - #[stable(since = "1.0.0", feature = "rust1")] + #[unstable(feature = "thread_local_internals")] pub struct Key { // Statically allocated initialization expression, using an `UnsafeCell` // for the same reasons as above. - #[stable(since = "1.0.0", feature = "rust1")] + #[unstable(feature = "thread_local_internals")] pub inner: UnsafeCell, // OS-TLS key that we'll use to key off. - #[stable(since = "1.0.0", feature = "rust1")] + #[unstable(feature = "thread_local_internals")] pub os: OsStaticKey, } @@ -528,7 +530,7 @@ mod imp { } #[doc(hidden)] - #[stable(feature = "rust1", since = "1.0.0")] + #[unstable(feature = "thread_local_internals")] pub unsafe extern fn destroy_value(ptr: *mut u8) { // The OS TLS ensures that this key contains a NULL value when this // destructor starts to run. We set it back to a sentinel value of 1 to diff --git a/src/libstd/thread_local/scoped.rs b/src/libstd/thread_local/scoped.rs index d89d69e949716..a5339568e9ef6 100644 --- a/src/libstd/thread_local/scoped.rs +++ b/src/libstd/thread_local/scoped.rs @@ -65,6 +65,7 @@ pub struct Key { #[doc(hidden)] pub inner: __impl::KeyInner } /// This macro declares a `static` item on which methods are used to get and /// set the value stored within. #[macro_export] +#[allow_internal_unstable] macro_rules! scoped_thread_local { (static $name:ident: $t:ty) => ( __scoped_thread_local_inner!(static $name: $t); @@ -76,6 +77,7 @@ macro_rules! scoped_thread_local { #[macro_export] #[doc(hidden)] +#[allow_internal_unstable] macro_rules! __scoped_thread_local_inner { (static $name:ident: $t:ty) => ( #[cfg_attr(not(any(windows, diff --git a/src/test/compile-fail/internal-unstable-thread-local.rs b/src/test/compile-fail/internal-unstable-thread-local.rs new file mode 100644 index 0000000000000..ff1584975462f --- /dev/null +++ b/src/test/compile-fail/internal-unstable-thread-local.rs @@ -0,0 +1,23 @@ +// 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. + +// aux-build:internal_unstable.rs + +#![feature(rustc_attrs)] +#![allow(dead_code)] + +extern crate internal_unstable; + + +thread_local!(static FOO: () = ()); +thread_local!(static BAR: () = internal_unstable::unstable()); //~ WARN use of unstable + +#[rustc_error] +fn main() {} //~ ERROR From 10426f69dac38186c7bb8946743023729d65e1e6 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Wed, 4 Mar 2015 18:10:54 +1100 Subject: [PATCH 3/4] Add more debugging to syntax::feature_gate. --- src/libsyntax/feature_gate.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 3b2d86cffe22c..bc955259ab5e0 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -285,7 +285,7 @@ pub const KNOWN_ATTRIBUTES: &'static [(&'static str, AttributeType)] = &[ ("recursion_limit", CrateLevel), ]; -#[derive(PartialEq, Copy)] +#[derive(PartialEq, Copy, Debug)] pub enum AttributeType { /// Normal, builtin attribute that is consumed /// by the compiler before the unused_attribute check @@ -353,7 +353,9 @@ struct Context<'a> { impl<'a> Context<'a> { fn gate_feature(&self, feature: &str, span: Span, explain: &str) { - if !self.has_feature(feature) { + let has_feature = self.has_feature(feature); + debug!("gate_feature(feature = {:?}, span = {:?}); has? {}", feature, span, has_feature); + if !has_feature { emit_feature_err(self.span_handler, feature, span, explain); } } @@ -634,12 +636,14 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> { } fn visit_attribute(&mut self, attr: &ast::Attribute) { + debug!("visit_attribute(attr = {:?})", attr); let name = &*attr.name(); for &(n, ty) in KNOWN_ATTRIBUTES { if n == name { if let Gated(gate, desc) = ty { self.gate_feature(gate, attr.span, desc); } + debug!("visit_attribute: {:?} is known, {:?}", name, ty); return; } } From b5c6ab20b7d1f4d6b0cc31c2987b2e0f8ea43e0c Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Wed, 4 Mar 2015 18:05:38 +1100 Subject: [PATCH 4/4] Run feature-gating on the final AST passed to the compiler. This ensures we catch everything; previously, an unknown attribute inserted by #[cfg_attr(...)] in a macro expansion would not be detected. --- src/librustc_driver/driver.rs | 25 ++++++++++++++++--- src/libsyntax/feature_gate.rs | 14 +++++++---- ...-attr-unknown-attribute-macro-expansion.rs | 20 +++++++++++++++ 3 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 src/test/compile-fail/cfg-attr-unknown-attribute-macro-expansion.rs diff --git a/src/librustc_driver/driver.rs b/src/librustc_driver/driver.rs index 73682faf1a77b..6ec36886252f0 100644 --- a/src/librustc_driver/driver.rs +++ b/src/librustc_driver/driver.rs @@ -493,12 +493,16 @@ pub fn phase_2_configure_and_expand(sess: &Session, } ); - // Needs to go *after* expansion to be able to check the results of macro expansion. - time(time_passes, "complete gated feature checking", (), |_| { + // Needs to go *after* expansion to be able to check the results + // of macro expansion. This runs before #[cfg] to try to catch as + // much as possible (e.g. help the programmer avoid platform + // specific differences) + time(time_passes, "complete gated feature checking 1", (), |_| { let features = syntax::feature_gate::check_crate(sess.codemap(), - &sess.parse_sess.span_diagnostic, - &krate); + &sess.parse_sess.span_diagnostic, + &krate, + true); *sess.features.borrow_mut() = features; sess.abort_if_errors(); }); @@ -521,6 +525,19 @@ pub fn phase_2_configure_and_expand(sess: &Session, time(time_passes, "checking that all macro invocations are gone", &krate, |krate| syntax::ext::expand::check_for_macros(&sess.parse_sess, krate)); + // One final feature gating of the true AST that gets compiled + // later, to make sure we've got everything (e.g. configuration + // can insert new attributes via `cfg_attr`) + time(time_passes, "complete gated feature checking 2", (), |_| { + let features = + syntax::feature_gate::check_crate(sess.codemap(), + &sess.parse_sess.span_diagnostic, + &krate, + false); + *sess.features.borrow_mut() = features; + sess.abort_if_errors(); + }); + Some(krate) } diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index bc955259ab5e0..fcfc14604843b 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -349,6 +349,7 @@ struct Context<'a> { features: Vec<&'static str>, span_handler: &'a SpanHandler, cm: &'a CodeMap, + do_warnings: bool, } impl<'a> Context<'a> { @@ -361,7 +362,7 @@ impl<'a> Context<'a> { } fn warn_feature(&self, feature: &str, span: Span, explain: &str) { - if !self.has_feature(feature) { + if !self.has_feature(feature) && self.do_warnings { emit_feature_warn(self.span_handler, feature, span, explain); } } @@ -700,6 +701,7 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> { } fn check_crate_inner(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::Crate, + do_warnings: bool, check: F) -> Features where F: FnOnce(&mut Context, &ast::Crate) @@ -707,6 +709,7 @@ fn check_crate_inner(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::C let mut cx = Context { features: Vec::new(), span_handler: span_handler, + do_warnings: do_warnings, cm: cm, }; @@ -786,13 +789,14 @@ fn check_crate_inner(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::C pub fn check_crate_macros(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::Crate) -> Features { - check_crate_inner(cm, span_handler, krate, + check_crate_inner(cm, span_handler, krate, true, |ctx, krate| visit::walk_crate(&mut MacroVisitor { context: ctx }, krate)) } -pub fn check_crate(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::Crate) --> Features { - check_crate_inner(cm, span_handler, krate, +pub fn check_crate(cm: &CodeMap, span_handler: &SpanHandler, krate: &ast::Crate, + do_warnings: bool) -> Features +{ + check_crate_inner(cm, span_handler, krate, do_warnings, |ctx, krate| visit::walk_crate(&mut PostExpansionVisitor { context: ctx }, krate)) } diff --git a/src/test/compile-fail/cfg-attr-unknown-attribute-macro-expansion.rs b/src/test/compile-fail/cfg-attr-unknown-attribute-macro-expansion.rs new file mode 100644 index 0000000000000..afcb896b43c2a --- /dev/null +++ b/src/test/compile-fail/cfg-attr-unknown-attribute-macro-expansion.rs @@ -0,0 +1,20 @@ +// 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. + +macro_rules! foo { + () => { + #[cfg_attr(all(), unknown)] //~ ERROR `unknown` is currently unknown + fn foo() {} + } +} + +foo!(); + +fn main() {}