From ae13a72ded3e9b42908a62cb23367a0ec337d7c9 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 29 Dec 2016 16:00:53 -0500 Subject: [PATCH 1/2] Dont check stability for items that are not pub to universe. Includes special case handling for trait methods. Fix #38412. --- src/librustc/middle/stability.rs | 37 +++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/src/librustc/middle/stability.rs b/src/librustc/middle/stability.rs index f45e86f2f4b96..e6c9d2c36d013 100644 --- a/src/librustc/middle/stability.rs +++ b/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) { From ab8e92514cc273477e63c089ea6f38b0adba5ddd Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 3 Jan 2017 09:54:38 -0500 Subject: [PATCH 2/2] Regression test and exploratory unit test. --- .../auxiliary/pub_and_stability.rs | 144 ++++++++++++++++++ .../explore-issue-38412.rs | 85 +++++++++++ src/test/compile-fail/issue-38412.rs | 20 +++ 3 files changed, 249 insertions(+) create mode 100644 src/test/compile-fail-fulldeps/auxiliary/pub_and_stability.rs create mode 100644 src/test/compile-fail-fulldeps/explore-issue-38412.rs create mode 100644 src/test/compile-fail/issue-38412.rs diff --git a/src/test/compile-fail-fulldeps/auxiliary/pub_and_stability.rs b/src/test/compile-fail-fulldeps/auxiliary/pub_and_stability.rs new file mode 100644 index 0000000000000..9dc4cf1252ec3 --- /dev/null +++ b/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 or the MIT license +// , 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 } + } +} diff --git a/src/test/compile-fail-fulldeps/explore-issue-38412.rs b/src/test/compile-fail-fulldeps/explore-issue-38412.rs new file mode 100644 index 0000000000000..aab92575321e3 --- /dev/null +++ b/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 or the MIT license +// , 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 + +} diff --git a/src/test/compile-fail/issue-38412.rs b/src/test/compile-fail/issue-38412.rs new file mode 100644 index 0000000000000..00305eb2bc04b --- /dev/null +++ b/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 or the MIT license +// , 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; +}