From 934d259b8ca1237c3fb50be21b8bba5ec8e6a7ce Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 19 Aug 2022 14:35:15 +0200 Subject: [PATCH 1/5] Fix invalid comparison for Class::Decoration in `is_equal_to` --- src/librustdoc/html/highlight.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 9d8ee52a3faf8..944cf08851240 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -291,8 +291,8 @@ impl Class { match (self, other) { (Self::Self_(_), Self::Self_(_)) | (Self::Macro(_), Self::Macro(_)) - | (Self::Ident(_), Self::Ident(_)) - | (Self::Decoration(_), Self::Decoration(_)) => true, + | (Self::Ident(_), Self::Ident(_)) => true, + (Self::Decoration(c1), Self::Decoration(c2)) => c1 == c2, (x, y) => x == y, } } From 042e0d02d74862796e9716abee07455b5e885151 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 19 Aug 2022 17:27:22 +0200 Subject: [PATCH 2/5] Merge "EnterSpan" events to reduce code blocks DOM size --- src/librustdoc/html/highlight.rs | 91 ++++++++++++++++++++++++-------- 1 file changed, 68 insertions(+), 23 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 944cf08851240..562a5933025f2 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -141,7 +141,7 @@ fn write_pending_elems( if !done { // We only want to "open" the tag ourselves if we have more than one pending and if the current // parent tag is not the same as our pending content. - let open_tag_ourselves = pending_elems.len() > 1; + let open_tag_ourselves = pending_elems.len() > 1 && current_class.is_some(); let close_tag = if open_tag_ourselves { enter_span(out, current_class.unwrap(), &href_context) } else { @@ -158,6 +158,18 @@ fn write_pending_elems( *current_class = None; } +fn handle_exit_span( + out: &mut Buffer, + href_context: &Option>, + pending_elems: &mut Vec<(&str, Option)>, + closing_tags: &mut Vec<(&str, Class)>, +) { + let class = closing_tags.last().expect("ExitSpan without EnterSpan").1; + // We flush everything just in case... + write_pending_elems(out, href_context, pending_elems, &mut Some(class), closing_tags); + exit_span(out, closing_tags.pop().expect("ExitSpan without EnterSpan").0); +} + /// Check if two `Class` can be merged together. In the following rules, "unclassified" means `None` /// basically (since it's `Option`). The following rules apply: /// @@ -171,7 +183,7 @@ fn can_merge(class1: Option, class2: Option, text: &str) -> bool { (Some(c1), Some(c2)) => c1.is_equal_to(c2), (Some(Class::Ident(_)), None) | (None, Some(Class::Ident(_))) => true, (Some(_), None) | (None, Some(_)) => text.trim().is_empty(), - _ => false, + (None, None) => true, } } @@ -196,6 +208,9 @@ fn write_code( let src = src.replace("\r\n", "\n"); // It contains the closing tag and the associated `Class`. let mut closing_tags: Vec<(&'static str, Class)> = Vec::new(); + // This is used because we don't automatically generate the closing tag on `ExitSpan` in + // case an `EnterSpan` event with the same class follows. + let mut pending_exit_span: Option = None; // The following two variables are used to group HTML elements with same `class` attributes // to reduce the DOM size. let mut current_class: Option = None; @@ -211,9 +226,21 @@ fn write_code( .highlight(&mut |highlight| { match highlight { Highlight::Token { text, class } => { + // If we received a `ExitSpan` event and then have a non-compatible `Class`, we + // need to close the ``. + if let Some(pending) = pending_exit_span && + !can_merge(Some(pending), class, text) { + handle_exit_span( + out, + &href_context, + &mut pending_elems, + &mut closing_tags, + ); + pending_exit_span = None; + current_class = class.map(Class::dummy); // If the two `Class` are different, time to flush the current content and start // a new one. - if !can_merge(current_class, class, text) { + } else if !can_merge(current_class, class, text) { write_pending_elems( out, &href_context, @@ -228,30 +255,48 @@ fn write_code( pending_elems.push((text, class)); } Highlight::EnterSpan { class } => { - // We flush everything just in case... - write_pending_elems( - out, - &href_context, - &mut pending_elems, - &mut current_class, - &closing_tags, - ); - closing_tags.push((enter_span(out, class, &href_context), class)) + let mut should_add = true; + if pending_exit_span.is_some() { + if !can_merge(Some(class), pending_exit_span, "") { + handle_exit_span(out, &href_context, &mut pending_elems, &mut closing_tags); + } else { + should_add = false; + } + } else { + // We flush everything just in case... + write_pending_elems( + out, + &href_context, + &mut pending_elems, + &mut current_class, + &closing_tags, + ); + } + current_class = None; + pending_exit_span = None; + if should_add { + let closing_tag = enter_span(out, class, &href_context); + closing_tags.push((closing_tag, class)); + } } Highlight::ExitSpan => { - // We flush everything just in case... - write_pending_elems( - out, - &href_context, - &mut pending_elems, - &mut current_class, - &closing_tags, - ); - exit_span(out, closing_tags.pop().expect("ExitSpan without EnterSpan").0) + current_class = None; + pending_exit_span = + Some(closing_tags.last().as_ref().expect("ExitSpan without EnterSpan").1); } }; }); - write_pending_elems(out, &href_context, &mut pending_elems, &mut current_class, &closing_tags); + if pending_exit_span.is_some() { + handle_exit_span(out, &href_context, &mut pending_elems, &mut closing_tags); + } else { + write_pending_elems( + out, + &href_context, + &mut pending_elems, + &mut current_class, + &closing_tags, + ); + } } fn write_footer(out: &mut Buffer, playground_button: Option<&str>) { @@ -761,7 +806,7 @@ impl<'a> Classifier<'a> { TokenKind::CloseBracket => { if self.in_attribute { self.in_attribute = false; - sink(Highlight::Token { text: "]", class: Some(Class::Attribute) }); + sink(Highlight::Token { text: "]", class: None }); sink(Highlight::ExitSpan); return; } From f5b5d867d5954c2a843f6a3db338ea86fafd66b1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 19 Aug 2022 18:16:02 +0200 Subject: [PATCH 3/5] Update rustdoc tests --- .../html/highlight/fixtures/decorations.html | 4 ++-- .../html/highlight/fixtures/sample.html | 15 ++++++++------- src/librustdoc/html/highlight/fixtures/sample.rs | 1 + src/test/rustdoc/issue-41783.codeblock.html | 4 ++-- src/test/rustdoc/issue-41783.rs | 6 ++++-- 5 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/librustdoc/html/highlight/fixtures/decorations.html b/src/librustdoc/html/highlight/fixtures/decorations.html index 2184897872153..f73e7c1fe7697 100644 --- a/src/librustdoc/html/highlight/fixtures/decorations.html +++ b/src/librustdoc/html/highlight/fixtures/decorations.html @@ -1,2 +1,2 @@ -let x = 1; -let y = 2; \ No newline at end of file +let x = 1; +let y = 2; \ No newline at end of file diff --git a/src/librustdoc/html/highlight/fixtures/sample.html b/src/librustdoc/html/highlight/fixtures/sample.html index ae2650528eb72..4a5a3cf609cd4 100644 --- a/src/librustdoc/html/highlight/fixtures/sample.html +++ b/src/librustdoc/html/highlight/fixtures/sample.html @@ -8,12 +8,13 @@ .lifetime { color: #B76514; } .question-mark { color: #ff9011; } -
#![crate_type = "lib"]
+
#![crate_type = "lib"]
 
-use std::path::{Path, PathBuf};
+use std::path::{Path, PathBuf};
 
-#[cfg(target_os = "linux")]
-fn main() -> () {
+#[cfg(target_os = "linux")]
+#[cfg(target_os = "windows")]
+fn main() -> () {
     let foo = true && false || true;
     let _: *const () = 0;
     let _ = &foo;
@@ -22,8 +23,8 @@
     mac!(foo, &mut bar);
     assert!(self.length < N && index <= self.length);
     ::std::env::var("gateau").is_ok();
-    #[rustfmt::skip]
-    let s:std::path::PathBuf = std::path::PathBuf::new();
+    #[rustfmt::skip]
+    let s:std::path::PathBuf = std::path::PathBuf::new();
     let mut s = String::new();
 
     match &s {
@@ -31,7 +32,7 @@
     }
 }
 
-macro_rules! bar {
+macro_rules! bar {
     ($foo:tt) => {};
 }
 

diff --git a/src/librustdoc/html/highlight/fixtures/sample.rs b/src/librustdoc/html/highlight/fixtures/sample.rs
index fbfdc6767337c..ef85b566cb3c4 100644
--- a/src/librustdoc/html/highlight/fixtures/sample.rs
+++ b/src/librustdoc/html/highlight/fixtures/sample.rs
@@ -3,6 +3,7 @@
 use std::path::{Path, PathBuf};
 
 #[cfg(target_os = "linux")]
+#[cfg(target_os = "windows")]
 fn main() -> () {
     let foo = true && false || true;
     let _: *const () = 0;
diff --git a/src/test/rustdoc/issue-41783.codeblock.html b/src/test/rustdoc/issue-41783.codeblock.html
index b919935e4b47a..89987491d1b46 100644
--- a/src/test/rustdoc/issue-41783.codeblock.html
+++ b/src/test/rustdoc/issue-41783.codeblock.html
@@ -1,5 +1,5 @@
 # single
 ## double
 ### triple
-#[outer]
-#![inner]
\ No newline at end of file
+#[outer]
+#![inner]
diff --git a/src/test/rustdoc/issue-41783.rs b/src/test/rustdoc/issue-41783.rs
index d67716028799b..87267a750c615 100644
--- a/src/test/rustdoc/issue-41783.rs
+++ b/src/test/rustdoc/issue-41783.rs
@@ -1,8 +1,10 @@
 // @has issue_41783/struct.Foo.html
 // @!hasraw - 'space'
 // @!hasraw - 'comment'
-// @hasraw - '#[outer]'
-// @hasraw - '#![inner]'
+// @hasraw - '#[outer]'
+// @!hasraw - '#[outer]'
+// @hasraw - '#![inner]'
+// @!hasraw - '#![inner]'
 // @snapshot 'codeblock' - '//*[@class="rustdoc-toggle top-doc"]/*[@class="docblock"]//pre/code'
 
 /// ```no_run

From 4c89c2886d9d915db3e11a9f87a188cada9dd457 Mon Sep 17 00:00:00 2001
From: Guillaume Gomez 
Date: Fri, 19 Aug 2022 21:35:09 +0200
Subject: [PATCH 4/5] Clean up highlight `` merge code

---
 src/librustdoc/html/highlight.rs | 239 +++++++++++++++----------------
 1 file changed, 118 insertions(+), 121 deletions(-)

diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs
index 562a5933025f2..4a12d74ddef5a 100644
--- a/src/librustdoc/html/highlight.rs
+++ b/src/librustdoc/html/highlight.rs
@@ -111,65 +111,6 @@ fn write_header(out: &mut Buffer, class: &str, extra_content: Option) {
     write!(out, "");
 }
 
-/// Write all the pending elements sharing a same (or at mergeable) `Class`.
-///
-/// If there is a "parent" (if a `EnterSpan` event was encountered) and the parent can be merged
-/// with the elements' class, then we simply write the elements since the `ExitSpan` event will
-/// close the tag.
-///
-/// Otherwise, if there is only one pending element, we let the `string` function handle both
-/// opening and closing the tag, otherwise we do it into this function.
-fn write_pending_elems(
-    out: &mut Buffer,
-    href_context: &Option>,
-    pending_elems: &mut Vec<(&str, Option)>,
-    current_class: &mut Option,
-    closing_tags: &[(&str, Class)],
-) {
-    if pending_elems.is_empty() {
-        return;
-    }
-    let mut done = false;
-    if let Some((_, parent_class)) = closing_tags.last() {
-        if can_merge(*current_class, Some(*parent_class), "") {
-            for (text, class) in pending_elems.iter() {
-                string(out, Escape(text), *class, &href_context, false);
-            }
-            done = true;
-        }
-    }
-    if !done {
-        // We only want to "open" the tag ourselves if we have more than one pending and if the current
-        // parent tag is not the same as our pending content.
-        let open_tag_ourselves = pending_elems.len() > 1 && current_class.is_some();
-        let close_tag = if open_tag_ourselves {
-            enter_span(out, current_class.unwrap(), &href_context)
-        } else {
-            ""
-        };
-        for (text, class) in pending_elems.iter() {
-            string(out, Escape(text), *class, &href_context, !open_tag_ourselves);
-        }
-        if open_tag_ourselves {
-            exit_span(out, close_tag);
-        }
-    }
-    pending_elems.clear();
-    *current_class = None;
-}
-
-fn handle_exit_span(
-    out: &mut Buffer,
-    href_context: &Option>,
-    pending_elems: &mut Vec<(&str, Option)>,
-    closing_tags: &mut Vec<(&str, Class)>,
-) {
-    let class = closing_tags.last().expect("ExitSpan without EnterSpan").1;
-    // We flush everything just in case...
-    write_pending_elems(out, href_context, pending_elems, &mut Some(class), closing_tags);
-    exit_span(out, closing_tags.pop().expect("ExitSpan without EnterSpan").0);
-}
-
 /// Check if two `Class` can be merged together. In the following rules, "unclassified" means `None`
 /// basically (since it's `Option`). The following rules apply:
 ///
@@ -187,6 +128,87 @@ fn can_merge(class1: Option, class2: Option, text: &str) -> bool {
     }
 }
 
+/// This type is used as a conveniency to prevent having to pass all its fields as arguments into
+/// the various functions (which became its methods).
+struct TokenHandler<'a, 'b, 'c, 'd, 'e> {
+    out: &'a mut Buffer,
+    /// It contains the closing tag and the associated `Class`.
+    closing_tags: Vec<(&'static str, Class)>,
+    /// This is used because we don't automatically generate the closing tag on `ExitSpan` in
+    /// case an `EnterSpan` event with the same class follows.
+    pending_exit_span: Option,
+    /// `current_class` and `pending_elems` are used to group HTML elements with same `class`
+    /// attributes to reduce the DOM size.
+    current_class: Option,
+    /// We need to keep the `Class` for each element because it could contain a `Span` which is
+    /// used to generate links.
+    pending_elems: Vec<(&'b str, Option)>,
+    href_context: Option>,
+}
+
+impl<'a, 'b, 'c, 'd, 'e> TokenHandler<'a, 'b, 'c, 'd, 'e> {
+    fn handle_exit_span(&mut self) {
+        // We can't get the last `closing_tags` element using `pop()` because `closing_tags` is
+        // being used in `write_pending_elems`.
+        let class = self.closing_tags.last().expect("ExitSpan without EnterSpan").1;
+        // We flush everything just in case...
+        self.write_pending_elems(Some(class));
+
+        exit_span(self.out, self.closing_tags.pop().expect("ExitSpan without EnterSpan").0);
+        self.pending_exit_span = None;
+    }
+
+    /// Write all the pending elements sharing a same (or at mergeable) `Class`.
+    ///
+    /// If there is a "parent" (if a `EnterSpan` event was encountered) and the parent can be merged
+    /// with the elements' class, then we simply write the elements since the `ExitSpan` event will
+    /// close the tag.
+    ///
+    /// Otherwise, if there is only one pending element, we let the `string` function handle both
+    /// opening and closing the tag, otherwise we do it into this function.
+    ///
+    /// It returns `true` if `current_class` must be set to `None` afterwards.
+    fn write_pending_elems(&mut self, current_class: Option) -> bool {
+        if self.pending_elems.is_empty() {
+            return false;
+        }
+        if let Some((_, parent_class)) = self.closing_tags.last() &&
+            can_merge(current_class, Some(*parent_class), "")
+        {
+            for (text, class) in self.pending_elems.iter() {
+                string(self.out, Escape(text), *class, &self.href_context, false);
+            }
+        } else {
+            // We only want to "open" the tag ourselves if we have more than one pending and if the
+            // current parent tag is not the same as our pending content.
+            let close_tag = if self.pending_elems.len() > 1 && current_class.is_some() {
+                Some(enter_span(self.out, current_class.unwrap(), &self.href_context))
+            } else {
+                None
+            };
+            for (text, class) in self.pending_elems.iter() {
+                string(self.out, Escape(text), *class, &self.href_context, close_tag.is_none());
+            }
+            if let Some(close_tag) = close_tag {
+                exit_span(self.out, close_tag);
+            }
+        }
+        self.pending_elems.clear();
+        true
+    }
+}
+
+impl<'a, 'b, 'c, 'd, 'e> Drop for TokenHandler<'a, 'b, 'c, 'd, 'e> {
+    /// When leaving, we need to flush all pending data to not have missing content.
+    fn drop(&mut self) {
+        if self.pending_exit_span.is_some() {
+            self.handle_exit_span();
+        } else {
+            self.write_pending_elems(self.current_class);
+        }
+    }
+}
+
 /// Convert the given `src` source code into HTML by adding classes for highlighting.
 ///
 /// This code is used to render code blocks (in the documentation) as well as the source code pages.
@@ -206,21 +228,18 @@ fn write_code(
 ) {
     // This replace allows to fix how the code source with DOS backline characters is displayed.
     let src = src.replace("\r\n", "\n");
-    // It contains the closing tag and the associated `Class`.
-    let mut closing_tags: Vec<(&'static str, Class)> = Vec::new();
-    // This is used because we don't automatically generate the closing tag on `ExitSpan` in
-    // case an `EnterSpan` event with the same class follows.
-    let mut pending_exit_span: Option = None;
-    // The following two variables are used to group HTML elements with same `class` attributes
-    // to reduce the DOM size.
-    let mut current_class: Option = None;
-    // We need to keep the `Class` for each element because it could contain a `Span` which is
-    // used to generate links.
-    let mut pending_elems: Vec<(&str, Option)> = Vec::new();
+    let mut token_handler = TokenHandler {
+        out,
+        closing_tags: Vec::new(),
+        pending_exit_span: None,
+        current_class: None,
+        pending_elems: Vec::new(),
+        href_context,
+    };
 
     Classifier::new(
         &src,
-        href_context.as_ref().map(|c| c.file_span).unwrap_or(DUMMY_SP),
+        token_handler.href_context.as_ref().map(|c| c.file_span).unwrap_or(DUMMY_SP),
         decoration_info,
     )
     .highlight(&mut |highlight| {
@@ -228,75 +247,53 @@ fn write_code(
             Highlight::Token { text, class } => {
                 // If we received a `ExitSpan` event and then have a non-compatible `Class`, we
                 // need to close the ``.
-                if let Some(pending) = pending_exit_span &&
+                let need_current_class_update = if let Some(pending) = token_handler.pending_exit_span &&
                     !can_merge(Some(pending), class, text) {
-                        handle_exit_span(
-                            out,
-                            &href_context,
-                            &mut pending_elems,
-                            &mut closing_tags,
-                        );
-                        pending_exit_span = None;
-                        current_class = class.map(Class::dummy);
+                        token_handler.handle_exit_span();
+                        true
                 // If the two `Class` are different, time to flush the current content and start
                 // a new one.
-                } else if !can_merge(current_class, class, text) {
-                    write_pending_elems(
-                        out,
-                        &href_context,
-                        &mut pending_elems,
-                        &mut current_class,
-                        &closing_tags,
-                    );
-                    current_class = class.map(Class::dummy);
-                } else if current_class.is_none() {
-                    current_class = class.map(Class::dummy);
+                } else if !can_merge(token_handler.current_class, class, text) {
+                    token_handler.write_pending_elems(token_handler.current_class);
+                    true
+                } else {
+                    token_handler.current_class.is_none()
+                };
+
+                if need_current_class_update {
+                    token_handler.current_class = class.map(Class::dummy);
                 }
-                pending_elems.push((text, class));
+                token_handler.pending_elems.push((text, class));
             }
             Highlight::EnterSpan { class } => {
                 let mut should_add = true;
-                if pending_exit_span.is_some() {
-                    if !can_merge(Some(class), pending_exit_span, "") {
-                        handle_exit_span(out, &href_context, &mut pending_elems, &mut closing_tags);
-                    } else {
+                if let Some(pending_exit_span) = token_handler.pending_exit_span {
+                    if class.is_equal_to(pending_exit_span) {
                         should_add = false;
+                    } else {
+                        token_handler.handle_exit_span();
                     }
                 } else {
                     // We flush everything just in case...
-                    write_pending_elems(
-                        out,
-                        &href_context,
-                        &mut pending_elems,
-                        &mut current_class,
-                        &closing_tags,
-                    );
+                    if token_handler.write_pending_elems(token_handler.current_class) {
+                        token_handler.current_class = None;
+                    }
                 }
-                current_class = None;
-                pending_exit_span = None;
                 if should_add {
-                    let closing_tag = enter_span(out, class, &href_context);
-                    closing_tags.push((closing_tag, class));
+                    let closing_tag = enter_span(token_handler.out, class, &token_handler.href_context);
+                    token_handler.closing_tags.push((closing_tag, class));
                 }
+
+                token_handler.current_class = None;
+                token_handler.pending_exit_span = None;
             }
             Highlight::ExitSpan => {
-                current_class = None;
-                pending_exit_span =
-                    Some(closing_tags.last().as_ref().expect("ExitSpan without EnterSpan").1);
+                token_handler.current_class = None;
+                token_handler.pending_exit_span =
+                    Some(token_handler.closing_tags.last().as_ref().expect("ExitSpan without EnterSpan").1);
             }
         };
     });
-    if pending_exit_span.is_some() {
-        handle_exit_span(out, &href_context, &mut pending_elems, &mut closing_tags);
-    } else {
-        write_pending_elems(
-            out,
-            &href_context,
-            &mut pending_elems,
-            &mut current_class,
-            &closing_tags,
-        );
-    }
 }
 
 fn write_footer(out: &mut Buffer, playground_button: Option<&str>) {

From 7ab8e0cbe4c6e7617fe43a44467c463fad3b010e Mon Sep 17 00:00:00 2001
From: Guillaume Gomez 
Date: Sat, 20 Aug 2022 14:24:05 +0200
Subject: [PATCH 5/5] Extend decoration test to detect regressions

---
 src/librustdoc/html/highlight/fixtures/decorations.html | 4 +++-
 src/librustdoc/html/highlight/tests.rs                  | 7 +++++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/librustdoc/html/highlight/fixtures/decorations.html b/src/librustdoc/html/highlight/fixtures/decorations.html
index f73e7c1fe7697..ebf29f9cb3a91 100644
--- a/src/librustdoc/html/highlight/fixtures/decorations.html
+++ b/src/librustdoc/html/highlight/fixtures/decorations.html
@@ -1,2 +1,4 @@
 let x = 1;
-let y = 2;
\ No newline at end of file
+let y = 2;
+let z = 3;
+let a = 4;
\ No newline at end of file
diff --git a/src/librustdoc/html/highlight/tests.rs b/src/librustdoc/html/highlight/tests.rs
index 4861a8ad32da6..a5e633df43448 100644
--- a/src/librustdoc/html/highlight/tests.rs
+++ b/src/librustdoc/html/highlight/tests.rs
@@ -69,9 +69,12 @@ fn test_union_highlighting() {
 fn test_decorations() {
     create_default_session_globals_then(|| {
         let src = "let x = 1;
-let y = 2;";
+let y = 2;
+let z = 3;
+let a = 4;";
         let mut decorations = FxHashMap::default();
-        decorations.insert("example", vec![(0, 10)]);
+        decorations.insert("example", vec![(0, 10), (11, 21)]);
+        decorations.insert("example2", vec![(22, 32)]);
 
         let mut html = Buffer::new();
         write_code(&mut html, src, None, Some(DecorationInfo(decorations)));