Skip to content

Commit

Permalink
Rollup merge of rust-lang#53545 - FelixMcFelix:fix-50865-beta, r=petr…
Browse files Browse the repository at this point in the history
…ochenkov

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

Adds a test case as suggested in rust-lang#50865, and implements @petrochenkov's suggestion. Fixes rust-lang#50865.

Impl-trait-returning functions are marked under a new (low) access level, which they propagate rather than `AccessLevels::Reachable`. `AccessLevels::is_reachable` returns false for such items (leaving stability analysis unaffected), these items may still be visible to the lints phase however.
  • Loading branch information
kennytm committed Aug 24, 2018
2 parents 62f29c4 + 85a05d1 commit a1ec2f7
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 7 deletions.
1 change: 1 addition & 0 deletions src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,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 @@ -434,6 +434,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 @@ -82,6 +82,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 @@ -132,6 +133,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 @@ -214,7 +216,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 @@ -226,6 +236,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
}

0 comments on commit a1ec2f7

Please sign in to comment.