rustc_ast: Stop using "string typing" for doc comment tokens

Explicitly store their kind and style retrieved during lexing in the token

Author: petrochenkov

src/librustc_ast/ast.rs

@@ -23,7 +23,7 @@ pub use GenericArgs::*;
 pub use UnsafeSource::*;
 
 use crate::ptr::P;
-use crate::token::{self, DelimToken};
+use crate::token::{self, CommentKind, DelimToken};
 use crate::tokenstream::{DelimSpan, TokenStream, TokenTree};
 
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
@@ -2365,7 +2365,7 @@ pub enum AttrKind {
     /// 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).
-    DocComment(Symbol),
+    DocComment(CommentKind, Symbol),
 }
 
 /// `TraitRef`s appear in impls.

src/librustc_ast/attr/mod.rs

@@ -7,7 +7,7 @@ use crate::ast::{MacArgs, MacDelimiter, MetaItem, MetaItemKind, NestedMetaItem};
 use crate::ast::{Path, PathSegment};
 use crate::mut_visit::visit_clobber;
 use crate::ptr::P;
-use crate::token::{self, Token};
+use crate::token::{self, CommentKind, Token};
 use crate::tokenstream::{DelimSpan, TokenStream, TokenTree, TreeAndJoint};
 
 use rustc_data_structures::sync::Lock;
@@ -169,7 +169,7 @@ impl Attribute {
     pub fn has_name(&self, name: Symbol) -> bool {
         match self.kind {
             AttrKind::Normal(ref item) => item.path == name,
-            AttrKind::DocComment(_) => false,
+            AttrKind::DocComment(..) => false,
         }
     }
 
@@ -198,7 +198,7 @@ impl Attribute {
                     None
                 }
             }
-            AttrKind::DocComment(_) => None,
+            AttrKind::DocComment(..) => None,
         }
     }
     pub fn name_or_empty(&self) -> Symbol {
@@ -218,7 +218,7 @@ impl Attribute {
                 Some(MetaItem { kind: MetaItemKind::List(list), .. }) => Some(list),
                 _ => None,
             },
-            AttrKind::DocComment(_) => None,
+            AttrKind::DocComment(..) => None,
         }
     }
 
