Auto merge of #38689 - pnkfelix:dont-check-stability-on-private-items…

…, r=nikomatsakis

Dont check stability for items that are not pub to universe.

Dont check stability for items that are not pub to universe.

In other words, skip it for private and even `pub(restricted)` items, because stability checks are only relevant to things visible in other crates.

Fix #38412.

src/librustc/middle/stability.rs

@@ -18,7 +18,7 @@ use hir::map as hir_map;
 use lint;
 use hir::def::Def;
 use hir::def_id::{CrateNum, CRATE_DEF_INDEX, DefId, DefIndex, LOCAL_CRATE};
-use ty::TyCtxt;
+use ty::{self, TyCtxt};
 use middle::privacy::AccessLevels;
 use syntax::symbol::Symbol;
 use syntax_pos::{Span, DUMMY_SP};
@@ -432,6 +432,36 @@ struct Checker<'a, 'tcx: 'a> {
 }
 
 impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
+    // (See issue #38412)
+    fn skip_stability_check_due_to_privacy(self, def_id: DefId) -> bool {
+        let visibility = {
+            // Check if `def_id` is a trait method.
+            match self.sess.cstore.associated_item(def_id) {
+                Some(ty::AssociatedItem { container: ty::TraitContainer(trait_def_id), .. }) => {
+                    // Trait methods do not declare visibility (even
+                    // for visibility info in cstore). Use containing
+                    // trait instead, so methods of pub traits are
+                    // themselves considered pub.
+                    self.sess.cstore.visibility(trait_def_id)
+                }
+                _ => {
+                    // Otherwise, cstore info works directly.
+                    self.sess.cstore.visibility(def_id)
+                }
+            }
+        };
+
+        match visibility {
+            // must check stability for pub items.
+            ty::Visibility::Public => false,
+
+            // these are not visible outside crate; therefore
+            // stability markers are irrelevant, if even present.
+            ty::Visibility::Restricted(..) |
+            ty::Visibility::Invisible => true,
+        }
+    }
+
     pub fn check_stability(self, def_id: DefId, id: NodeId, span: Span) {
         if self.sess.codemap().span_allows_unstable(span) {
             debug!("stability: \
@@ -492,6 +522,11 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             self.stability.borrow_mut().used_features.insert(feature.clone(), level.clone());
         }
 
+        // Issue 38412: private items lack stability markers.
+        if self.skip_stability_check_due_to_privacy(def_id) {
+            return
+        }
+
         match stability {
             Some(&Stability { level: attr::Unstable {ref reason, issue}, ref feature, .. }) => {
                 if !self.stability.borrow().active_features.contains(feature) {

src/test/compile-fail-fulldeps/auxiliary/pub_and_stability.rs

@@ -0,0 +1,144 @@
+// Copyright 2017 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.
+
+// This crate attempts to enumerate the various scenarios for how a
+// type can define fields and methods with various visiblities and
+// stabilities.
+//
+// The basic stability pattern in this file has four cases:
+// 1. no stability attribute at all
+// 2. a stable attribute (feature "unit_test")
+// 3. an unstable attribute that unit test declares (feature "unstable_declared")
+// 4. an unstable attribute that unit test fails to declare (feature "unstable_undeclared")
+//
+// This file also covers four kinds of visibility: private,
+// pub(module), pub(crate), and pub.
+//
+// However, since stability attributes can only be observed in
+// cross-crate linkage scenarios, there is little reason to take the
+// cross-product (4 stability cases * 4 visiblity cases), because the
+// first three visibility cases cannot be accessed outside this crate,
+// and therefore stability is only relevant when the visibility is pub
+// to the whole universe.
+//
+// (The only reason to do so would be if one were worried about the
+// compiler having some subtle bug where adding a stability attribute
+// introduces a privacy violation. As a way to provide evidence that
+// this is not occurring, I have put stability attributes on some
+// non-pub fields, marked with SILLY below)
+
+#![feature(staged_api)]
+#![feature(pub_restricted)]
+
+#![stable(feature = "unit_test", since = "0.0.0")]
+
+#[stable(feature = "unit_test", since = "0.0.0")]
+pub use m::{Record, Trait, Tuple};
+
+mod m {
+    #[derive(Default)]
+    #[stable(feature = "unit_test", since = "0.0.0")]
+    pub struct Record {
+        #[stable(feature = "unit_test", since = "0.0.0")]
+        pub a_stable_pub: i32,
+        #[unstable(feature = "unstable_declared", issue = "38412")]
+        pub a_unstable_declared_pub: i32,
+        #[unstable(feature = "unstable_undeclared", issue = "38412")]
+        pub a_unstable_undeclared_pub: i32,
+        #[unstable(feature = "unstable_undeclared", issue = "38412")] // SILLY
+        pub(crate) b_crate: i32,
+        #[unstable(feature = "unstable_declared", issue = "38412")] // SILLY
+        pub(m) c_mod: i32,
+        #[stable(feature = "unit_test", since = "0.0.0")] // SILLY
+        d_priv: i32
+    }
+
+    #[derive(Default)]
+    #[stable(feature = "unit_test", since = "1.0.0")]
+    pub struct Tuple(
+        #[stable(feature = "unit_test", since = "0.0.0")]
+        pub i32,
+        #[unstable(feature = "unstable_declared", issue = "38412")]
+        pub i32,
+        #[unstable(feature = "unstable_undeclared", issue = "38412")]
+        pub i32,
+
+        pub(crate) i32,
+        pub(m) i32,
+        i32);
+
+    impl Record {
+        #[stable(feature = "unit_test", since = "1.0.0")]
+        pub fn new() -> Self { Default::default() }
+    }
+
+    impl Tuple {
+        #[stable(feature = "unit_test", since = "1.0.0")]
+        pub fn new() -> Self { Default::default() }
+    }
+
+
+    #[stable(feature = "unit_test", since = "0.0.0")]
+    pub trait Trait {
+        #[stable(feature = "unit_test", since = "0.0.0")]
+        type Type;
+        #[stable(feature = "unit_test", since = "0.0.0")]
+        fn stable_trait_method(&self) -> Self::Type;
+        #[unstable(feature = "unstable_undeclared", issue = "38412")]
+        fn unstable_undeclared_trait_method(&self) -> Self::Type;
+        #[unstable(feature = "unstable_declared", issue = "38412")]
+        fn unstable_declared_trait_method(&self) -> Self::Type;
+    }
+
+    #[stable(feature = "unit_test", since = "0.0.0")]
+    impl Trait for Record {
+        type Type = i32;
+        fn stable_trait_method(&self) -> i32 { self.d_priv }
+        fn unstable_undeclared_trait_method(&self) -> i32 { self.d_priv }
+        fn unstable_declared_trait_method(&self) -> i32 { self.d_priv }
+    }
+
+    #[stable(feature = "unit_test", since = "0.0.0")]
+    impl Trait for Tuple {
+        type Type = i32;
+        fn stable_trait_method(&self) -> i32 { self.3 }
+        fn unstable_undeclared_trait_method(&self) -> i32 { self.3 }
+        fn unstable_declared_trait_method(&self) -> i32 { self.3 }
+    }
+
+    impl Record {
+        #[unstable(feature = "unstable_undeclared", issue = "38412")]
+        pub fn unstable_undeclared(&self) -> i32 { self.d_priv }
+        #[unstable(feature = "unstable_declared", issue = "38412")]
+        pub fn unstable_declared(&self) -> i32 { self.d_priv }
+        #[stable(feature = "unit_test", since = "0.0.0")]
+        pub fn stable(&self) -> i32 { self.d_priv }
+
+        #[unstable(feature = "unstable_undeclared", issue = "38412")] // SILLY
+        pub(crate) fn pub_crate(&self) -> i32 { self.d_priv }
+        #[unstable(feature = "unstable_declared", issue = "38412")] // SILLY
+        pub(m) fn pub_mod(&self) -> i32 { self.d_priv }
+        #[stable(feature = "unit_test", since = "0.0.0")] // SILLY
+        fn private(&self) -> i32 { self.d_priv }
+    }
+
+    impl Tuple {
+        #[unstable(feature = "unstable_undeclared", issue = "38412")]
+        pub fn unstable_undeclared(&self) -> i32 { self.0 }
+        #[unstable(feature = "unstable_declared", issue = "38412")]
+        pub fn unstable_declared(&self) -> i32 { self.0 }
+        #[stable(feature = "unit_test", since = "0.0.0")]
+        pub fn stable(&self) -> i32 { self.0 }
+
+        pub(crate) fn pub_crate(&self) -> i32 { self.0 }
+        pub(m) fn pub_mod(&self) -> i32 { self.0 }
+        fn private(&self) -> i32 { self.0 }
+    }
+}

src/test/compile-fail-fulldeps/explore-issue-38412.rs

@@ -0,0 +1,85 @@
+// Copyright 2014 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:pub_and_stability.rs
+
+#![feature(staged_api)]
+#![feature(unused_feature)]
+
+// A big point of this test is that we *declare* `unstable_declared`,
+// but do *not* declare `unstable_undeclared`. This way we can check
+// that the compiler is letting in uses of declared feature-gated
+// stuff but still rejecting uses of undeclared feature-gated stuff.
+#![feature(unstable_declared)]
+
+extern crate pub_and_stability;
+use pub_and_stability::{Record, Trait, Tuple};
+
+fn main() {
+    // Okay
+    let Record { .. } = Record::new();
+    // Okay (for now; see RFC Issue #902)
+    let Tuple(..) = Tuple::new();
+
+    // Okay
+    let Record { a_stable_pub: _, a_unstable_declared_pub: _, .. } = Record::new();
+    // Okay (for now; see RFC Issue #902)
+    let Tuple(_, _, ..) = Tuple::new(); // analogous to above
+
+    let Record { a_stable_pub: _, a_unstable_declared_pub: _, a_unstable_undeclared_pub: _, .. } =
+        Record::new();
+    //~^^ ERROR use of unstable library feature 'unstable_undeclared'
+
+    let Tuple(_, _, _, ..) = Tuple::new(); // analogous to previous
+    //~^ ERROR use of unstable library feature 'unstable_undeclared'
+
+    let r = Record::new();
+    let t = Tuple::new();
+
+    r.a_stable_pub;
+    r.a_unstable_declared_pub;
+    r.a_unstable_undeclared_pub; //~ ERROR use of unstable library feature
+    r.b_crate;                   //~ ERROR is private
+    r.c_mod;                     //~ ERROR is private
+    r.d_priv;                    //~ ERROR is private
+
+    t.0;
+    t.1;
+    t.2;                         //~ ERROR use of unstable library feature
+    t.3;                         //~ ERROR is private
+    t.4;                         //~ ERROR is private
+    t.5;                         //~ ERROR is private
+
+    r.stable_trait_method();
+    r.unstable_declared_trait_method();
+    r.unstable_undeclared_trait_method(); //~ ERROR use of unstable library feature
+
+    r.stable();
+    r.unstable_declared();
+    r.unstable_undeclared();              //~ ERROR use of unstable library feature
+
+    r.pub_crate();                        //~ ERROR `pub_crate` is private
+    r.pub_mod();                          //~ ERROR `pub_mod` is private
+    r.private();                          //~ ERROR `private` is private
+
+    let t = Tuple::new();
+    t.stable_trait_method();
+    t.unstable_declared_trait_method();
+    t.unstable_undeclared_trait_method(); //~ ERROR use of unstable library feature
+
+    t.stable();
+    t.unstable_declared();
+    t.unstable_undeclared();              //~ ERROR use of unstable library feature
+
+    t.pub_crate();                        //~ ERROR `pub_crate` is private
+    t.pub_mod();                          //~ ERROR `pub_mod` is private
+    t.private();                          //~ ERROR `private` is private
+
+}

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

@@ -0,0 +1,20 @@
+// Copyright 2016 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.
+
+fn main() {
+    let Box(a) = loop { };
+    //~^ ERROR field `0` of struct `std::boxed::Box` is private
+
+    // (The below is a trick to allow compiler to infer a type for
+    // variable `a` without attempting to ascribe a type to the
+    // pattern or otherwise attempting to name the Box type, which
+    // would run afoul of issue #22207)
+    let _b: *mut i32 = *a;
+}