From 73307475f9bc405d3aebd6f3a5240b364746217d Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 24 Nov 2015 03:36:12 +0300 Subject: [PATCH] Prohibit private variant reexports --- src/librustc_resolve/build_reduced_graph.rs | 15 +++++++++++---- src/librustc_resolve/lib.rs | 8 ++++++++ src/librustc_resolve/resolve_imports.rs | 4 ++-- src/test/compile-fail/issue-17546.rs | 2 +- src/test/compile-fail/private-variant-reexport.rs | 15 +++++++++++++++ 5 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 src/test/compile-fail/private-variant-reexport.rs diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 6733279d22a..11d09fa3e9a 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/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) { + parent: &Rc, + 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. diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 46fc3f37f7b..41858d0f01b 100644 --- a/src/librustc_resolve/lib.rs +++ b/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()) })) diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 94c3ea073d2..c4296241633 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/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); diff --git a/src/test/compile-fail/issue-17546.rs b/src/test/compile-fail/issue-17546.rs index a0b7935550c..e640ba3f00f 100644 --- a/src/test/compile-fail/issue-17546.rs +++ b/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 } diff --git a/src/test/compile-fail/private-variant-reexport.rs b/src/test/compile-fail/private-variant-reexport.rs new file mode 100644 index 00000000000..f3854616799 --- /dev/null +++ b/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 or the MIT license +// , 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() {}