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

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

Merged
merged 7 commits into from
Aug 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1086,6 +1086,7 @@ impl_stable_hash_for!(enum traits::Reveal {
});

impl_stable_hash_for!(enum ::middle::privacy::AccessLevel {
ReachableFromImplTrait,
Reachable,
Exported,
Public
Expand Down
4 changes: 3 additions & 1 deletion src/librustc/middle/privacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/middle/reachable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
23 changes: 17 additions & 6 deletions src/librustc_privacy/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,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>,
}
Expand Down Expand Up @@ -133,6 +134,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,
}
Expand Down Expand Up @@ -215,7 +217,15 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
}
}
}
hir::ItemKind::Existential(..) |
// 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));
}
}
}
hir::ItemKind::Use(..) |
hir::ItemKind::Static(..) |
hir::ItemKind::Const(..) |
Expand All @@ -227,6 +237,10 @@ impl<'a, 'tcx> Visitor<'tcx> for EmbargoVisitor<'a, 'tcx> {
hir::ItemKind::ExternCrate(..) => {}
}

// Store this node's access level here to propagate the correct
// reachability level through interfaces and children.
let orig_level = replace(&mut self.prev_level, item_level);

// Mark all items in interfaces of reachable items as reachable
match item.node {
// The interface is empty
Expand Down Expand Up @@ -324,9 +338,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;
Expand Down Expand Up @@ -462,7 +473,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);
}
}
}
Expand All @@ -483,7 +494,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);
}
}

Expand Down
24 changes: 24 additions & 0 deletions src/test/run-pass/issue-50865-private-impl-trait/auxiliary/lib.rs
Original file line number Diff line number Diff line change
@@ -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
}
25 changes: 25 additions & 0 deletions src/test/run-pass/issue-50865-private-impl-trait/main.rs
Original file line number Diff line number Diff line change
@@ -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
}