Auto merge of #65750 - nnethercote:cheaper-doc-comments, r=petrochenkov

Cheaper doc comments

This PR implements the idea from #60935: represent doc comments more cheaply, rather than converting them into `#[doc="..."]` attribute form. Unlike #60936 (which is about coalescing doc comments to reduce their number), this approach does not have any backwards compatibility concerns, and it eliminates about 80-90% of the current cost of doc comments (as estimated using the numbers in #60930, which eliminated the cost of doc comments entirely by treating them as normal comments).

r? @petrochenkov

Author: bors

src/librustc/hir/lowering.rs

@@ -997,14 +997,20 @@ impl<'a> LoweringContext<'a> {
         // Note that we explicitly do not walk the path. Since we don't really
         // lower attributes (we use the AST version) there is nowhere to keep
         // the `HirId`s. We don't actually need HIR version of attributes anyway.
+        let kind = match attr.kind {
+            AttrKind::Normal(ref item) => {
+                AttrKind::Normal(AttrItem {
+                    path: item.path.clone(),
+                    tokens: self.lower_token_stream(item.tokens.clone()),
+                })
+            }
+            AttrKind::DocComment(comment) => AttrKind::DocComment(comment)
+        };
+
         Attribute {
-            item: AttrItem {
-                path: attr.path.clone(),
-                tokens: self.lower_token_stream(attr.tokens.clone()),
-            },
+            kind,
             id: attr.id,
             style: attr.style,
-            is_sugared_doc: attr.is_sugared_doc,
             span: attr.span,
         }
     }

src/librustc/ich/impls_syntax.rs

@@ -177,7 +177,7 @@ impl<'a> HashStable<StableHashingContext<'a>> for [ast::Attribute] {
         let filtered: SmallVec<[&ast::Attribute; 8]> = self
             .iter()
             .filter(|attr| {
-                !attr.is_sugared_doc &&
+                !attr.is_doc_comment() &&
                 !attr.ident().map_or(false, |ident| hcx.is_ignored_attr(ident.name))
             })
             .collect();
@@ -207,19 +207,16 @@ impl<'a> HashStable<StableHashingContext<'a>> for ast::Attribute {
     fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
         // Make sure that these have been filtered out.
         debug_assert!(!self.ident().map_or(false, |ident| hcx.is_ignored_attr(ident.name)));
-        debug_assert!(!self.is_sugared_doc);
-
-        let ast::Attribute {
-            ref item,
-            id: _,
-            style,
-            is_sugared_doc: _,
-            span,
-        } = *self;
-
-        item.hash_stable(hcx, hasher);
-        style.hash_stable(hcx, hasher);
-        span.hash_stable(hcx, hasher);
+        debug_assert!(!self.is_doc_comment());
+
+        let ast::Attribute { kind, id: _, style, span } = self;
+        if let ast::AttrKind::Normal(item) = kind {
+            item.hash_stable(hcx, hasher);
+            style.hash_stable(hcx, hasher);
+            span.hash_stable(hcx, hasher);
+        } else {
+            unreachable!();
+        }
     }
 }
 

src/librustc_lint/builtin.rs

@@ -706,7 +706,7 @@ impl EarlyLintPass for DeprecatedAttr {
             }
         }
         if attr.check_name(sym::no_start) || attr.check_name(sym::crate_id) {
-            let path_str = pprust::path_to_string(&attr.path);
+            let path_str = pprust::path_to_string(&attr.get_normal_item().path);
             let msg = format!("use of deprecated attribute `{}`: no longer used.", path_str);
             lint_deprecated_attr(cx, attr, &msg, None);
         }
