Move doc = "" parsing out of DocParser to keep doc attributes order

Author: GuillaumeGomez

compiler/rustc_attr_parsing/src/attributes/doc.rs

@@ -2,7 +2,6 @@
 #![allow(unused_imports)]
 
 use rustc_ast::ast::{AttrStyle, LitKind, MetaItemLit};
-use rustc_ast::token::CommentKind;
 use rustc_errors::MultiSpan;
 use rustc_feature::template;
 use rustc_hir::attrs::{
@@ -81,19 +80,10 @@ fn parse_keyword_and_attribute<'c, S, F>(
     *attr_value = Some((value, path.span()));
 }
 
-#[derive(Debug)]
-struct DocComment {
-    style: AttrStyle,
-    kind: CommentKind,
-    span: Span,
-    comment: Symbol,
-}
-
 #[derive(Default, Debug)]
 pub(crate) struct DocParser {
     attribute: DocAttribute,
     nb_doc_attrs: usize,
-    doc_comment: Option<DocComment>,
 }
 
 impl DocParser {
@@ -481,17 +471,11 @@ impl DocParser {
                 }
             }
             ArgParser::NameValue(nv) => {
-                let Some(comment) = nv.value_as_str() else {
+                if nv.value_as_str().is_none() {
                     cx.expected_string_literal(nv.value_span, Some(nv.value_as_lit()));
-                    return;
-                };
-
-                self.doc_comment = Some(DocComment {
-                    style: cx.attr_style,
-                    kind: CommentKind::Block,
-                    span: nv.value_span,
-                    comment,
-                });
+                } else {
+                    unreachable!("Should have been handled at the same time as sugar-syntaxed doc comments");
+                }
             }
         }
     }
@@ -537,9 +521,7 @@ impl<S: Stage> AttributeParser<S> for DocParser {
     ]);
 
     fn finalize(self, _cx: &FinalizeContext<'_, '_, S>) -> Option<AttributeKind> {
-        if let Some(DocComment { style, kind, span, comment }) = self.doc_comment {
-            Some(AttributeKind::DocComment { style, kind, span, comment })
-        } else if self.nb_doc_attrs != 0 {
+        if self.nb_doc_attrs != 0 {
             Some(AttributeKind::Doc(Box::new(self.attribute)))
         } else {
             None

compiler/rustc_attr_parsing/src/interface.rs

@@ -2,6 +2,7 @@ use std::borrow::Cow;
 
 use rustc_ast as ast;
 use rustc_ast::{AttrStyle, NodeId};
+use rustc_ast::token::CommentKind;
 use rustc_errors::DiagCtxtHandle;
 use rustc_feature::{AttributeTemplate, Features};
 use rustc_hir::attrs::AttributeKind;
@@ -258,7 +259,8 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
             // that's expanded right? But no, sometimes, when parsing attributes on macros,
             // we already use the lowering logic and these are still there. So, when `omit_doc`
             // is set we *also* want to ignore these.
-            if omit_doc == OmitDoc::Skip && attr.has_name(sym::doc) {
+            let is_doc_attribute = attr.has_name(sym::doc);
+            if omit_doc == OmitDoc::Skip && is_doc_attribute {
                 continue;
             }
 
@@ -290,8 +292,40 @@ impl<'sess, S: Stage> AttributeParser<'sess, S> {
                         ) else {
                             continue;
                         };
-                        let path = parser.path();
                         let args = parser.args();
+
+                        // Special-case handling for `#[doc = "..."]`: if we go through with
+                        // `DocParser`, the order of doc comments will be messed up because `///`
+                        // doc comments are added into `attributes` whereas attributes parsed with
+                        // `DocParser` are added into `parsed_attributes` which are then appended
+                        // to `attributes`. So if you have:
+                        //
+                        // /// bla
+                        // #[doc = "a"]
+                        // /// blob
+                        //
+                        // You would get:
+                        //
+                        // bla
+                        // blob
+                        // a
+                        if is_doc_attribute
+                            && let ArgParser::NameValue(nv) = args
+                            // If not a string key/value, it should emit an error, but to make
+                            // things simpler, it's handled in `DocParser` because it's simpler to
+                            // emit an error with `AcceptContext`.
+                            && let Some(comment) = nv.value_as_str()
+                        {
+                            attributes.push(Attribute::Parsed(AttributeKind::DocComment {
+                                style: attr.style,
+                                kind: CommentKind::Block,
+                                span: nv.value_span,
+                                comment,
+                            }));
+                            continue;
+                        }
+
+                        let path = parser.path();
                         for accept in accepts {
                             let mut cx: AcceptContext<'_, 'sess, S> = AcceptContext {
                                 shared: SharedContext {