Skip to content

Commit

Permalink
cfg taken out of Attributes, put in Item
Browse files Browse the repository at this point in the history
check item.is_fake() instead of self_id.is_some()

Remove empty branching in Attributes::from_ast

diverse small refacto after Josha review

cfg computation moved in merge_attrs

refacto use from_ast twice for coherence

take cfg out of Attributes and move it to Item
  • Loading branch information
Timothée Delabrouille committed Apr 27, 2021
1 parent b4f1dfd commit 727f904
Show file tree
Hide file tree
Showing 12 changed files with 104 additions and 85 deletions.
1 change: 1 addition & 0 deletions src/librustdoc/clean/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
synthetic: true,
blanket_impl: None,
}),
cfg: None,
})
}

Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/clean/blanket_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
synthetic: false,
blanket_impl: Some(trait_ref.self_ty().clean(self.cx)),
}),
cfg: None,
});
}
}
Expand Down
46 changes: 28 additions & 18 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Support for inlining external documentation into the current AST.

use std::iter::once;
use std::sync::Arc;

use rustc_ast as ast;
use rustc_data_structures::fx::FxHashSet;
Expand All @@ -15,7 +16,7 @@ use rustc_span::hygiene::MacroKind;
use rustc_span::symbol::{kw, sym, Symbol};
use rustc_span::Span;

use crate::clean::{self, Attributes, GetDefId, ToSource};
use crate::clean::{self, Attributes, AttributesExt, GetDefId, ToSource};
use crate::core::DocContext;
use crate::formats::item_type::ItemType;

Expand Down Expand Up @@ -120,11 +121,16 @@ crate fn try_inline(
_ => return None,
};

let target_attrs = load_attrs(cx, did);
let attrs = box merge_attrs(cx, Some(parent_module), target_attrs, attrs_clone);

let (attrs, cfg) = merge_attrs(cx, Some(parent_module), load_attrs(cx, did), attrs_clone);
cx.inlined.insert(did);
ret.push(clean::Item::from_def_id_and_attrs_and_parts(did, Some(name), kind, attrs, cx));
ret.push(clean::Item::from_def_id_and_attrs_and_parts(
did,
Some(name),
kind,
box attrs,
cx,
cfg,
));
Some(ret)
}

Expand Down Expand Up @@ -288,22 +294,24 @@ fn merge_attrs(
parent_module: Option<DefId>,
old_attrs: Attrs<'_>,
new_attrs: Option<Attrs<'_>>,
) -> clean::Attributes {
) -> (clean::Attributes, Option<Arc<clean::cfg::Cfg>>) {
// NOTE: If we have additional attributes (from a re-export),
// always insert them first. This ensure that re-export
// doc comments show up before the original doc comments
// when we render them.
if let Some(inner) = new_attrs {
if let Some(new_id) = parent_module {
let diag = cx.sess().diagnostic();
Attributes::from_ast(diag, old_attrs, Some((inner, new_id)))
} else {
let mut both = inner.to_vec();
both.extend_from_slice(old_attrs);
both.clean(cx)
}
let mut both = inner.to_vec();
both.extend_from_slice(old_attrs);
(
if let Some(new_id) = parent_module {
Attributes::from_ast(old_attrs, Some((inner, new_id)))
} else {
Attributes::from_ast(&both, None)
},
both.cfg(cx.sess().diagnostic()),
)
} else {
old_attrs.clean(cx)
(old_attrs.clean(cx), old_attrs.cfg(cx.sess().diagnostic()))
}
}

Expand Down Expand Up @@ -414,8 +422,8 @@ crate fn build_impl(

debug!("build_impl: impl {:?} for {:?}", trait_.def_id(), for_.def_id());

let attrs = box merge_attrs(cx, parent_module.into(), load_attrs(cx, did), attrs);
debug!("merged_attrs={:?}", attrs);
let (merged_attrs, cfg) = merge_attrs(cx, parent_module.into(), load_attrs(cx, did), attrs);
debug!("merged_attrs={:?}", merged_attrs);

ret.push(clean::Item::from_def_id_and_attrs_and_parts(
did,
Expand All @@ -432,8 +440,9 @@ crate fn build_impl(
synthetic: false,
blanket_impl: None,
}),
attrs,
box merged_attrs,
cx,
cfg,
));
}

Expand Down Expand Up @@ -479,6 +488,7 @@ fn build_module(
},
true,
)),
cfg: None,
});
} else if let Some(i) = try_inline(cx, did, item.res, item.ident.name, None, visited) {
items.extend(i)
Expand Down
6 changes: 4 additions & 2 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ impl Clean<Item> for doctree::Module<'_> {
}

impl Clean<Attributes> for [ast::Attribute] {
fn clean(&self, cx: &mut DocContext<'_>) -> Attributes {
Attributes::from_ast(cx.sess().diagnostic(), self, None)
fn clean(&self, _cx: &mut DocContext<'_>) -> Attributes {
Attributes::from_ast(self, None)
}
}

Expand Down Expand Up @@ -1998,13 +1998,15 @@ fn clean_extern_crate(
return items;
}
}