@@ -736,7 +736,7 @@ impl UnusedDocComment {
         let mut sugared_span: Option<Span> = None;
 
         while let Some(attr) = attrs.next() {
-            if attr.is_sugared_doc {
+            if attr.is_doc_comment() {
                 sugared_span = Some(
                     sugared_span.map_or_else(
                         || attr.span,
@@ -745,7 +745,7 @@ impl UnusedDocComment {
                 );
             }
 
-            if attrs.peek().map(|next_attr| next_attr.is_sugared_doc).unwrap_or_default() {
+            if attrs.peek().map(|next_attr| next_attr.is_doc_comment()).unwrap_or_default() {
                 continue;
             }
 

src/librustc_metadata/link_args.rs

@@ -11,7 +11,7 @@ crate fn collect(tcx: TyCtxt<'_>) -> Vec<String> {
     tcx.hir().krate().visit_all_item_likes(&mut collector);
 
     for attr in tcx.hir().krate().attrs.iter() {
-        if attr.path == sym::link_args {
+        if attr.has_name(sym::link_args) {
             if let Some(linkarg) = attr.value_str() {
                 collector.add_link_args(&linkarg.as_str());
             }

src/librustc_passes/ast_validation.rs

@@ -328,7 +328,7 @@ impl<'a> AstValidator<'a> {
                 let arr = [sym::allow, sym::cfg, sym::cfg_attr, sym::deny, sym::forbid, sym::warn];
                 !arr.contains(&attr.name_or_empty()) && is_builtin_attr(attr)
             })
-            .for_each(|attr| if attr.is_sugared_doc {
+            .for_each(|attr| if attr.is_doc_comment() {
                 let mut err = self.err_handler().struct_span_err(
                     attr.span,
                     "documentation comments cannot be applied to function parameters"

src/librustc_resolve/build_reduced_graph.rs

@@ -1229,8 +1229,10 @@ impl<'a, 'b> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b> {
     }
 
     fn visit_attribute(&mut self, attr: &'b ast::Attribute) {
-        if !attr.is_sugared_doc && is_builtin_attr(attr) {
-            self.r.builtin_attrs.push((attr.path.segments[0].ident, self.parent_scope));
+        if !attr.is_doc_comment() && is_builtin_attr(attr) {
+            self.r.builtin_attrs.push(
+                (attr.get_normal_item().path.segments[0].ident, self.parent_scope)
+            );
         }
         visit::walk_attribute(self, attr);
     }

src/librustc_resolve/macros.rs

@@ -179,7 +179,10 @@ impl<'a> base::Resolver for Resolver<'a> {
 
         let (path, kind, derives, after_derive) = match invoc.kind {
             InvocationKind::Attr { ref attr, ref derives, after_derive, .. } =>
-                (&attr.path, MacroKind::Attr, self.arenas.alloc_ast_paths(derives), after_derive),
+                (&attr.get_normal_item().path,
+                 MacroKind::Attr,
+                 self.arenas.alloc_ast_paths(derives),
+                 after_derive),
             InvocationKind::Bang { ref mac, .. } =>
                 (&mac.path, MacroKind::Bang, &[][..], false),
             InvocationKind::Derive { ref path, .. } =>

src/librustc_save_analysis/lib.rs

@@ -885,7 +885,7 @@ impl<'l, 'tcx> SaveContext<'l, 'tcx> {
         for attr in attrs {
             if attr.check_name(sym::doc) {
                 if let Some(val) = attr.value_str() {
-                    if attr.is_sugared_doc {
+                    if attr.is_doc_comment() {
                         result.push_str(&strip_doc_comment_decoration(&val.as_str()));
                     } else {
                         result.push_str(&val.as_str());
@@ -1195,7 +1195,7 @@ fn null_id() -> rls_data::Id {
 fn lower_attributes(attrs: Vec<Attribute>, scx: &SaveContext<'_, '_>) -> Vec<rls_data::Attribute> {
     attrs.into_iter()
     // Only retain real attributes. Doc comments are lowered separately.
-    .filter(|attr| attr.path != sym::doc)
+    .filter(|attr| !attr.has_name(sym::doc))
     .map(|mut attr| {
         // Remove the surrounding '#[..]' or '#![..]' of the pretty printed
         // attribute. First normalize all inner attribute (#![..]) to outer

src/librustc_typeck/collect.rs

@@ -2706,7 +2706,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
     }
 
     codegen_fn_attrs.inline = attrs.iter().fold(InlineAttr::None, |ia, attr| {
-        if attr.path != sym::inline {
+        if !attr.has_name(sym::inline) {
             return ia;
         }
         match attr.meta().map(|i| i.kind) {
@@ -2746,7 +2746,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
     });
 
     codegen_fn_attrs.optimize = attrs.iter().fold(OptimizeAttr::None, |ia, attr| {
-        if attr.path != sym::optimize {
+        if !attr.has_name(sym::optimize) {
             return ia;
         }
         let err = |sp, s| span_err!(tcx.sess.diagnostic(), sp, E0722, "{}", s);

src/librustdoc/clean/mod.rs

@@ -26,7 +26,7 @@ use rustc::ty::{self, DefIdTree, TyCtxt, Region, RegionVid, Ty, AdtKind};
 use rustc::ty::fold::TypeFolder;
 use rustc::ty::layout::VariantIdx;
 use rustc::util::nodemap::{FxHashMap, FxHashSet};
-use syntax::ast::{self, Attribute, AttrStyle, AttrItem, Ident};
+use syntax::ast::{self, Attribute, AttrStyle, AttrKind, Ident};
 use syntax::attr;
 use syntax::parse::lexer::comments;
 use syntax::source_map::DUMMY_SP;
@@ -859,31 +859,32 @@ impl Attributes {
         let mut cfg = Cfg::True;
         let mut doc_line = 0;
 
-        /// Converts `attr` to a normal `#[doc="foo"]` comment, if it is a
-        /// comment like `///` or `/** */`. (Returns `attr` unchanged for
-        /// non-sugared doc attributes.)
-        pub fn with_desugared_doc<T>(attr: &Attribute, f: impl FnOnce(&Attribute) -> T) -> T {
-            if attr.is_sugared_doc {
-                let comment = attr.value_str().unwrap();
-                let meta = attr::mk_name_value_item_str(
-                    Ident::with_dummy_span(sym::doc),
-                    Symbol::intern(&comments::strip_doc_comment_decoration(&comment.as_str())),
-                    DUMMY_SP,
-                );
-                f(&Attribute {
-                    item: AttrItem { path: meta.path, tokens: meta.kind.tokens(meta.span) },
-                    id: attr.id,
-                    style: attr.style,
-                    is_sugared_doc: true,
-                    span: attr.span,
-                })
-            } else {
-                f(attr)
+        /// If `attr` is a doc comment, strips the leading and (if present)
+        /// trailing comments symbols, e.g. `///`, `/**`, and `*/`. Otherwise,
+        /// returns `attr` unchanged.
+        pub fn with_doc_comment_markers_stripped<T>(
+            attr: &Attribute,
+            f: impl FnOnce(&Attribute) -> T
+        ) -> T {
+            match attr.kind {
+                AttrKind::Normal(_) => {
+                    f(attr)
+                }
+                AttrKind::DocComment(comment) => {
+                    let comment =
+                        Symbol::intern(&comments::strip_doc_comment_decoration(&comment.as_str()));
+                    f(&Attribute {
+                        kind: AttrKind::DocComment(comment),
+                        id: attr.id,
+                        style: attr.style,
+                        span: attr.span,
+                    })
+                }
             }
         }
 
         let other_attrs = attrs.iter().filter_map(|attr| {
-            with_desugared_doc(attr, |attr| {
+            with_doc_comment_markers_stripped(attr, |attr| {
                 if attr.check_name(sym::doc) {
                     if let Some(mi) = attr.meta() {
                         if let Some(value) = mi.value_str() {
@@ -892,7 +893,7 @@ impl Attributes {
                             let line = doc_line;
                             doc_line += value.lines().count();
 
-                            if attr.is_sugared_doc {
+                            if attr.is_doc_comment() {
                                 doc_strings.push(DocFragment::SugaredDoc(line, attr.span, value));
                             } else {
                                 doc_strings.push(DocFragment::RawDoc(line, attr.span, value));

src/libsyntax/ast.rs

@@ -2190,22 +2190,29 @@ pub struct AttrItem {
 }
 
 /// Metadata associated with an item.
-/// Doc-comments are promoted to attributes that have `is_sugared_doc = true`.
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
 pub struct Attribute {
-    pub item: AttrItem,
+    pub kind: AttrKind,
     pub id: AttrId,
     /// Denotes if the attribute decorates the following construct (outer)
     /// or the construct this attribute is contained within (inner).
     pub style: AttrStyle,
-    pub is_sugared_doc: bool,
     pub span: Span,
 }
 
-// Compatibility impl to avoid churn, consider removing.
-impl std::ops::Deref for Attribute {
-    type Target = AttrItem;
-    fn deref(&self) -> &Self::Target { &self.item }
+#[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
+pub enum AttrKind {
+    /// A normal attribute.
+    Normal(AttrItem),
+
+    /// A doc comment (e.g. `/// ...`, `//! ...`, `/** ... */`, `/*! ... */`).
+    /// Doc attributes (e.g. `#[doc="..."]`) are represented with the `Normal`
+    /// variant (which is much less compact and thus more expensive).
+    ///
+    /// Note: `self.has_name(sym::doc)` and `self.check_name(sym::doc)` succeed
+    /// for this variant, but this may change in the future.
+    /// ```
+    DocComment(Symbol),
 }
 
 /// `TraitRef`s appear in impls.

src/libsyntax/attr/builtin.rs

@@ -228,18 +228,18 @@ fn find_stability_generic<'a, I>(sess: &ParseSess,
             sym::stable,
             sym::rustc_promotable,
             sym::rustc_allow_const_fn_ptr,
-        ].iter().any(|&s| attr.path == s) {
+        ].iter().any(|&s| attr.has_name(s)) {
             continue // not a stability level
         }
 
         mark_used(attr);
 
         let meta = attr.meta();
 
-        if attr.path == sym::rustc_promotable {
+        if attr.has_name(sym::rustc_promotable) {
             promotable = true;
         }
-        if attr.path == sym::rustc_allow_const_fn_ptr {
+        if attr.has_name(sym::rustc_allow_const_fn_ptr) {
             allow_const_fn_ptr = true;
         }
         // attributes with data
@@ -778,7 +778,7 @@ pub fn find_repr_attrs(sess: &ParseSess, attr: &Attribute) -> Vec<ReprAttr> {
 
     let mut acc = Vec::new();
     let diagnostic = &sess.span_diagnostic;
-    if attr.path == sym::repr {
+    if attr.has_name(sym::repr) {
         if let Some(items) = attr.meta_item_list() {
             mark_used(attr);
             for item in items {