rustc_errors: only box the diagnostic field in DiagnosticBuilder.

Author: eddyb

compiler/rustc_errors/src/diagnostic_builder.rs

@@ -15,20 +15,14 @@ use tracing::debug;
 /// extending `HandlerFlags`, accessed via `self.handler.flags`.
 #[must_use]
 #[derive(Clone)]
-pub struct DiagnosticBuilder<'a>(Box<DiagnosticBuilderInner<'a>>);
-
-/// This is a large type, and often used as a return value, especially within
-/// the frequently-used `PResult` type. In theory, return value optimization
-/// (RVO) should avoid unnecessary copying. In practice, it does not (at the
-/// time of writing). The split between `DiagnosticBuilder` and
-/// `DiagnosticBuilderInner` exists to avoid many `memcpy` calls.
-// FIXME(eddyb) try having two pointers in `DiagnosticBuilder`, by only boxing
-// `Diagnostic` (i.e. `struct DiagnosticBuilder(&Handler, Box<Diagnostic>);`).
-#[must_use]
-#[derive(Clone)]
-struct DiagnosticBuilderInner<'a> {
+pub struct DiagnosticBuilder<'a> {
     handler: &'a Handler,
-    diagnostic: Diagnostic,
+
+    /// `Diagnostic` is a large type, and `DiagnosticBuilder` is often used as a
+    /// return value, especially within the frequently-used `PResult` type.
+    /// In theory, return value optimization (RVO) should avoid unnecessary
+    /// copying. In practice, it does not (at the time of writing).
+    diagnostic: Box<Diagnostic>,
 }
 
 /// In general, the `DiagnosticBuilder` uses deref to allow access to
@@ -61,7 +55,7 @@ macro_rules! forward {
         $(#[$attrs])*
         #[doc = concat!("See [`Diagnostic::", stringify!($n), "()`].")]
         pub fn $n(&mut self, $($name: $ty),*) -> &mut Self {
-            self.0.diagnostic.$n($($name),*);
+            self.diagnostic.$n($($name),*);
             self
         }
     };
@@ -78,7 +72,7 @@ macro_rules! forward {
         $(#[$attrs])*
         #[doc = concat!("See [`Diagnostic::", stringify!($n), "()`].")]
         pub fn $n<$($generic: $bound),*>(&mut self, $($name: $ty),*) -> &mut Self {
-            self.0.diagnostic.$n($($name),*);
+            self.diagnostic.$n($($name),*);
             self
         }
     };
@@ -88,20 +82,20 @@ impl<'a> Deref for DiagnosticBuilder<'a> {
     type Target = Diagnostic;
 
     fn deref(&self) -> &Diagnostic {
-        &self.0.diagnostic
+        &self.diagnostic
     }
 }
 
 impl<'a> DerefMut for DiagnosticBuilder<'a> {
     fn deref_mut(&mut self) -> &mut Diagnostic {
-        &mut self.0.diagnostic
+        &mut self.diagnostic
     }
 }
 
 impl<'a> DiagnosticBuilder<'a> {
     /// Emit the diagnostic.
     pub fn emit(&mut self) {
-        self.0.handler.emit_diagnostic(&self);
+        self.handler.emit_diagnostic(&self);
         self.cancel();
     }
 
@@ -131,19 +125,19 @@ impl<'a> DiagnosticBuilder<'a> {
     /// Converts the builder to a `Diagnostic` for later emission,
     /// unless handler has disabled such buffering.
     pub fn into_diagnostic(mut self) -> Option<(Diagnostic, &'a Handler)> {
-        if self.0.handler.flags.dont_buffer_diagnostics
-            || self.0.handler.flags.treat_err_as_bug.is_some()
+        if self.handler.flags.dont_buffer_diagnostics
+            || self.handler.flags.treat_err_as_bug.is_some()
         {
             self.emit();
             return None;
         }
 
-        let handler = self.0.handler;
+        let handler = self.handler;
 
         // We must use `Level::Cancelled` for `dummy` to avoid an ICE about an
         // unused diagnostic.
         let dummy = Diagnostic::new(Level::Cancelled, "");
-        let diagnostic = std::mem::replace(&mut self.0.diagnostic, dummy);
+        let diagnostic = std::mem::replace(&mut *self.diagnostic, dummy);
 
         // Logging here is useful to help track down where in logs an error was
         // actually emitted.
@@ -170,7 +164,7 @@ impl<'a> DiagnosticBuilder<'a> {
     /// locally in whichever way makes the most sense.
     pub fn delay_as_bug(&mut self) {
         self.level = Level::Bug;
-        self.0.handler.delay_as_bug(self.0.diagnostic.clone());
+        self.handler.delay_as_bug((*self.diagnostic).clone());
         self.cancel();
     }
 
@@ -187,7 +181,7 @@ impl<'a> DiagnosticBuilder<'a> {
     /// ["primary span"][`MultiSpan`]; only the `Span` supplied when creating the diagnostic is
     /// primary.
     pub fn span_label(&mut self, span: Span, label: impl Into<String>) -> &mut Self {
-        self.0.diagnostic.span_label(span, label);
+        self.diagnostic.span_label(span, label);
         self
     }
 
@@ -200,7 +194,7 @@ impl<'a> DiagnosticBuilder<'a> {
     ) -> &mut Self {
         let label = label.as_ref();
         for span in spans {
-            self.0.diagnostic.span_label(span, label);
+            self.diagnostic.span_label(span, label);
         }
         self
     }
@@ -340,13 +334,13 @@ impl<'a> DiagnosticBuilder<'a> {
     /// diagnostic.
     crate fn new_diagnostic(handler: &'a Handler, diagnostic: Diagnostic) -> DiagnosticBuilder<'a> {
         debug!("Created new diagnostic");
-        DiagnosticBuilder(Box::new(DiagnosticBuilderInner { handler, diagnostic }))
+        DiagnosticBuilder { handler, diagnostic: Box::new(diagnostic) }
     }
 }
 
 impl<'a> Debug for DiagnosticBuilder<'a> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.0.diagnostic.fmt(f)
+        self.diagnostic.fmt(f)
     }
 }
 
@@ -356,7 +350,7 @@ impl<'a> Drop for DiagnosticBuilder<'a> {
     fn drop(&mut self) {
         if !panicking() && !self.cancelled() {
             let mut db = DiagnosticBuilder::new(
-                self.0.handler,
+                self.handler,
                 Level::Bug,
                 "the following error was constructed but not emitted",
             );

compiler/rustc_errors/src/lib.rs

@@ -54,9 +54,9 @@ pub use snippet::Style;
 pub type PResult<'a, T> = Result<T, DiagnosticBuilder<'a>>;
 
 // `PResult` is used a lot. Make sure it doesn't unintentionally get bigger.
-// (See also the comment on `DiagnosticBuilderInner`.)
+// (See also the comment on `DiagnosticBuilder`'s `diagnostic` field.)
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
-rustc_data_structures::static_assert_size!(PResult<'_, bool>, 16);
+rustc_data_structures::static_assert_size!(PResult<'_, bool>, 24);
 
 #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash, Encodable, Decodable)]
 pub enum SuggestionStyle {

src/tools/rustfmt/src/parse/session.rs

@@ -315,7 +315,7 @@ mod tests {
                 code: None,
                 message: vec![],
                 children: vec![],
-                suggestions: vec![],
+                suggestions: Ok(vec![]),
                 span: span.unwrap_or_else(MultiSpan::new),
                 sort_span: DUMMY_SP,
                 is_lint: false,