// FIXME: using `from_def_id_and_kind` breaks `rustdoc/masked` for some reason
vec![Item {
name: Some(name),
attrs: box attrs.clean(cx),
def_id: crate_def_id,
visibility: krate.vis.clean(cx),
kind: box ExternCrateItem { src: orig_name },
cfg: attrs.cfg(cx.sess().diagnostic()),
}]
}

Expand Down
90 changes: 53 additions & 37 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,11 +219,13 @@ crate struct Item {
/// E.g., struct vs enum vs function.
crate kind: Box<ItemKind>,
crate def_id: DefId,

crate cfg: Option<Arc<Cfg>>,
}

// `Item` is used a lot. Make sure it doesn't unintentionally get bigger.
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(Item, 40);
rustc_data_structures::static_assert_size!(Item, 48);

impl fmt::Debug for Item {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand All @@ -235,6 +237,7 @@ impl fmt::Debug for Item {
.field("kind", &self.kind)
.field("visibility", &self.visibility)
.field("def_id", def_id)
.field("cfg", &self.cfg)
.finish()
}
}
Expand Down Expand Up @@ -262,6 +265,10 @@ impl Item {
if self.is_fake() { None } else { tcx.lookup_deprecation(self.def_id) }
}

crate fn inner_docs(&self, tcx: TyCtxt<'_>) -> bool {
if self.is_fake() { false } else { tcx.get_attrs(self.def_id).inner_docs() }
}

