Prohibit private variant reexports

Author: petrochenkov

src/librustc_resolve/build_reduced_graph.rs

@@ -390,9 +390,15 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                 let module = Module::new(parent_link, Some(def), false, is_public);
                 name_bindings.define_module(module.clone(), sp);
 
+                let variant_modifiers = if is_public {
+                    DefModifiers::empty()
+                } else {
+                    DefModifiers::PRIVATE_VARIANT
+                };
                 for variant in &(*enum_definition).variants {
                     let item_def_id = self.ast_map.local_def_id(item.id);
-                    self.build_reduced_graph_for_variant(variant, item_def_id, &module);
+                    self.build_reduced_graph_for_variant(variant, item_def_id,
+                                                         &module, variant_modifiers);
                 }
                 parent.clone()
             }
@@ -494,7 +500,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
     fn build_reduced_graph_for_variant(&mut self,
                                        variant: &Variant,
                                        item_id: DefId,
-                                       parent: &Rc<Module>) {
+                                       parent: &Rc<Module>,
+                                       variant_modifiers: DefModifiers) {
         let name = variant.node.name;
         let is_exported = if variant.node.data.is_struct() {
             // Not adding fields for variants as they are not accessed with a self receiver
@@ -512,12 +519,12 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                       self.ast_map.local_def_id(variant.node.data.id()),
                                       is_exported),
                            variant.span,
-                           DefModifiers::PUBLIC | DefModifiers::IMPORTABLE);
+                           DefModifiers::PUBLIC | DefModifiers::IMPORTABLE | variant_modifiers);
         child.define_type(DefVariant(item_id,
                                      self.ast_map.local_def_id(variant.node.data.id()),
                                      is_exported),
                           variant.span,
-                          DefModifiers::PUBLIC | DefModifiers::IMPORTABLE);
+                          DefModifiers::PUBLIC | DefModifiers::IMPORTABLE | variant_modifiers);
     }
 
     /// Constructs the reduced graph for one foreign item.

src/librustc_resolve/lib.rs

@@ -907,6 +907,9 @@ bitflags! {
     flags DefModifiers: u8 {
         const PUBLIC     = 1 << 0,
         const IMPORTABLE = 1 << 1,
+        // All variants are considered `PUBLIC`, but some of them live in private enums.
+        // We need to track them to prohibit reexports like `pub use PrivEnum::Variant`.
+        const PRIVATE_VARIANT = 1 << 2,
     }
 }
 
@@ -1007,6 +1010,11 @@ impl NameBinding {
         self.defined_with(DefModifiers::PUBLIC)
     }
 
+    fn is_reexportable(&self) -> bool {
+        self.defined_with(DefModifiers::PUBLIC) &&
+        !self.defined_with(DefModifiers::PRIVATE_VARIANT)
+    }
+
     fn def_and_lp(&self) -> (Def, LastPrivate) {
         let def = self.def().unwrap();
         (def, LastMod(if self.is_public() { AllPublic } else { DependsOn(def.def_id()) }))

src/librustc_resolve/resolve_imports.rs

@@ -443,7 +443,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                     debug!("(resolving single import) found value binding");
                     value_result = BoundResult(target_module.clone(),
                                                child_name_bindings.value_ns.clone());
-                    if directive.is_public && !child_name_bindings.value_ns.is_public() {
+                    if directive.is_public && !child_name_bindings.value_ns.is_reexportable() {
                         let msg = format!("`{}` is private, and cannot be reexported", source);
                         let note_msg = format!("Consider marking `{}` as `pub` in the imported \
                                                 module",
@@ -458,7 +458,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                     type_result = BoundResult(target_module.clone(),
                                               child_name_bindings.type_ns.clone());
                     if !pub_err && directive.is_public &&
-                       !child_name_bindings.type_ns.is_public() {
+                       !child_name_bindings.type_ns.is_reexportable() {
                         let msg = format!("`{}` is private, and cannot be reexported", source);
                         let note_msg = format!("Consider declaring module `{}` as a `pub mod`",
                                                source);

src/test/compile-fail/issue-17546.rs

@@ -14,7 +14,7 @@ use foo::NoResult; // Through a re-export
 mod foo {
     pub use self::MyEnum::NoResult;
 
-    enum MyEnum {
+    pub enum MyEnum {
         Result,
         NoResult
     }

src/test/compile-fail/private-variant-reexport.rs

@@ -0,0 +1,15 @@
+// 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 <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.
+
+pub use E::V; //~ERROR `V` is private, and cannot be reexported
+
+enum E { V }
+
+fn main() {}