Auto merge of #33204 - Manishearth:rollup, r=Manishearth

Rollup of 7 pull requests

- Successful merges: #33107, #33133, #33160, #33167, #33194, #33196, #33200
- Failed merges:

Author: bors

src/librustc_typeck/coherence/mod.rs

@@ -34,9 +34,7 @@ use CrateCtxt;
 use rustc::infer::{self, InferCtxt, TypeOrigin, new_infer_ctxt};
 use std::cell::RefCell;
 use std::rc::Rc;
-use syntax::ast;
 use syntax::codemap::Span;
-use syntax::errors::DiagnosticBuilder;
 use util::nodemap::{DefIdMap, FnvHashMap};
 use rustc::dep_graph::DepNode;
 use rustc::hir::map as hir_map;
@@ -517,13 +515,6 @@ fn enforce_trait_manually_implementable(tcx: &TyCtxt, sp: Span, trait_def_id: De
     err.emit();
 }
 
-// Factored out into helper because the error cannot be defined in multiple locations.
-pub fn report_duplicate_item<'tcx>(tcx: &TyCtxt<'tcx>, sp: Span, name: ast::Name)
-                                   -> DiagnosticBuilder<'tcx>
-{
-    struct_span_err!(tcx.sess, sp, E0201, "duplicate definitions with name `{}`:", name)
-}
-
 pub fn check_coherence(crate_context: &CrateCtxt) {
     let _task = crate_context.tcx.dep_graph.in_task(DepNode::Coherence);
     let infcx = new_infer_ctxt(crate_context.tcx,

src/librustc_typeck/collect.rs

@@ -63,7 +63,6 @@ use lint;
 use hir::def::Def;
 use hir::def_id::DefId;
 use constrained_type_params as ctp;
-use coherence;
 use middle::lang_items::SizedTraitLangItem;
 use middle::resolve_lifetime;
 use middle::const_val::ConstVal;
@@ -80,13 +79,14 @@ use rscope::*;
 use rustc::dep_graph::DepNode;
 use rustc::hir::map as hir_map;
 use util::common::{ErrorReported, MemoizationMap};
-use util::nodemap::{FnvHashMap, FnvHashSet};
+use util::nodemap::FnvHashMap;
 use write_ty_to_tcx;
 
 use rustc_const_math::ConstInt;
 
 use std::cell::RefCell;
 use std::collections::HashSet;
+use std::collections::hash_map::Entry::{Occupied, Vacant};
 use std::rc::Rc;
 
 use syntax::abi;
@@ -746,16 +746,27 @@ fn convert_item(ccx: &CrateCtxt, it: &hir::Item) {
 
             // Convert all the associated consts.
             // Also, check if there are any duplicate associated items
-            let mut seen_type_items = FnvHashSet();
-            let mut seen_value_items = FnvHashSet();
+            let mut seen_type_items = FnvHashMap();
+            let mut seen_value_items = FnvHashMap();
 
             for impl_item in impl_items {
                 let seen_items = match impl_item.node {
                     hir::ImplItemKind::Type(_) => &mut seen_type_items,
                     _                    => &mut seen_value_items,
                 };
-                if !seen_items.insert(impl_item.name) {
-                    coherence::report_duplicate_item(tcx, impl_item.span, impl_item.name).emit();
+                match seen_items.entry(impl_item.name) {
+                    Occupied(entry) => {
+                        let mut err = struct_span_err!(tcx.sess, impl_item.span, E0201,
+                                                       "duplicate definitions with name `{}`:",
+                                                       impl_item.name);
+                        span_note!(&mut err, *entry.get(),
+                                   "previous definition of `{}` here",
+                                   impl_item.name);
+                        err.emit();
+                    }
+                    Vacant(entry) => {
+                        entry.insert(impl_item.span);
+                    }
                 }
 
                 if let hir::ImplItemKind::Const(ref ty, _) = impl_item.node {

src/librustdoc/clean/inline.rs

@@ -76,6 +76,7 @@ fn try_inline_def(cx: &DocContext, tcx: &TyCtxt,
     let inner = match def {
         Def::Trait(did) => {
             record_extern_fqn(cx, did, clean::TypeTrait);
+            ret.extend(build_impls(cx, tcx, did));
             clean::TraitItem(build_external_trait(cx, tcx, did))
         }
         Def::Fn(did) => {
@@ -247,12 +248,10 @@ pub fn build_impls(cx: &DocContext,
     // Primarily, the impls will be used to populate the documentation for this
     // type being inlined, but impls can also be used when generating
     // documentation for primitives (no way to find those specifically).
-    if !cx.all_crate_impls.borrow_mut().contains_key(&did.krate) {
-        let mut impls = Vec::new();
+    if cx.populated_crate_impls.borrow_mut().insert(did.krate) {
         for item in tcx.sess.cstore.crate_top_level_items(did.krate) {
             populate_impls(cx, tcx, item.def, &mut impls);
         }
-        cx.all_crate_impls.borrow_mut().insert(did.krate, impls);
 
         fn populate_impls(cx: &DocContext, tcx: &TyCtxt,
                           def: cstore::DefLike,
@@ -269,21 +268,7 @@ pub fn build_impls(cx: &DocContext,
         }
     }
 
-    let mut candidates = cx.all_crate_impls.borrow_mut();
-    let candidates = candidates.get_mut(&did.krate).unwrap();
-    for i in (0..candidates.len()).rev() {
-        let remove = match candidates[i].inner {
-            clean::ImplItem(ref i) => {
-                i.for_.def_id() == Some(did) || i.for_.primitive_type().is_some()
-            }
-            _ => continue,
-        };
-        if remove {
-            impls.push(candidates.swap_remove(i));
-        }
-    }
-
-    return impls;
+    impls
 }
 
 pub fn build_impl(cx: &DocContext,

src/librustdoc/clean/mod.rs

@@ -38,7 +38,7 @@ use rustc_trans::back::link;
 use rustc::middle::cstore::{self, CrateStore};
 use rustc::middle::privacy::AccessLevels;
 use rustc::hir::def::Def;
-use rustc::hir::def_id::{DefId, DefIndex};
+use rustc::hir::def_id::{DefId, DefIndex, CRATE_DEF_INDEX};
 use rustc::ty::subst::{self, ParamSpace, VecPerParamSpace};
 use rustc::ty;
 use rustc::middle::stability;
@@ -2388,7 +2388,7 @@ impl Clean<Item> for doctree::ExternCrate {
             name: None,
             attrs: self.attrs.clean(cx),
             source: self.whence.clean(cx),
-            def_id: cx.map.local_def_id(0),
+            def_id: DefId { krate: self.cnum, index: CRATE_DEF_INDEX },
             visibility: self.vis.clean(cx),
             stability: None,
             deprecation: None,

src/librustdoc/core.rs

@@ -30,7 +30,7 @@ use syntax::feature_gate::UnstableFeatures;
 use syntax::parse::token;
 
 use std::cell::{RefCell, Cell};
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use std::rc::Rc;
 
 use visit_ast::RustdocVisitor;
@@ -54,7 +54,7 @@ pub struct DocContext<'a, 'tcx: 'a> {
     pub map: &'a hir_map::Map<'tcx>,
     pub maybe_typed: MaybeTyped<'a, 'tcx>,
     pub input: Input,
-    pub all_crate_impls: RefCell<HashMap<ast::CrateNum, Vec<clean::Item>>>,
+    pub populated_crate_impls: RefCell<HashSet<ast::CrateNum>>,
     pub deref_trait_did: Cell<Option<DefId>>,
     // Note that external items for which `doc(hidden)` applies to are shown as
     // non-reachable while local items aren't. This is because we're reusing
@@ -189,7 +189,7 @@ pub fn run_core(search_paths: SearchPaths,
             map: &tcx.map,
             maybe_typed: Typed(tcx),
             input: input,
-            all_crate_impls: RefCell::new(HashMap::new()),
+            populated_crate_impls: RefCell::new(HashSet::new()),
             deref_trait_did: Cell::new(None),
             access_levels: RefCell::new(access_levels),
             external_traits: RefCell::new(HashMap::new()),

src/librustdoc/doctree.rs

@@ -232,6 +232,7 @@ pub struct Macro {
 
 pub struct ExternCrate {
     pub name: Name,
+    pub cnum: ast::CrateNum,
     pub path: Option<String>,
     pub vis: hir::Visibility,
     pub attrs: hir::HirVec<ast::Attribute>,

src/librustdoc/html/format.rs

@@ -57,6 +57,11 @@ pub struct TyParamBounds<'a>(pub &'a [clean::TyParamBound]);
 pub struct CommaSep<'a, T: 'a>(pub &'a [T]);
 pub struct AbiSpace(pub Abi);
 
+pub struct HRef<'a> {
+    pub did: DefId,
+    pub text: &'a str,
+}
+
 impl<'a> VisSpace<'a> {
     pub fn get(self) -> &'a Option<clean::Visibility> {
         let VisSpace(v) = self; v
@@ -291,17 +296,19 @@ impl fmt::Display for clean::Path {
 
 pub fn href(did: DefId) -> Option<(String, ItemType, Vec<String>)> {
     let cache = cache();
+    if !did.is_local() && !cache.access_levels.is_doc_reachable(did) {
+        return None
+    }
+
     let loc = CURRENT_LOCATION_KEY.with(|l| l.borrow().clone());
     let &(ref fqp, shortty) = match cache.paths.get(&did) {
         Some(p) => p,
         None => return None,
     };
+
     let mut url = if did.is_local() || cache.inlined.contains(&did) {
         repeat("../").take(loc.len()).collect::<String>()
     } else {
-        if !cache.access_levels.is_doc_reachable(did) {
-            return None
-        }
         match cache.extern_locations[&did.krate] {
             (_, render::Remote(ref s)) => s.to_string(),
             (_, render::Local) => repeat("../").take(loc.len()).collect(),
@@ -361,15 +368,7 @@ fn resolved_path(w: &mut fmt::Formatter, did: DefId, path: &clean::Path,
             }
         }
     }
-
-    match href(did) {
-        Some((url, shortty, fqp)) => {
-            write!(w, "<a class='{}' href='{}' title='{}'>{}</a>",
-                   shortty, url, fqp.join("::"), last.name)?;
-        }
-        _ => write!(w, "{}", last.name)?,
-    }
-    write!(w, "{}", last.params)?;
+    write!(w, "{}{}", HRef::new(did, &last.name), last.params)?;
     Ok(())
 }
 
@@ -435,6 +434,24 @@ fn tybounds(w: &mut fmt::Formatter,
     }
 }
 
+impl<'a> HRef<'a> {
+    pub fn new(did: DefId, text: &'a str) -> HRef<'a> {
+        HRef { did: did, text: text }
+    }
+}
+
+impl<'a> fmt::Display for HRef<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match href(self.did) {
+            Some((url, shortty, fqp)) => {
+                write!(f, "<a class='{}' href='{}' title='{}'>{}</a>",
+                       shortty, url, fqp.join("::"), self.text)
+            }
+            _ => write!(f, "{}", self.text),
+        }
+    }
+}
+
 impl fmt::Display for clean::Type {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {

src/librustdoc/html/render.rs

@@ -1640,8 +1640,8 @@ fn plain_summary_line(s: Option<&str>) -> String {
 }
 
 fn document(w: &mut fmt::Formatter, cx: &Context, item: &clean::Item) -> fmt::Result {
-    if let Some(s) = short_stability(item, cx, true) {
-        write!(w, "<div class='stability'>{}</div>", s)?;
+    for stability in short_stability(item, cx, true) {
+        write!(w, "<div class='stability'>{}</div>", stability)?;
     }
     if let Some(s) = item.doc_value() {
         write!(w, "<div class='docblock'>{}</div>", Markdown(s))?;
@@ -1739,16 +1739,19 @@ fn item_module(w: &mut fmt::Formatter, cx: &Context,
 
         match myitem.inner {
             clean::ExternCrateItem(ref name, ref src) => {
+                use html::format::HRef;
+
                 match *src {
                     Some(ref src) => {
                         write!(w, "<tr><td><code>{}extern crate {} as {};",
                                VisSpace(&myitem.visibility),
-                               src,
+                               HRef::new(myitem.def_id, src),
                                name)?
                     }
                     None => {
                         write!(w, "<tr><td><code>{}extern crate {};",
-                               VisSpace(&myitem.visibility), name)?
+                               VisSpace(&myitem.visibility),
+                               HRef::new(myitem.def_id, name))?
                     }
                 }
                 write!(w, "</code></td></tr>")?;
@@ -1761,8 +1764,15 @@ fn item_module(w: &mut fmt::Formatter, cx: &Context,
 
             _ => {
                 if myitem.name.is_none() { continue }
-                let stab_docs = if let Some(s) = short_stability(myitem, cx, false) {
-                    format!("[{}]", s)
+
+                let stabilities = short_stability(myitem, cx, false);
+
+                let stab_docs = if !stabilities.is_empty() {
+                    stabilities.iter()
+                               .map(|s| format!("[{}]", s))
+                               .collect::<Vec<_>>()
+                               .as_slice()
+                               .join(" ")
                 } else {
                     String::new()
                 };
@@ -1789,21 +1799,26 @@ fn item_module(w: &mut fmt::Formatter, cx: &Context,
     write!(w, "</table>")
 }
 
-fn short_stability(item: &clean::Item, cx: &Context, show_reason: bool) -> Option<String> {
-    item.stability.as_ref().and_then(|stab| {
+fn short_stability(item: &clean::Item, cx: &Context, show_reason: bool) -> Vec<String> {
+    let mut stability = vec![];
+
+    if let Some(stab) = item.stability.as_ref() {
         let reason = if show_reason && !stab.reason.is_empty() {
             format!(": {}", stab.reason)
         } else {
             String::new()
         };
-        let text = if !stab.deprecated_since.is_empty() {
+        if !stab.deprecated_since.is_empty() {
             let since = if show_reason {
                 format!(" since {}", Escape(&stab.deprecated_since))
             } else {
                 String::new()
             };
-            format!("Deprecated{}{}", since, Markdown(&reason))
-        } else if stab.level == stability::Unstable {
+            let text = format!("Deprecated{}{}", since, Markdown(&reason));
+            stability.push(format!("<em class='stab deprecated'>{}</em>", text))
+        };
+
+        if stab.level == stability::Unstable {
             let unstable_extra = if show_reason {
                 match (!stab.feature.is_empty(), &cx.shared.issue_tracker_base_url, stab.issue) {
                     (true, &Some(ref tracker_url), Some(issue_no)) if issue_no > 0 =>
@@ -1819,29 +1834,26 @@ fn short_stability(item: &clean::Item, cx: &Context, show_reason: bool) -> Optio
             } else {
                 String::new()
             };
-            format!("Unstable{}{}", unstable_extra, Markdown(&reason))
+            let text = format!("Unstable{}{}", unstable_extra, Markdown(&reason));
+            stability.push(format!("<em class='stab unstable'>{}</em>", text))
+        };
+    } else if let Some(depr) = item.deprecation.as_ref() {
+        let note = if show_reason && !depr.note.is_empty() {
+            format!(": {}", depr.note)
         } else {
-            return None
+            String::new()
+        };
+        let since = if show_reason && !depr.since.is_empty() {
+            format!(" since {}", Escape(&depr.since))
+        } else {
+            String::new()
         };
-        Some(format!("<em class='stab {}'>{}</em>",
-                     item.stability_class(), text))
-    }).or_else(|| {
-        item.deprecation.as_ref().and_then(|depr| {
-            let note = if show_reason && !depr.note.is_empty() {
-                format!(": {}", depr.note)
-            } else {
-                String::new()
-            };
-            let since = if show_reason && !depr.since.is_empty() {
-                format!(" since {}", Escape(&depr.since))
-            } else {
-                String::new()
-            };
 
-            let text = format!("Deprecated{}{}", since, Markdown(&note));
-            Some(format!("<em class='stab deprecated'>{}</em>", text))
-        })
-    })
+        let text = format!("Deprecated{}{}", since, Markdown(&note));
+        stability.push(format!("<em class='stab deprecated'>{}</em>", text))
+    }
+
+    stability
 }
 
 struct Initializer<'a>(&'a str);
@@ -2548,10 +2560,11 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi
                 if !is_static || render_static {
                     let id = derive_id(format!("{}.{}", shortty, name));
                     write!(w, "<h4 id='{}' class='{}'>", id, shortty)?;
-                    render_stability_since_raw(w, item.stable_since(), outer_version)?;
                     write!(w, "<code>")?;
                     render_assoc_item(w, item, link.anchor(&id))?;
-                    write!(w, "</code></h4>\n")?;
+                    write!(w, "</code>")?;
+                    render_stability_since_raw(w, item.stable_since(), outer_version)?;
+                    write!(w, "</h4>\n")?;
                 }
             }
             clean::TypedefItem(ref tydef, _) => {