crate fn span(&self, tcx: TyCtxt<'_>) -> Span {
let kind = match &*self.kind {
ItemKind::StrippedItem(k) => k,
Expand Down Expand Up @@ -305,12 +312,15 @@ impl Item {
kind: ItemKind,
cx: &mut DocContext<'_>,
) -> Item {
let ast_attrs = cx.tcx.get_attrs(def_id);

Self::from_def_id_and_attrs_and_parts(
def_id,
name,
kind,
box cx.tcx.get_attrs(def_id).clean(cx),
box ast_attrs.clean(cx),
cx,
ast_attrs.cfg(cx.sess().diagnostic()),
)
}

Expand All @@ -320,6 +330,7 @@ impl Item {
kind: ItemKind,
attrs: Box<Attributes>,
cx: &mut DocContext<'_>,
cfg: Option<Arc<Cfg>>,
) -> Item {
debug!("name={:?}, def_id={:?}", name, def_id);

Expand All @@ -329,6 +340,7 @@ impl Item {
name,
attrs,
visibility: cx.tcx.visibility(def_id).clean(cx),
cfg,
}
}

Expand Down Expand Up @@ -668,6 +680,8 @@ crate trait AttributesExt {
fn inner_docs(&self) -> bool;

fn other_attrs(&self) -> Vec<ast::Attribute>;

fn cfg(&self, diagnostic: &::rustc_errors::Handler) -> Option<Arc<Cfg>>;
}

impl AttributesExt for [ast::Attribute] {
Expand All @@ -691,6 +705,41 @@ impl AttributesExt for [ast::Attribute] {
fn other_attrs(&self) -> Vec<ast::Attribute> {
self.iter().filter(|attr| attr.doc_str().is_none()).cloned().collect()
}

fn cfg(&self, diagnostic: &::rustc_errors::Handler) -> Option<Arc<Cfg>> {
let mut cfg = Cfg::True;

for attr in self.iter() {
if attr.doc_str().is_none() && attr.has_name(sym::doc) {
if let Some(mi) = attr.meta() {
if let Some(cfg_mi) = Attributes::extract_cfg(&mi) {
// Extracted #[doc(cfg(...))]
match Cfg::parse(cfg_mi) {
Ok(new_cfg) => cfg &= new_cfg,
Err(e) => diagnostic.span_err(e.span, e.msg),
}
}
}
}
}

for attr in self.lists(sym::target_feature) {
if attr.has_name(sym::enable) {
if let Some(feat) = attr.value_str() {
let meta = attr::mk_name_value_item_str(
Ident::with_dummy_span(sym::target_feature),
feat,
DUMMY_SP,
);
if let Ok(feat_cfg) = Cfg::parse(&meta) {
cfg &= feat_cfg;
}
}
}
}

if cfg == Cfg::True { None } else { Some(Arc::new(cfg)) }
}
}

crate trait NestedAttributesExt {
Expand Down Expand Up @@ -799,7 +848,6 @@ impl<'a> FromIterator<&'a DocFragment> for String {
crate struct Attributes {
crate doc_strings: Vec<DocFragment>,
crate other_attrs: Vec<ast::Attribute>,
crate cfg: Option<Arc<Cfg>>,
}

#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)]
Expand Down Expand Up @@ -914,12 +962,10 @@ impl Attributes {
}

crate fn from_ast(
diagnostic: &::rustc_errors::Handler,
attrs: &[ast::Attribute],
additional_attrs: Option<(&[ast::Attribute], DefId)>,
) -> Attributes {
let mut doc_strings: Vec<DocFragment> = vec![];
let mut cfg = Cfg::True;
let mut doc_line = 0;

fn update_need_backline(doc_strings: &mut Vec<DocFragment>, frag: &DocFragment) {
Expand Down Expand Up @@ -967,14 +1013,7 @@ impl Attributes {
} else {
if attr.has_name(sym::doc) {
if let Some(mi) = attr.meta() {
if let Some(cfg_mi) = Attributes::extract_cfg(&mi) {
// Extracted #[doc(cfg(...))]
match Cfg::parse(cfg_mi) {
Ok(new_cfg) => cfg &= new_cfg,
Err(e) => diagnostic.span_err(e.span, e.msg),
}
} else if let Some((filename, contents)) = Attributes::extract_include(&mi)
{
if let Some((filename, contents)) = Attributes::extract_include(&mi) {
let line = doc_line;
doc_line += contents.as_str().lines().count();
let frag = DocFragment {
Expand Down Expand Up @@ -1004,28 +1043,7 @@ impl Attributes {
.filter_map(clean_attr)
.collect();

// treat #[target_feature(enable = "feat")] attributes as if they were
// #[doc(cfg(target_feature = "feat"))] attributes as well
for attr in attrs.lists(sym::target_feature) {
if attr.has_name(sym::enable) {
if let Some(feat) = attr.value_str() {
let meta = attr::mk_name_value_item_str(
Ident::with_dummy_span(sym::target_feature),
feat,
DUMMY_SP,
);
if let Ok(feat_cfg) = Cfg::parse(&meta) {
cfg &= feat_cfg;
}
}
}
}

Attributes {
doc_strings,
other_attrs,
cfg: if cfg == Cfg::True { None } else { Some(Arc::new(cfg)) },
}
Attributes { doc_strings, other_attrs }
}

/// Finds the `doc` attribute as a NameValue and returns the corresponding
Expand Down Expand Up @@ -1091,7 +1109,6 @@ impl Attributes {
impl PartialEq for Attributes {
fn eq(&self, rhs: &Self) -> bool {
self.doc_strings == rhs.doc_strings
&& self.cfg == rhs.cfg
&& self
.other_attrs
.iter()
Expand All @@ -1105,7 +1122,6 @@ impl Eq for Attributes {}
impl Hash for Attributes {
fn hash<H: Hasher>(&self, hasher: &mut H) {
self.doc_strings.hash(hasher);
self.cfg.hash(hasher);
for attr in &self.other_attrs {
attr.id.hash(hasher);
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/doctest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1093,9 +1093,9 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> {
nested: F,
) {
let ast_attrs = self.tcx.hir().attrs(hir_id);
let mut attrs = Attributes::from_ast(ast_attrs, None);

let mut attrs = Attributes::from_ast(self.sess.diagnostic(), ast_attrs, None);
if let Some(ref cfg) = attrs.cfg {
if let Some(ref cfg) = ast_attrs.cfg(self.sess.diagnostic()) {
if !cfg.matches(&self.sess.parse_sess, Some(&self.sess.features_untracked())) {
return;
}
Expand Down
3 changes: 1 addition & 2 deletions src/librustdoc/html/render/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ use serde::ser::{Serialize, SerializeStruct, Serializer};

use crate::clean;
use crate::clean::types::{
AttributesExt, FnDecl, FnRetTy, GenericBound, Generics, GetDefId, Type, TypeKind,
WherePredicate,
AttributesExt, FnDecl, FnRetTy, GenericBound, Generics, GetDefId, Type, WherePredicate,
};
use crate::formats::cache::Cache;
use crate::formats::item_type::ItemType;
Expand Down
9 changes: 2 additions & 7 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,17 +608,12 @@ fn document_item_info(
}

fn portability(item: &clean::Item, parent: Option<&clean::Item>) -> Option<String> {
let cfg = match (&item.attrs.cfg, parent.and_then(|p| p.attrs.cfg.as_ref())) {
let cfg = match (&item.cfg, parent.and_then(|p| p.cfg.as_ref())) {
(Some(cfg), Some(parent_cfg)) => cfg.simplify_with(parent_cfg),
(cfg, _) => cfg.as_deref().cloned(),
};

debug!(
"Portability {:?} - {:?} = {:?}",
item.attrs.cfg,
parent.and_then(|p| p.attrs.cfg.as_ref()),
cfg
);
debug!("Portability {:?} - {:?} = {:?}", item.cfg, parent.and_then(|p| p.cfg.as_ref()), cfg);

Some(format!("<div class=\"stab portability\">{}</div>", cfg?.render_long_html()))
}
Expand Down
Loading

0 comments on commit 727f904

Please sign in to comment.