Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

:vis matcher for macro_rules #41012

Merged
merged 10 commits into from
Apr 17, 2017
1 change: 1 addition & 0 deletions src/doc/unstable-book/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@
- [lookup_host](lookup-host.md)
- [loop_break_value](loop-break-value.md)
- [macro_reexport](macro-reexport.md)
- [macro_vis_matcher](macro-vis-matcher.md)
- [main](main.md)
- [manually_drop](manually-drop.md)
- [map_entry_recover_keys](map-entry-recover-keys.md)
Expand Down
14 changes: 14 additions & 0 deletions src/doc/unstable-book/src/macro-vis-matcher.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# `macro_vis_matcher`

The tracking issue for this feature is: [#41022]

With this feature gate enabled, the [list of fragment specifiers][frags] gains one more entry:

* `vis`: a visibility qualifier. Examples: nothing (default visibility); `pub`; `pub(crate)`.

A `vis` variable may be followed by a comma, ident, type, or path.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm feeling dense: Why can a vis variable be followed by a comma?

Copy link
Contributor Author

@durka durka Apr 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To quote @DanielKeep's rationale from above (hidden comment):

I allowed comma because having some kind of sequence-delimiting token is useful. If you're passing bits of parsed input around, you might have inner!($thing, $a_vis, $more_tts).

This isn't required, we can exclude it and you'd have to use ($a_vis) to pass around parsed visibility fragments within macros.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems harmless to me. I can't imagine using , as an "operator" in a visibility specifier (at last outside of parens or something).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pnkfelix are you still worried about this? Should we exclude it in order to get this PR into FCP? I just don't want this to continue languishing for too long...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no its okay; thanks for your patience.


[#41022]: https://github.com/rust-lang/rust/issues/41022
[frags]: ../book/first-edition/macros.html#syntactic-requirements

------------------------
4 changes: 3 additions & 1 deletion src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,9 @@ impl<'a> Resolver<'a> {
LoadedMacro::ProcMacro(ext) => return ext,
};

let ext = Rc::new(macro_rules::compile(&self.session.parse_sess, &macro_def));
let ext = Rc::new(macro_rules::compile(&self.session.parse_sess,
&self.session.features,
&macro_def));
self.macro_map.insert(def_id, ext.clone());
ext
}
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,9 @@ impl<'a> Resolver<'a> {
}

let def_id = self.definitions.local_def_id(item.id);
let ext = Rc::new(macro_rules::compile(&self.session.parse_sess, item));
let ext = Rc::new(macro_rules::compile(&self.session.parse_sess,
&self.session.features,
item));
self.macro_map.insert(def_id, ext);
*legacy_scope = LegacyScope::Binding(self.arenas.alloc_legacy_binding(LegacyBinding {
parent: Cell::new(*legacy_scope), name: ident.name, def_id: def_id, span: item.span,
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/ext/tt/macro_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,7 @@ fn parse_nt<'a>(p: &mut Parser<'a>, sp: Span, name: &str) -> Nonterminal {
token::NtPath(panictry!(p.parse_path(PathStyle::Type)))
},
"meta" => token::NtMeta(panictry!(p.parse_meta_item())),
"vis" => token::NtVis(panictry!(p.parse_visibility(true))),
// this is not supposed to happen, since it has been checked
// when compiling the macro.
_ => p.span_bug(sp, "invalid fragment specifier")
Expand Down
79 changes: 58 additions & 21 deletions src/libsyntax/ext/tt/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ use ext::tt::macro_parser::{MatchedSeq, MatchedNonterminal};
use ext::tt::macro_parser::{parse, parse_failure_msg};
use ext::tt::quoted;
use ext::tt::transcribe::transcribe;
use feature_gate::{self, emit_feature_err, Features, GateIssue};
use parse::{Directory, ParseSess};
use parse::parser::Parser;
use parse::token::{self, NtTT};
use parse::token::Token::*;
use symbol::Symbol;
use tokenstream::{TokenStream, TokenTree};

use std::cell::RefCell;
use std::collections::{HashMap};
use std::collections::hash_map::{Entry};
use std::rc::Rc;
Expand Down Expand Up @@ -154,7 +156,7 @@ fn generic_extension<'cx>(cx: &'cx ExtCtxt,
// Holy self-referential!

/// Converts a `macro_rules!` invocation into a syntax extension.
pub fn compile(sess: &ParseSess, def: &ast::Item) -> SyntaxExtension {
pub fn compile(sess: &ParseSess, features: &RefCell<Features>, def: &ast::Item) -> SyntaxExtension {
let lhs_nm = ast::Ident::with_empty_ctxt(Symbol::gensym("lhs"));
let rhs_nm = ast::Ident::with_empty_ctxt(Symbol::gensym("rhs"));

Expand Down Expand Up @@ -208,7 +210,7 @@ pub fn compile(sess: &ParseSess, def: &ast::Item) -> SyntaxExtension {
if let MatchedNonterminal(ref nt) = **m {
if let NtTT(ref tt) = **nt {
let tt = quoted::parse(tt.clone().into(), true, sess).pop().unwrap();
valid &= check_lhs_nt_follows(sess, &tt);
valid &= check_lhs_nt_follows(sess, features, &tt);
return tt;
}
}
Expand Down Expand Up @@ -251,11 +253,13 @@ pub fn compile(sess: &ParseSess, def: &ast::Item) -> SyntaxExtension {
NormalTT(exp, Some(def.span), attr::contains_name(&def.attrs, "allow_internal_unstable"))
}

fn check_lhs_nt_follows(sess: &ParseSess, lhs: &quoted::TokenTree) -> bool {
fn check_lhs_nt_follows(sess: &ParseSess,
features: &RefCell<Features>,
lhs: &quoted::TokenTree) -> bool {
// lhs is going to be like TokenTree::Delimited(...), where the
// entire lhs is those tts. Or, it can be a "bare sequence", not wrapped in parens.
match lhs {
&quoted::TokenTree::Delimited(_, ref tts) => check_matcher(sess, &tts.tts),
&quoted::TokenTree::Delimited(_, ref tts) => check_matcher(sess, features, &tts.tts),
_ => {
let msg = "invalid macro matcher; matchers must be contained in balanced delimiters";
sess.span_diagnostic.span_err(lhs.span(), msg);
Expand Down Expand Up @@ -307,11 +311,13 @@ fn check_rhs(sess: &ParseSess, rhs: &quoted::TokenTree) -> bool {
false
}

fn check_matcher(sess: &ParseSess, matcher: &[quoted::TokenTree]) -> bool {
fn check_matcher(sess: &ParseSess,
features: &RefCell<Features>,
matcher: &[quoted::TokenTree]) -> bool {
let first_sets = FirstSets::new(matcher);
let empty_suffix = TokenSet::empty();
let err = sess.span_diagnostic.err_count();
check_matcher_core(sess, &first_sets, matcher, &empty_suffix);
check_matcher_core(sess, features, &first_sets, matcher, &empty_suffix);
err == sess.span_diagnostic.err_count()
}

Expand Down Expand Up @@ -553,6 +559,7 @@ impl TokenSet {
// Requires that `first_sets` is pre-computed for `matcher`;
// see `FirstSets::new`.
fn check_matcher_core(sess: &ParseSess,
features: &RefCell<Features>,
first_sets: &FirstSets,
matcher: &[quoted::TokenTree],
follow: &TokenSet) -> TokenSet {
Expand Down Expand Up @@ -583,12 +590,11 @@ fn check_matcher_core(sess: &ParseSess,
match *token {
TokenTree::Token(..) | TokenTree::MetaVarDecl(..) => {
let can_be_followed_by_any;
if let Err(bad_frag) = has_legal_fragment_specifier(token) {
if let Err(bad_frag) = has_legal_fragment_specifier(sess, features, token) {
let msg = format!("invalid fragment specifier `{}`", bad_frag);
sess.span_diagnostic.struct_span_err(token.span(), &msg)
.help("valid fragment specifiers are `ident`, `block`, \
`stmt`, `expr`, `pat`, `ty`, `path`, `meta`, `tt` \
and `item`")
.help("valid fragment specifiers are `ident`, `block`, `stmt`, `expr`, \
`pat`, `ty`, `path`, `meta`, `tt`, `item` and `vis`")
.emit();
// (This eliminates false positives and duplicates
// from error messages.)
Expand All @@ -610,7 +616,7 @@ fn check_matcher_core(sess: &ParseSess,
}
TokenTree::Delimited(span, ref d) => {
let my_suffix = TokenSet::singleton(d.close_tt(span));
check_matcher_core(sess, first_sets, &d.tts, &my_suffix);
check_matcher_core(sess, features, first_sets, &d.tts, &my_suffix);
// don't track non NT tokens
last.replace_with_irrelevant();

Expand Down Expand Up @@ -642,7 +648,7 @@ fn check_matcher_core(sess: &ParseSess,
// At this point, `suffix_first` is built, and
// `my_suffix` is some TokenSet that we can use
// for checking the interior of `seq_rep`.
let next = check_matcher_core(sess, first_sets, &seq_rep.tts, my_suffix);
let next = check_matcher_core(sess, features, first_sets, &seq_rep.tts, my_suffix);
if next.maybe_empty {
last.add_all(&next);
} else {
Expand Down Expand Up @@ -790,30 +796,61 @@ fn is_in_follow(tok: &quoted::TokenTree, frag: &str) -> Result<bool, (String, &'
// harmless
Ok(true)
},
"vis" => {
// Explicitly disallow `priv`, on the off chance it comes back.
match *tok {
TokenTree::Token(_, ref tok) => match *tok {
Comma => Ok(true),
Ident(i) if i.name != "priv" => Ok(true),
ref tok => Ok(tok.can_begin_type())
},
TokenTree::MetaVarDecl(_, _, frag) if frag.name == "ident"
|| frag.name == "ty"
|| frag.name == "path" => Ok(true),
_ => Ok(false)
}
},
"" => Ok(true), // keywords::Invalid
_ => Err((format!("invalid fragment specifier `{}`", frag),
"valid fragment specifiers are `ident`, `block`, \
`stmt`, `expr`, `pat`, `ty`, `path`, `meta`, `tt` \
and `item`"))
`stmt`, `expr`, `pat`, `ty`, `path`, `meta`, `tt`, \
`item` and `vis`"))
}
}
}

fn has_legal_fragment_specifier(tok: &quoted::TokenTree) -> Result<(), String> {
fn has_legal_fragment_specifier(sess: &ParseSess,
features: &RefCell<Features>,
tok: &quoted::TokenTree) -> Result<(), String> {
debug!("has_legal_fragment_specifier({:?})", tok);
if let quoted::TokenTree::MetaVarDecl(_, _, frag_spec) = *tok {
let s = &frag_spec.name.as_str();
if !is_legal_fragment_specifier(s) {
return Err(s.to_string());
if let quoted::TokenTree::MetaVarDecl(_, _, ref frag_spec) = *tok {
let frag_name = frag_spec.name.as_str();
let frag_span = tok.span();
if !is_legal_fragment_specifier(sess, features, &frag_name, frag_span) {
return Err(frag_name.to_string());
}
}
Ok(())
}

fn is_legal_fragment_specifier(frag: &str) -> bool {
match frag {
fn is_legal_fragment_specifier(sess: &ParseSess,
features: &RefCell<Features>,
frag_name: &str,
frag_span: Span) -> bool {
match frag_name {
"item" | "block" | "stmt" | "expr" | "pat" |
"path" | "ty" | "ident" | "meta" | "tt" | "" => true,
"vis" => {
if !features.borrow().macro_vis_matcher {
let explain = feature_gate::EXPLAIN_VIS_MATCHER;
emit_feature_err(sess,
"macro_vis_matcher",
frag_span,
GateIssue::Language,
explain);
}
true
},
_ => false,
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,9 @@ declare_features! (

// Allows overlapping impls of marker traits
(active, overlapping_marker_traits, "1.18.0", Some(29864)),

// Allows use of the :vis macro fragment specifier
(active, macro_vis_matcher, "1.18.0", Some(41022)),
);

declare_features! (
Expand Down Expand Up @@ -1012,6 +1015,9 @@ pub const EXPLAIN_DEPR_CUSTOM_DERIVE: &'static str =
pub const EXPLAIN_DERIVE_UNDERSCORE: &'static str =
"attributes of the form `#[derive_*]` are reserved for the compiler";

pub const EXPLAIN_VIS_MATCHER: &'static str =
":vis fragment specifier is experimental and subject to change";

pub const EXPLAIN_PLACEMENT_IN: &'static str =
"placement-in expression syntax is experimental and subject to change.";

Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,7 @@ pub fn noop_fold_interpolated<T: Folder>(nt: token::Nonterminal, fld: &mut T)
token::NtWhereClause(where_clause) =>
token::NtWhereClause(fld.fold_where_clause(where_clause)),
token::NtArg(arg) => token::NtArg(fld.fold_arg(arg)),
token::NtVis(vis) => token::NtVis(fld.fold_vis(vis)),
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5056,7 +5056,9 @@ impl<'a> Parser<'a> {
/// and `pub(super)` for `pub(in super)`. If the following element can't be a tuple (i.e. it's
/// a function definition, it's not a tuple struct field) and the contents within the parens
/// isn't valid, emit a proper diagnostic.
fn parse_visibility(&mut self, can_take_tuple: bool) -> PResult<'a, Visibility> {
pub fn parse_visibility(&mut self, can_take_tuple: bool) -> PResult<'a, Visibility> {
maybe_whole!(self, NtVis, |x| x);

if !self.eat_keyword(keywords::Pub) {
return Ok(Visibility::Inherited)
}
Expand Down
2 changes: 2 additions & 0 deletions src/libsyntax/parse/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,7 @@ pub enum Nonterminal {
/// Stuff inside brackets for attributes
NtMeta(ast::MetaItem),
NtPath(ast::Path),
NtVis(ast::Visibility),
NtTT(TokenTree),
// These are not exposed to macros, but are used by quasiquote.
NtArm(ast::Arm),
Expand Down Expand Up @@ -392,6 +393,7 @@ impl fmt::Debug for Nonterminal {
NtGenerics(..) => f.pad("NtGenerics(..)"),
NtWhereClause(..) => f.pad("NtWhereClause(..)"),
NtArg(..) => f.pad("NtArg(..)"),
NtVis(..) => f.pad("NtVis(..)"),
}
}
}
19 changes: 11 additions & 8 deletions src/libsyntax/print/pprust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ pub fn token_to_string(tok: &Token) -> String {
token::NtGenerics(ref e) => generics_to_string(&e),
token::NtWhereClause(ref e) => where_clause_to_string(&e),
token::NtArg(ref e) => arg_to_string(&e),
token::NtVis(ref e) => vis_to_string(&e),
}
}
}
Expand Down Expand Up @@ -373,6 +374,10 @@ pub fn ident_to_string(id: ast::Ident) -> String {
to_string(|s| s.print_ident(id))
}

pub fn vis_to_string(v: &ast::Visibility) -> String {
to_string(|s| s.print_visibility(v))
}

pub fn fun_to_string(decl: &ast::FnDecl,
unsafety: ast::Unsafety,
constness: ast::Constness,
Expand Down Expand Up @@ -427,13 +432,7 @@ pub fn mac_to_string(arg: &ast::Mac) -> String {
}

pub fn visibility_qualified(vis: &ast::Visibility, s: &str) -> String {
match *vis {
ast::Visibility::Public => format!("pub {}", s),
ast::Visibility::Crate(_) => format!("pub(crate) {}", s),
ast::Visibility::Restricted { ref path, .. } =>
format!("pub({}) {}", to_string(|s| s.print_path(path, false, 0, true)), s),
ast::Visibility::Inherited => s.to_string()
}
format!("{}{}", to_string(|s| s.print_visibility(vis)), s)
}

fn needs_parentheses(expr: &ast::Expr) -> bool {
Expand Down Expand Up @@ -1468,7 +1467,11 @@ impl<'a> State<'a> {
ast::Visibility::Crate(_) => self.word_nbsp("pub(crate)"),
ast::Visibility::Restricted { ref path, .. } => {
let path = to_string(|s| s.print_path(path, false, 0, true));
self.word_nbsp(&format!("pub({})", path))
if path == "self" || path == "super" {
self.word_nbsp(&format!("pub({})", path))
} else {
self.word_nbsp(&format!("pub(in {})", path))
}
}
ast::Visibility::Inherited => Ok(())
}
Expand Down
19 changes: 19 additions & 0 deletions src/test/compile-fail/feature-gate-macro-vis-matcher.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright 2017 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Test that the MSP430 interrupt ABI cannot be used when msp430_interrupt
// feature gate is not used.

macro_rules! m { ($v:vis) => {} }
//~^ ERROR :vis fragment specifier is experimental and subject to change

fn main() {
m!(pub);
}
Loading