@@ -314,13 +314,13 @@ impl Attribute {
     pub fn is_doc_comment(&self) -> bool {
         match self.kind {
             AttrKind::Normal(_) => false,
-            AttrKind::DocComment(_) => true,
+            AttrKind::DocComment(..) => true,
         }
     }
 
     pub fn doc_str(&self) -> Option<Symbol> {
         match self.kind {
-            AttrKind::DocComment(symbol) => Some(symbol),
+            AttrKind::DocComment(.., data) => Some(data),
             AttrKind::Normal(ref item) if item.path == sym::doc => {
                 item.meta(self.span).and_then(|meta| meta.value_str())
             }
@@ -331,14 +331,14 @@ impl Attribute {
     pub fn get_normal_item(&self) -> &AttrItem {
         match self.kind {
             AttrKind::Normal(ref item) => item,
-            AttrKind::DocComment(_) => panic!("unexpected doc comment"),
+            AttrKind::DocComment(..) => panic!("unexpected doc comment"),
         }
     }
 
     pub fn unwrap_normal_item(self) -> AttrItem {
         match self.kind {
             AttrKind::Normal(item) => item,
-            AttrKind::DocComment(_) => panic!("unexpected doc comment"),
+            AttrKind::DocComment(..) => panic!("unexpected doc comment"),
         }
     }
 
@@ -405,8 +405,13 @@ pub fn mk_attr_outer(item: MetaItem) -> Attribute {
     mk_attr(AttrStyle::Outer, item.path, item.kind.mac_args(item.span), item.span)
 }
 
-pub fn mk_doc_comment(style: AttrStyle, comment: Symbol, span: Span) -> Attribute {
-    Attribute { kind: AttrKind::DocComment(comment), id: mk_attr_id(), style, span }
+pub fn mk_doc_comment(
+    comment_kind: CommentKind,
+    style: AttrStyle,
+    data: Symbol,
+    span: Span,
+) -> Attribute {
+    Attribute { kind: AttrKind::DocComment(comment_kind, data), id: mk_attr_id(), style, span }
 }
 
 pub fn list_contains_name(items: &[NestedMetaItem], name: Symbol) -> bool {

src/librustc_ast/mut_visit.rs

@@ -582,7 +582,7 @@ pub fn noop_visit_attribute<T: MutVisitor>(attr: &mut Attribute, vis: &mut T) {
             vis.visit_path(path);
             visit_mac_args(args, vis);
         }
-        AttrKind::DocComment(_) => {}
+        AttrKind::DocComment(..) => {}
     }
     vis.visit_span(span);
 }

src/librustc_ast/token.rs

@@ -17,6 +17,12 @@ use rustc_span::{self, Span, DUMMY_SP};
 use std::borrow::Cow;
 use std::{fmt, mem};
 
+#[derive(Clone, Copy, PartialEq, RustcEncodable, RustcDecodable, Debug, HashStable_Generic)]
+pub enum CommentKind {
+    Line,
+    Block,
+}
+
 #[derive(Clone, PartialEq, RustcEncodable, RustcDecodable, Hash, Debug, Copy)]
 #[derive(HashStable_Generic)]
 pub enum BinOpToken {
@@ -238,9 +244,10 @@ pub enum TokenKind {
 
     Interpolated(Lrc<Nonterminal>),
 
-    // Can be expanded into several tokens.
-    /// A doc comment.
-    DocComment(Symbol),
+    /// A doc comment token.
+    /// `Symbol` is the doc comment's data excluding its "quotes" (`///`, `/**`, etc)
+    /// similarly to symbols in string literal tokens.
+    DocComment(CommentKind, ast::AttrStyle, Symbol),
 
     // Junk. These carry no data because we don't really care about the data
     // they *would* carry, and don't really want to allocate a new ident for

src/librustc_ast/util/comments.rs

@@ -1,11 +1,10 @@
 pub use CommentStyle::*;
 
-use crate::ast;
+use crate::ast::AttrStyle;
+use crate::token::CommentKind;
 use rustc_span::source_map::SourceMap;
 use rustc_span::{BytePos, CharPos, FileName, Pos, Symbol};
 
-use log::debug;
-
 #[cfg(test)]
 mod tests;
 
@@ -28,43 +27,46 @@ pub struct Comment {
     pub pos: BytePos,
 }
 
-pub fn is_line_doc_comment(s: &str) -> bool {
-    let res = (s.starts_with("///") && *s.as_bytes().get(3).unwrap_or(&b' ') != b'/')
-        || s.starts_with("//!");
-    debug!("is {:?} a doc comment? {}", s, res);
-    res
-}
-
-pub fn is_block_doc_comment(s: &str) -> bool {
-    // Prevent `/**/` from being parsed as a doc comment
-    let res = ((s.starts_with("/**") && *s.as_bytes().get(3).unwrap_or(&b' ') != b'*')
-        || s.starts_with("/*!"))
-        && s.len() >= 5;
-    debug!("is {:?} a doc comment? {}", s, res);
-    res
-}
-
-// FIXME(#64197): Try to privatize this again.
-pub fn is_doc_comment(s: &str) -> bool {
-    (s.starts_with("///") && is_line_doc_comment(s))
-        || s.starts_with("//!")
-        || (s.starts_with("/**") && is_block_doc_comment(s))
-        || s.starts_with("/*!")
+/// For a full line comment string returns its doc comment style if it's a doc comment
+/// and returns `None` if it's a regular comment.
+pub fn line_doc_comment_style(line_comment: &str) -> Option<AttrStyle> {
+    let line_comment = line_comment.as_bytes();
+    assert!(line_comment.starts_with(b"//"));
+    match line_comment.get(2) {
+        // `//!` is an inner line doc comment.
+        Some(b'!') => Some(AttrStyle::Inner),
+        Some(b'/') => match line_comment.get(3) {
+            // `////` (more than 3 slashes) is not considered a doc comment.
+            Some(b'/') => None,
+            // Otherwise `///` is an outer line doc comment.
+            _ => Some(AttrStyle::Outer),
+        },
+        _ => None,
+    }
 }
 
-pub fn doc_comment_style(comment: Symbol) -> ast::AttrStyle {
-    let comment = &comment.as_str();
-    assert!(is_doc_comment(comment));
-    if comment.starts_with("//!") || comment.starts_with("/*!") {
-        ast::AttrStyle::Inner
-    } else {
-        ast::AttrStyle::Outer
+/// For a full block comment string returns its doc comment style if it's a doc comment
+/// and returns `None` if it's a regular comment.
+pub fn block_doc_comment_style(block_comment: &str, terminated: bool) -> Option<AttrStyle> {
+    let block_comment = block_comment.as_bytes();
+    assert!(block_comment.starts_with(b"/*"));
+    assert!(!terminated || block_comment.ends_with(b"*/"));
+    match block_comment.get(2) {
+        // `/*!` is an inner block doc comment.
+        Some(b'!') => Some(AttrStyle::Inner),
+        Some(b'*') => match block_comment.get(3) {
+            // `/***` (more than 2 stars) is not considered a doc comment.
+            Some(b'*') => None,
+            // `/**/` is not considered a doc comment.
+            Some(b'/') if block_comment.len() == 4 => None,
+            // Otherwise `/**` is an outer block doc comment.
+            _ => Some(AttrStyle::Outer),
+        },
+        _ => None,
     }
 }
 
-pub fn strip_doc_comment_decoration(comment: Symbol) -> String {
-    let comment = &comment.as_str();
-
+pub fn strip_doc_comment_decoration(data: Symbol, comment_kind: CommentKind) -> String {
     /// remove whitespace-only lines from the start/end of lines
     fn vertical_trim(lines: Vec<String>) -> Vec<String> {
         let mut i = 0;
@@ -126,26 +128,19 @@ pub fn strip_doc_comment_decoration(comment: Symbol) -> String {
         }
     }
 
-    // one-line comments lose their prefix
-    const ONELINERS: &[&str] = &["///!", "///", "//!", "//"];
-
-    for prefix in ONELINERS {
-        if comment.starts_with(*prefix) {
-            return (&comment[prefix.len()..]).to_string();
+    match comment_kind {
+        CommentKind::Line => {
+            let data = data.as_str();
+            let prefix_len = if data.starts_with('!') { 1 } else { 0 };
+            data[prefix_len..].to_string()
+        }
+        CommentKind::Block => {
+            let lines = data.as_str().lines().map(|s| s.to_string()).collect::<Vec<String>>();
+            let lines = vertical_trim(lines);
+            let lines = horizontal_trim(lines);
+            lines.join("\n")
         }
     }
-
-    if comment.starts_with("/*") {
-        let lines =
-            comment[3..comment.len() - 2].lines().map(|s| s.to_string()).collect::<Vec<String>>();
-
-        let lines = vertical_trim(lines);
-        let lines = horizontal_trim(lines);
-
-        return lines.join("\n");
-    }
-
-    panic!("not a doc-comment: {}", comment);
 }
 
 /// Returns `None` if the first `col` chars of `s` contain a non-whitespace char.
@@ -226,8 +221,8 @@ pub fn gather_comments(sm: &SourceMap, path: FileName, src: String) -> Vec<Comme
                     }
                 }
             }
-            rustc_lexer::TokenKind::BlockComment { terminated: _ } => {
-                if !is_block_doc_comment(token_text) {
+            rustc_lexer::TokenKind::BlockComment { terminated } => {
+                if block_doc_comment_style(token_text, terminated).is_none() {
                     let code_to_the_right = match text[pos + token.len..].chars().next() {
                         Some('\r' | '\n') => false,
                         _ => true,
@@ -249,7 +244,7 @@ pub fn gather_comments(sm: &SourceMap, path: FileName, src: String) -> Vec<Comme
                 }
             }
             rustc_lexer::TokenKind::LineComment => {
-                if !is_doc_comment(token_text) {
+                if line_doc_comment_style(token_text).is_none() {
                     comments.push(Comment {
                         style: if code_to_the_left { Trailing } else { Isolated },
                         lines: vec![token_text.to_string()],

src/librustc_ast/util/comments/tests.rs

@@ -1,58 +1,50 @@
 use super::*;
 use crate::with_default_session_globals;
 
+#[test]
+fn line_doc_comments() {
+    assert!(line_doc_comment_style("///").is_some());
+    assert!(line_doc_comment_style("/// blah").is_some());
+    assert!(line_doc_comment_style("////").is_none());
+}
+
 #[test]
 fn test_block_doc_comment_1() {
     with_default_session_globals(|| {
-        let comment = "/**\n * Test \n **  Test\n *   Test\n*/";
-        let stripped = strip_doc_comment_decoration(Symbol::intern(comment));
+        let comment = "\n * Test \n **  Test\n *   Test\n";
+        let stripped = strip_doc_comment_decoration(Symbol::intern(comment), CommentKind::Block);
         assert_eq!(stripped, " Test \n*  Test\n   Test");
     })
 }
 
 #[test]
 fn test_block_doc_comment_2() {
     with_default_session_globals(|| {
-        let comment = "/**\n * Test\n *  Test\n*/";
-        let stripped = strip_doc_comment_decoration(Symbol::intern(comment));
+        let comment = "\n * Test\n *  Test\n";
+        let stripped = strip_doc_comment_decoration(Symbol::intern(comment), CommentKind::Block);
         assert_eq!(stripped, " Test\n  Test");
     })
 }
 
 #[test]
 fn test_block_doc_comment_3() {
     with_default_session_globals(|| {
-        let comment = "/**\n let a: *i32;\n *a = 5;\n*/";
-        let stripped = strip_doc_comment_decoration(Symbol::intern(comment));
+        let comment = "\n let a: *i32;\n *a = 5;\n";
+        let stripped = strip_doc_comment_decoration(Symbol::intern(comment), CommentKind::Block);
         assert_eq!(stripped, " let a: *i32;\n *a = 5;");
     })
 }
 
-#[test]
-fn test_block_doc_comment_4() {
-    with_default_session_globals(|| {
-        let comment = "/*******************\n test\n *********************/";
-        let stripped = strip_doc_comment_decoration(Symbol::intern(comment));
-        assert_eq!(stripped, " test");
-    })
-}
-
 #[test]
 fn test_line_doc_comment() {
     with_default_session_globals(|| {
-        let stripped = strip_doc_comment_decoration(Symbol::intern("/// test"));
+        let stripped = strip_doc_comment_decoration(Symbol::intern(" test"), CommentKind::Line);
         assert_eq!(stripped, " test");
-        let stripped = strip_doc_comment_decoration(Symbol::intern("///! test"));
+        let stripped = strip_doc_comment_decoration(Symbol::intern("! test"), CommentKind::Line);
         assert_eq!(stripped, " test");
-        let stripped = strip_doc_comment_decoration(Symbol::intern("// test"));
-        assert_eq!(stripped, " test");
-        let stripped = strip_doc_comment_decoration(Symbol::intern("// test"));
-        assert_eq!(stripped, " test");
-        let stripped = strip_doc_comment_decoration(Symbol::intern("///test"));
-        assert_eq!(stripped, "test");
-        let stripped = strip_doc_comment_decoration(Symbol::intern("///!test"));
+        let stripped = strip_doc_comment_decoration(Symbol::intern("test"), CommentKind::Line);
         assert_eq!(stripped, "test");
-        let stripped = strip_doc_comment_decoration(Symbol::intern("//test"));
+        let stripped = strip_doc_comment_decoration(Symbol::intern("!test"), CommentKind::Line);
         assert_eq!(stripped, "test");
     })
 }

src/librustc_ast/visit.rs

@@ -880,7 +880,7 @@ pub fn walk_vis<'a, V: Visitor<'a>>(visitor: &mut V, vis: &'a Visibility) {
 pub fn walk_attribute<'a, V: Visitor<'a>>(visitor: &mut V, attr: &'a Attribute) {
     match attr.kind {
         AttrKind::Normal(ref item) => walk_mac_args(visitor, &item.args),
-        AttrKind::DocComment(_) => {}
+        AttrKind::DocComment(..) => {}
     }
 }
 

src/librustc_ast_lowering/lib.rs

@@ -981,7 +981,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 path: item.path.clone(),
                 args: self.lower_mac_args(&item.args),
             }),
-            AttrKind::DocComment(comment) => AttrKind::DocComment(comment),
+            AttrKind::DocComment(comment_kind, data) => AttrKind::DocComment(comment_kind, data),
         };
 
         Attribute { kind, id: attr.id, style: attr.style, span: attr.span }