Fix #50865: ICE on impl-trait returning functions reaching private items

src/Cargo.lock

@@ -2237,6 +2237,7 @@ dependencies = [
 name = "rustc_privacy"
 version = "0.0.0"
 dependencies = [
+ "log 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc 0.0.0",
  "rustc_data_structures 0.0.0",
  "rustc_typeck 0.0.0",

src/librustc/ich/impls_ty.rs

@@ -1086,6 +1086,7 @@ impl_stable_hash_for!(enum traits::Reveal {
 });
 
 impl_stable_hash_for!(enum ::middle::privacy::AccessLevel {
+    ReachableFromImplTrait,
     Reachable,
     Exported,
     Public

src/librustc/middle/privacy.rs

@@ -21,6 +21,8 @@ use syntax::ast::NodeId;
 // Accessibility levels, sorted in ascending order
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)]
 pub enum AccessLevel {
+    // Superset of Reachable used to mark impl Trait items.
+    ReachableFromImplTrait,
     // Exported items + items participating in various kinds of public interfaces,
     // but not directly nameable. For example, if function `fn f() -> T {...}` is
     // public, then type `T` is reachable. Its values can be obtained by other crates
@@ -40,7 +42,7 @@ pub struct AccessLevels<Id = NodeId> {
 
 impl<Id: Hash + Eq> AccessLevels<Id> {
     pub fn is_reachable(&self, id: Id) -> bool {
-        self.map.contains_key(&id)
+        self.map.get(&id) >= Some(&AccessLevel::Reachable)
     }
     pub fn is_exported(&self, id: Id) -> bool {
         self.map.get(&id) >= Some(&AccessLevel::Exported)

src/librustc/middle/reachable.rs

@@ -446,6 +446,8 @@ fn reachable_set<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, crate_num: CrateNum) ->
     // Step 2: Mark all symbols that the symbols on the worklist touch.
     reachable_context.propagate();
 
+    debug!("Inline reachability shows: {:?}", reachable_context.reachable_symbols);
+
     // Return the set of reachable symbols.
     ReachableSet(Lrc::new(reachable_context.reachable_symbols))
 }

src/librustc_privacy/Cargo.toml

@@ -9,6 +9,7 @@ path = "lib.rs"
 crate-type = ["dylib"]
 
 [dependencies]
+log = { version = "0.4", features = ["release_max_level_info", "std"] }
 rustc = { path = "../librustc" }
 rustc_typeck = { path = "../librustc_typeck" }
 syntax = { path = "../libsyntax" }

src/librustc_privacy/lib.rs

@@ -18,6 +18,7 @@
 
 #![recursion_limit="256"]
 
+#[macro_use] extern crate log;
 #[macro_use] extern crate rustc;
 #[macro_use] extern crate syntax;
 extern crate rustc_typeck;
@@ -83,6 +84,7 @@ struct EmbargoVisitor<'a, 'tcx: 'a> {
 }
 
 struct ReachEverythingInTheInterfaceVisitor<'b, 'a: 'b, 'tcx: 'a> {
+    access_level: Option<AccessLevel>,
     item_def_id: DefId,
     ev: &'b mut EmbargoVisitor<'a, 'tcx>,
 }
@@ -133,6 +135,7 @@ impl<'a, 'tcx> EmbargoVisitor<'a, 'tcx> {
     fn reach<'b>(&'b mut self, item_id: ast::NodeId)
                  -> ReachEverythingInTheInterfaceVisitor<'b, 'a, 'tcx> {
         ReachEverythingInTheInterfaceVisitor {
+            access_level: self.prev_level.map(|l| l.min(AccessLevel::Reachable)),
             item_def_id: self.tcx.hir.local_def_id(item_id),
             ev: self,
         }
@@ -147,6 +150,7 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
     }
 
     fn visit_item(&mut self, item: &'tcx hir::Item) {
+        debug!("Walked item {:?}", item);
         let inherited_item_level = match item.node {
             // Impls inherit level from their types and traits
             hir::ItemKind::Impl(..) => {
@@ -157,12 +161,21 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
             hir::ItemKind::ForeignMod(..) => {
                 self.prev_level
             }
+            // Impl trait return types mark their parent function.
+            // It (and its children) are revisited if the change applies.
+            hir::ItemKind::Existential(ref ty_data) => {
+                if let Some(impl_trait_fn) = ty_data.impl_trait_fn {
+                    if let Some(node_id) = self.tcx.hir.as_local_node_id(impl_trait_fn) {
+                        self.update(node_id, Some(AccessLevel::ReachableFromImplTrait));
+                    }
+                }
+                if item.vis.node.is_pub() { self.prev_level } else { None }
+            }
             // Other `pub` items inherit levels from parents
             hir::ItemKind::Const(..) | hir::ItemKind::Enum(..) | hir::ItemKind::ExternCrate(..) |
             hir::ItemKind::GlobalAsm(..) | hir::ItemKind::Fn(..) | hir::ItemKind::Mod(..) |
             hir::ItemKind::Static(..) | hir::ItemKind::Struct(..) |
             hir::ItemKind::Trait(..) | hir::ItemKind::TraitAlias(..) |
-            hir::ItemKind::Existential(..) |
             hir::ItemKind::Ty(..) | hir::ItemKind::Union(..) | hir::ItemKind::Use(..) => {
                 if item.vis.node.is_pub() { self.prev_level } else { None }
             }
@@ -171,6 +184,8 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
         // Update level of the item itself
         let item_level = self.update(item.id, inherited_item_level);
 
+        debug!("Its privacy is believed to be: {:?}", item_level);
+
         // Update levels of nested things
         match item.node {
             hir::ItemKind::Enum(ref def, _) => {
@@ -227,6 +242,9 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
             hir::ItemKind::ExternCrate(..) => {}
         }
 
+        let orig_level = self.prev_level;
+        self.prev_level = item_level;
+
         // Mark all items in interfaces of reachable items as reachable
         match item.node {
             // The interface is empty
@@ -324,9 +342,6 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
             }
         }
 
-        let orig_level = self.prev_level;
-        self.prev_level = item_level;
-
         intravisit::walk_item(self, item);
 
         self.prev_level = orig_level;
@@ -462,7 +477,7 @@ impl<'b, 'a, 'tcx> ReachEverythingInTheInterfaceVisitor<'b, 'a, 'tcx> {
     fn check_trait_ref(&mut self, trait_ref: ty::TraitRef<'tcx>) {
         if let Some(node_id) = self.ev.tcx.hir.as_local_node_id(trait_ref.def_id) {
             let item = self.ev.tcx.hir.expect_item(node_id);
-            self.ev.update(item.id, Some(AccessLevel::Reachable));
+            self.ev.update(item.id, self.access_level);
         }
     }
 }
@@ -483,7 +498,7 @@ impl<'b, 'a, 'tcx> TypeVisitor<'tcx> for ReachEverythingInTheInterfaceVisitor<'b
 
         if let Some(def_id) = ty_def_id {
             if let Some(node_id) = self.ev.tcx.hir.as_local_node_id(def_id) {
-                self.ev.update(node_id, Some(AccessLevel::Reachable));
+                self.ev.update(node_id, self.access_level);
             }
         }
 
@@ -1737,6 +1752,8 @@ fn privacy_access_levels<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
     }
     visitor.update(ast::CRATE_NODE_ID, Some(AccessLevel::Public));
 
+    debug!("access levels after embargo: {:?}", &visitor.access_levels);
+
     {
         let mut visitor = ObsoleteVisiblePrivateTypesVisitor {
             tcx,
@@ -1766,6 +1783,8 @@ fn privacy_access_levels<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         krate.visit_all_item_likes(&mut DeepVisitor::new(&mut visitor));
     }
 
+    debug!("final access levels: {:?}", &visitor.access_levels);
+
     Lrc::new(visitor.access_levels)
 }
 

src/test/run-pass/issue-50865-private-impl-trait/auxiliary/lib.rs

@@ -0,0 +1,24 @@
+// Copyright 2018 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.
+
+#![crate_type = "lib"]
+
+pub fn bar<P>( // Error won't happen if "bar" is not generic
+    _baz: P,
+) {
+    hide_foo()();
+}
+
+fn hide_foo() -> impl Fn() { // Error won't happen if "iterate" hasn't impl Trait or has generics
+    foo
+}
+
+fn foo() { // Error won't happen if "foo" isn't used in "iterate" or has generics
+}

src/test/run-pass/issue-50865-private-impl-trait/main.rs

@@ -0,0 +1,25 @@
+// Copyright 2018 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.
+
+// aux-build:lib.rs
+
+// Regression test for #50865.
+// When using generics or specifying the type directly, this example
+// codegens `foo` internally. However, when using a private `impl Trait`
+// function which references another private item, `foo` (in this case)
+// wouldn't be codegenned until main.rs used `bar`, as with impl Trait
+// it is not cast to `fn()` automatically to satisfy e.g.
+// `fn foo() -> fn() { ... }`. 
+
+extern crate lib;
+
+fn main() {
+    lib::bar(()); // Error won't happen if bar is called from same crate
+}