Auto merge of rust-lang#38140 - jseyfried:proc_macro_visibility, r=nrc

Require `#[proc_macro_derive]` functions to be `pub` r? @nrc
alexcrichton · Dec 19, 2016 · 0102127 · 0102127
2 parents 71c06a5 + 1605254
commit 0102127
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 10 deletions.
diff --git a/src/libsyntax_ext/proc_macro_registrar.rs b/src/libsyntax_ext/proc_macro_registrar.rs
@@ -108,6 +108,8 @@ impl<'a> CollectCustomDerives<'a> {
 
 impl<'a> Visitor<'a> for CollectCustomDerives<'a> {
     fn visit_item(&mut self, item: &'a ast::Item) {
+        let mut attrs = item.attrs.iter().filter(|a| a.check_name("proc_macro_derive"));
+
         // First up, make sure we're checking a bare function. If we're not then
         // we're just not interested in this item.
         //
@@ -117,10 +119,7 @@ impl<'a> Visitor<'a> for CollectCustomDerives<'a> {
             ast::ItemKind::Fn(..) => {}
             _ => {
                 // Check for invalid use of proc_macro_derive
-                let attr = item.attrs.iter()
-                    .filter(|a| a.check_name("proc_macro_derive"))
-                    .next();
-                if let Some(attr) = attr {
+                if let Some(attr) = attrs.next() {
                     self.handler.span_err(attr.span(),
                                           "the `#[proc_macro_derive]` \
                                           attribute may only be used \
@@ -132,8 +131,6 @@ impl<'a> Visitor<'a> for CollectCustomDerives<'a> {
             }
         }
 
-        let mut attrs = item.attrs.iter()
-                            .filter(|a| a.check_name("proc_macro_derive"));
         let attr = match attrs.next() {
             Some(attr) => attr,
             None => {
@@ -227,16 +224,20 @@ impl<'a> Visitor<'a> for CollectCustomDerives<'a> {
             Vec::new()
         };
 
-        if self.in_root {
+        if self.in_root && item.vis == ast::Visibility::Public {
             self.derives.push(CustomDerive {
                 span: item.span,
                 trait_name: trait_name,
                 function_name: item.ident,
                 attrs: proc_attrs,
             });
         } else {
-            let msg = "functions tagged with `#[proc_macro_derive]` must \
-                       currently reside in the root of the crate";
+            let msg = if !self.in_root {
+                "functions tagged with `#[proc_macro_derive]` must \
+                 currently reside in the root of the crate"
+            } else {
+                "functions tagged with `#[proc_macro_derive]` must be `pub`"
+            };
             self.handler.span_err(item.span, msg);
         }
 

diff --git a/...e-fail-fulldeps/proc-macro/at-the-root.rs → ...-fulldeps/proc-macro/pub-at-crate-root.rs b/...e-fail-fulldeps/proc-macro/at-the-root.rs → ...-fulldeps/proc-macro/pub-at-crate-root.rs
@@ -23,3 +23,8 @@ pub mod a { //~ `proc-macro` crate types cannot export any items
     }
 }
 
+#[proc_macro_derive(B)]
+fn bar(a: proc_macro::TokenStream) -> proc_macro::TokenStream {
+//~^ ERROR: functions tagged with `#[proc_macro_derive]` must be `pub`
+    a
+}
diff --git a/src/test/compile-fail-fulldeps/proc-macro/signature.rs b/src/test/compile-fail-fulldeps/proc-macro/signature.rs
@@ -15,7 +15,7 @@
 extern crate proc_macro;
 
 #[proc_macro_derive(A)]
-unsafe extern fn foo(a: i32, b: u32) -> u32 {
+pub unsafe extern fn foo(a: i32, b: u32) -> u32 {
     //~^ ERROR: mismatched types
     //~| NOTE: expected normal fn, found unsafe fn
     //~| NOTE: expected type `fn(proc_macro::TokenStream) -> proc_macro::TokenStream`