Skip to content

Commit

Permalink
Auto merge of #30206 - petrochenkov:newdepr, r=brson
Browse files Browse the repository at this point in the history 
Closes #29935

The attributes `deprecated` and `rustc_deprecated` are completely independent in this implementation and it leads to some noticeable code duplication. Representing `deprecated` as
```
Stability {
    level: Stable { since: "" },
    feature: "",
    depr: Some(Deprecation),
}
```
or, contrariwise, splitting rustc_deprecation from stability makes most of the duplication go away.
I can do this refactoring, but before doing it I must be sure, that further divergence of `deprecated` and `rustc_deprecated` is certainly not a goal.

cc @llogiq
  • Loading branch information
@bors
bors committed Dec 16, 2015
2 parents 9ace0a4 + 67a9784 commit ac2c5ff
Show file tree
Hide file tree
Showing 26 changed files with 1,003 additions and 137 deletions.
2 changes: 2 additions & 0 deletions src/doc/reference.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2390,6 +2390,8 @@ The currently implemented features of the reference compiler are:
* - `stmt_expr_attributes` - Allows attributes on expressions and
non-item statements.

* - `deprecated` - Allows using the `#[deprecated]` attribute.

If a feature is promoted to a language feature, then all existing programs will
start to receive compilation warnings about `#![feature]` directives which enabled
the new feature (because the directive is no longer necessary). However, if a
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/middle/cstore.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ pub enum FoundAst<'ast> {
pub trait CrateStore<'tcx> : Any {
// item info
fn stability(&self, def: DefId) -> Option<attr::Stability>;
fn deprecation(&self, def: DefId) -> Option<attr::Deprecation>;
fn closure_kind(&self, tcx: &ty::ctxt<'tcx>, def_id: DefId)
-> ty::ClosureKind;
fn closure_ty(&self, tcx: &ty::ctxt<'tcx>, def_id: DefId)
Expand Down Expand Up @@ -292,6 +293,7 @@ pub struct DummyCrateStore;
impl<'tcx> CrateStore<'tcx> for DummyCrateStore {
// item info
fn stability(&self, def: DefId) -> Option<attr::Stability> { unimplemented!() }
fn deprecation(&self, def: DefId) -> Option<attr::Deprecation> { unimplemented!() }
fn closure_kind(&self, tcx: &ty::ctxt<'tcx>, def_id: DefId)
-> ty::ClosureKind { unimplemented!() }
fn closure_ty(&self, tcx: &ty::ctxt<'tcx>, def_id: DefId)
Expand Down
158 changes: 92 additions & 66 deletions src/librustc/middle/stability.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use syntax::codemap::{Span, DUMMY_SP};
use syntax::ast;
use syntax::ast::{NodeId, Attribute};
use syntax::feature_gate::{GateIssue, emit_feature_err};
use syntax::attr::{self, Stability, AttrMetaMethods};
use syntax::attr::{self, Stability, Deprecation, AttrMetaMethods};
use util::nodemap::{DefIdMap, FnvHashSet, FnvHashMap};

use rustc_front::hir;
Expand Down Expand Up @@ -61,7 +61,8 @@ enum AnnotationKind {
pub struct Index<'tcx> {
/// This is mostly a cache, except the stabilities of local items
/// are filled by the annotator.
map: DefIdMap<Option<&'tcx Stability>>,
stab_map: DefIdMap<Option<&'tcx Stability>>,
depr_map: DefIdMap<Option<Deprecation>>,

/// Maps for each crate whether it is part of the staged API.
staged_api: FnvHashMap<ast::CrateNum, bool>
Expand All @@ -71,7 +72,8 @@ pub struct Index<'tcx> {
struct Annotator<'a, 'tcx: 'a> {
tcx: &'a ty::ctxt<'tcx>,
index: &'a mut Index<'tcx>,
parent: Option<&'tcx Stability>,
parent_stab: Option<&'tcx Stability>,
parent_depr: Option<Deprecation>,
access_levels: &'a AccessLevels,
in_trait_impl: bool,
in_enum: bool,
Expand All @@ -86,31 +88,35 @@ impl<'a, 'tcx: 'a> Annotator<'a, 'tcx> {
{
if self.index.staged_api[&LOCAL_CRATE] && self.tcx.sess.features.borrow().staged_api {
debug!("annotate(id = {:?}, attrs = {:?})", id, attrs);
if let Some(..) = attr::find_deprecation(self.tcx.sess.diagnostic(), attrs, item_sp) {
self.tcx.sess.span_err(item_sp, "`#[deprecated]` cannot be used in staged api, \
use `#[rustc_deprecated]` instead");
}
if let Some(mut stab) = attr::find_stability(self.tcx.sess.diagnostic(),
attrs, item_sp) {
// Error if prohibited, or can't inherit anything from a container
if kind == AnnotationKind::Prohibited ||
(kind == AnnotationKind::Container &&
stab.level.is_stable() &&
stab.depr.is_none()) {
stab.rustc_depr.is_none()) {
self.tcx.sess.span_err(item_sp, "This stability annotation is useless");
}

debug!("annotate: found {:?}", stab);
// If parent is deprecated and we're not, inherit this by merging
// deprecated_since and its reason.
if let Some(parent_stab) = self.parent {
if parent_stab.depr.is_some() && stab.depr.is_none() {
stab.depr = parent_stab.depr.clone()
if let Some(parent_stab) = self.parent_stab {
if parent_stab.rustc_depr.is_some() && stab.rustc_depr.is_none() {
stab.rustc_depr = parent_stab.rustc_depr.clone()
}
}

let stab = self.tcx.intern_stability(stab);

// Check if deprecated_since < stable_since. If it is,
// this is *almost surely* an accident.
if let (&Some(attr::Deprecation {since: ref dep_since, ..}),
&attr::Stable {since: ref stab_since}) = (&stab.depr, &stab.level) {
if let (&Some(attr::RustcDeprecation {since: ref dep_since, ..}),
&attr::Stable {since: ref stab_since}) = (&stab.rustc_depr, &stab.level) {
// Explicit version of iter::order::lt to handle parse errors properly
for (dep_v, stab_v) in dep_since.split(".").zip(stab_since.split(".")) {
if let (Ok(dep_v), Ok(stab_v)) = (dep_v.parse::<u64>(), stab_v.parse()) {
Expand All @@ -134,20 +140,20 @@ impl<'a, 'tcx: 'a> Annotator<'a, 'tcx> {
}

let def_id = self.tcx.map.local_def_id(id);
self.index.map.insert(def_id, Some(stab));
self.index.stab_map.insert(def_id, Some(stab));

let parent = replace(&mut self.parent, Some(stab));
let orig_parent_stab = replace(&mut self.parent_stab, Some(stab));
visit_children(self);
self.parent = parent;
self.parent_stab = orig_parent_stab;
} else {
debug!("annotate: not found, parent = {:?}", self.parent);
debug!("annotate: not found, parent = {:?}", self.parent_stab);
let mut is_error = kind == AnnotationKind::Required &&
self.access_levels.is_reachable(id) &&
!self.tcx.sess.opts.test;
if let Some(stab) = self.parent {
if let Some(stab) = self.parent_stab {
if stab.level.is_unstable() {
let def_id = self.tcx.map.local_def_id(id);
self.index.map.insert(def_id, Some(stab));
self.index.stab_map.insert(def_id, Some(stab));
is_error = false;
}
}
Expand All @@ -167,7 +173,26 @@ impl<'a, 'tcx: 'a> Annotator<'a, 'tcx> {
outside of the standard library");
}
}
visit_children(self);

if let Some(depr) = attr::find_deprecation(self.tcx.sess.diagnostic(), attrs, item_sp) {
if kind == AnnotationKind::Prohibited {
self.tcx.sess.span_err(item_sp, "This deprecation annotation is useless");
}

// `Deprecation` is just two pointers, no need to intern it
let def_id = self.tcx.map.local_def_id(id);
self.index.depr_map.insert(def_id, Some(depr.clone()));

let orig_parent_depr = replace(&mut self.parent_depr, Some(depr));
visit_children(self);
self.parent_depr = orig_parent_depr;
} else if let Some(depr) = self.parent_depr.clone() {
let def_id = self.tcx.map.local_def_id(id);
self.index.depr_map.insert(def_id, Some(depr));
visit_children(self);
} else {
visit_children(self);
}
}
}
}
Expand Down Expand Up @@ -269,7 +294,8 @@ impl<'tcx> Index<'tcx> {
let mut annotator = Annotator {
tcx: tcx,
index: self,
parent: None,
parent_stab: None,
parent_depr: None,
access_levels: access_levels,
in_trait_impl: false,
in_enum: false,
Expand All @@ -291,7 +317,8 @@ impl<'tcx> Index<'tcx> {
staged_api.insert(LOCAL_CRATE, is_staged_api);
Index {
staged_api: staged_api,
map: DefIdMap(),
stab_map: DefIdMap(),
depr_map: DefIdMap(),
}
}
}
Expand Down Expand Up @@ -327,7 +354,11 @@ struct Checker<'a, 'tcx: 'a> {
}

impl<'a, 'tcx> Checker<'a, 'tcx> {
fn check(&mut self, id: DefId, span: Span, stab: &Option<&Stability>) {
fn check(&mut self, id: DefId, span: Span,
stab: &Option<&Stability>, _depr: &Option<Deprecation>) {
if !is_staged_api(self.tcx, id) {
return;
}
// Only the cross-crate scenario matters when checking unstable APIs
let cross_crate = !id.is_local();
if !cross_crate {
Expand Down Expand Up @@ -395,31 +426,31 @@ impl<'a, 'v, 'tcx> Visitor<'v> for Checker<'a, 'tcx> {
if item.span == DUMMY_SP && item.name.as_str() == "__test" { return }

check_item(self.tcx, item, true,
&mut |id, sp, stab| self.check(id, sp, stab));
&mut |id, sp, stab, depr| self.check(id, sp, stab, depr));
intravisit::walk_item(self, item);
}

fn visit_expr(&mut self, ex: &hir::Expr) {
check_expr(self.tcx, ex,
&mut |id, sp, stab| self.check(id, sp, stab));
&mut |id, sp, stab, depr| self.check(id, sp, stab, depr));
intravisit::walk_expr(self, ex);
}

fn visit_path(&mut self, path: &hir::Path, id: ast::NodeId) {
check_path(self.tcx, path, id,
&mut |id, sp, stab| self.check(id, sp, stab));
&mut |id, sp, stab, depr| self.check(id, sp, stab, depr));
intravisit::walk_path(self, path)
}

fn visit_path_list_item(&mut self, prefix: &hir::Path, item: &hir::PathListItem) {
check_path_list_item(self.tcx, item,
&mut |id, sp, stab| self.check(id, sp, stab));
&mut |id, sp, stab, depr| self.check(id, sp, stab, depr));
intravisit::walk_path_list_item(self, prefix, item)
}

fn visit_pat(&mut self, pat: &hir::Pat) {
check_pat(self.tcx, pat,
&mut |id, sp, stab| self.check(id, sp, stab));
&mut |id, sp, stab, depr| self.check(id, sp, stab, depr));
intravisit::walk_pat(self, pat)
}

Expand All @@ -441,7 +472,7 @@ impl<'a, 'v, 'tcx> Visitor<'v> for Checker<'a, 'tcx> {

/// Helper for discovering nodes to check for stability
pub fn check_item(tcx: &ty::ctxt, item: &hir::Item, warn_about_defns: bool,
cb: &mut FnMut(DefId, Span, &Option<&Stability>)) {
cb: &mut FnMut(DefId, Span, &Option<&Stability>, &Option<Deprecation>)) {
match item.node {
hir::ItemExternCrate(_) => {
// compiler-generated `extern crate` items have a dummy span.
Expand Down Expand Up @@ -478,7 +509,7 @@ pub fn check_item(tcx: &ty::ctxt, item: &hir::Item, warn_about_defns: bool,

/// Helper for discovering nodes to check for stability
pub fn check_expr(tcx: &ty::ctxt, e: &hir::Expr,
cb: &mut FnMut(DefId, Span, &Option<&Stability>)) {
cb: &mut FnMut(DefId, Span, &Option<&Stability>, &Option<Deprecation>)) {
let span;
let id = match e.node {
hir::ExprMethodCall(i, _, _) => {
Expand Down Expand Up @@ -539,7 +570,7 @@ pub fn check_expr(tcx: &ty::ctxt, e: &hir::Expr,
}

pub fn check_path(tcx: &ty::ctxt, path: &hir::Path, id: ast::NodeId,
cb: &mut FnMut(DefId, Span, &Option<&Stability>)) {
cb: &mut FnMut(DefId, Span, &Option<&Stability>, &Option<Deprecation>)) {
match tcx.def_map.borrow().get(&id).map(|d| d.full_def()) {
Some(def::DefPrimTy(..)) => {}
Some(def::DefSelfTy(..)) => {}
Expand All @@ -551,7 +582,7 @@ pub fn check_path(tcx: &ty::ctxt, path: &hir::Path, id: ast::NodeId,
}

pub fn check_path_list_item(tcx: &ty::ctxt, item: &hir::PathListItem,
cb: &mut FnMut(DefId, Span, &Option<&Stability>)) {
cb: &mut FnMut(DefId, Span, &Option<&Stability>, &Option<Deprecation>)) {
match tcx.def_map.borrow().get(&item.node.id()).map(|d| d.full_def()) {
Some(def::DefPrimTy(..)) => {}
Some(def) => {
Expand All @@ -562,7 +593,7 @@ pub fn check_path_list_item(tcx: &ty::ctxt, item: &hir::PathListItem,
}

pub fn check_pat(tcx: &ty::ctxt, pat: &hir::Pat,
cb: &mut FnMut(DefId, Span, &Option<&Stability>)) {
cb: &mut FnMut(DefId, Span, &Option<&Stability>, &Option<Deprecation>)) {
debug!("check_pat(pat = {:?})", pat);
if is_internal(tcx, pat.span) { return; }

Expand Down Expand Up @@ -591,21 +622,21 @@ pub fn check_pat(tcx: &ty::ctxt, pat: &hir::Pat,
}

fn maybe_do_stability_check(tcx: &ty::ctxt, id: DefId, span: Span,
cb: &mut FnMut(DefId, Span, &Option<&Stability>)) {
if !is_staged_api(tcx, id) {
debug!("maybe_do_stability_check: \
skipping id={:?} since it is not staged_api", id);
return;
}
cb: &mut FnMut(DefId, Span,
&Option<&Stability>, &Option<Deprecation>)) {
if is_internal(tcx, span) {
debug!("maybe_do_stability_check: \
skipping span={:?} since it is internal", span);
return;
}
let ref stability = lookup(tcx, id);
let (stability, deprecation) = if is_staged_api(tcx, id) {
(lookup_stability(tcx, id), None)
} else {
(None, lookup_deprecation(tcx, id))
};
debug!("maybe_do_stability_check: \
inspecting id={:?} span={:?} of stability={:?}", id, span, stability);
cb(id, span, stability);
cb(id, span, &stability, &deprecation);
}

fn is_internal(tcx: &ty::ctxt, span: Span) -> bool {
Expand All @@ -627,47 +658,42 @@ fn is_staged_api(tcx: &ty::ctxt, id: DefId) -> bool {

/// Lookup the stability for a node, loading external crate
/// metadata as necessary.
pub fn lookup<'tcx>(tcx: &ty::ctxt<'tcx>, id: DefId) -> Option<&'tcx Stability> {
if let Some(st) = tcx.stability.borrow().map.get(&id) {
pub fn lookup_stability<'tcx>(tcx: &ty::ctxt<'tcx>, id: DefId) -> Option<&'tcx Stability> {
if let Some(st) = tcx.stability.borrow().stab_map.get(&id) {
return *st;
}

let st = lookup_uncached(tcx, id);
tcx.stability.borrow_mut().map.insert(id, st);
let st = lookup_stability_uncached(tcx, id);
tcx.stability.borrow_mut().stab_map.insert(id, st);
st
}

fn lookup_uncached<'tcx>(tcx: &ty::ctxt<'tcx>, id: DefId) -> Option<&'tcx Stability> {
debug!("lookup(id={:?})", id);

// is this definition the implementation of a trait method?
match tcx.trait_item_of_item(id) {
Some(ty::MethodTraitItemId(trait_method_id)) if trait_method_id != id => {
debug!("lookup: trait_method_id={:?}", trait_method_id);
return lookup(tcx, trait_method_id)
}
_ => {}
pub fn lookup_deprecation<'tcx>(tcx: &ty::ctxt<'tcx>, id: DefId) -> Option<Deprecation> {
if let Some(depr) = tcx.stability.borrow().depr_map.get(&id) {
return depr.clone();
}

let item_stab = if id.is_local() {
let depr = lookup_deprecation_uncached(tcx, id);
tcx.stability.borrow_mut().depr_map.insert(id, depr.clone());
depr
}

fn lookup_stability_uncached<'tcx>(tcx: &ty::ctxt<'tcx>, id: DefId) -> Option<&'tcx Stability> {
debug!("lookup(id={:?})", id);
if id.is_local() {
None // The stability cache is filled partially lazily
} else {
tcx.sess.cstore.stability(id).map(|st| tcx.intern_stability(st))
};

item_stab.or_else(|| {
if tcx.is_impl(id) {
if let Some(trait_id) = tcx.trait_id_of_impl(id) {
// FIXME (#18969): for the time being, simply use the
// stability of the trait to determine the stability of any
// unmarked impls for it. See FIXME above for more details.
}
}

debug!("lookup: trait_id={:?}", trait_id);
return lookup(tcx, trait_id);
}
}
None
})
fn lookup_deprecation_uncached<'tcx>(tcx: &ty::ctxt<'tcx>, id: DefId) -> Option<Deprecation> {
debug!("lookup(id={:?})", id);
if id.is_local() {
None // The stability cache is filled partially lazily
} else {
tcx.sess.cstore.deprecation(id)
}
}

/// Given the list of enabled features that were not language features (i.e. that
Expand Down
Loading

0 comments on commit ac2c5ff

Please sign in to comment.