bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Box DiagnosticBuilder.

Browse source 
It's a large type -- 176 bytes on 64-bit. And it's passed around and
returned from a lot of functions, including within PResult.

This commit boxes it, which reduces memory traffic. In particular,
`PResult` shrinks to 16 bytes in the best case; this reduces instruction
counts by up to 2% on various workloads.
This commit is contained in:
Nicholas Nethercote 2019-09-12 08:29:17 +10:00
parent 2b8116dced
commit 2fcd870711
4 changed files with 55 additions and 37 deletions
Download patch file Download diff file Expand all files Collapse all files

2
src/librustc_errors/annotate_snippet_emitter_writer.rs
View file

@ -37,7 +37,7 @@ impl Emitter for AnnotateSnippetEmitterWriter {
&mut primary_span, &mut primary_span,
&mut children, &mut children,
&db.level, &db.level,
db.handler.flags.external_macro_backtrace); db.handler().flags.external_macro_backtrace);
self.emit_messages_default(&db.level, self.emit_messages_default(&db.level,
db.message(), db.message(),

81
src/librustc_errors/diagnostic_builder.rs
View file

@ -18,8 +18,17 @@ use log::debug;
/// extending `HandlerFlags`, accessed via `self.handler.flags`. /// extending `HandlerFlags`, accessed via `self.handler.flags`.
#[must_use] #[must_use]
#[derive(Clone)] #[derive(Clone)]
pub struct DiagnosticBuilder<'a> { pub struct DiagnosticBuilder<'a>(Box<DiagnosticBuilderInner<'a>>);
pub handler: &'a Handler,
/// 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.
#[must_use]
#[derive(Clone)]
struct DiagnosticBuilderInner<'a> {
handler: &'a Handler,
diagnostic: Diagnostic, diagnostic: Diagnostic,
allow_suggestions: bool, allow_suggestions: bool,
} }
@ -52,7 +61,7 @@ macro_rules! forward {
) => { ) => {
$(#[$attrs])* $(#[$attrs])*
pub fn $n(&mut self, $($name: $ty),*) -> &mut Self { pub fn $n(&mut self, $($name: $ty),*) -> &mut Self {
self.diagnostic.$n($($name),*); self.0.diagnostic.$n($($name),*);
self self
} }
}; };
@ -69,7 +78,7 @@ macro_rules! forward {
) => { ) => {
$(#[$attrs])* $(#[$attrs])*
pub fn $n<S: Into<MultiSpan>>(&mut self, $($name: $ty),*) -> &mut Self { pub fn $n<S: Into<MultiSpan>>(&mut self, $($name: $ty),*) -> &mut Self {
self.diagnostic.$n($($name),*); self.0.diagnostic.$n($($name),*);
self self
} }
}; };
@ -79,24 +88,28 @@ impl<'a> Deref for DiagnosticBuilder<'a> {
type Target = Diagnostic; type Target = Diagnostic;
fn deref(&self) -> &Diagnostic { fn deref(&self) -> &Diagnostic {
&self.diagnostic &self.0.diagnostic
} }
} }
impl<'a> DerefMut for DiagnosticBuilder<'a> { impl<'a> DerefMut for DiagnosticBuilder<'a> {
fn deref_mut(&mut self) -> &mut Diagnostic { fn deref_mut(&mut self) -> &mut Diagnostic {
&mut self.diagnostic &mut self.0.diagnostic
} }
} }
impl<'a> DiagnosticBuilder<'a> { impl<'a> DiagnosticBuilder<'a> {
pub fn handler(&self) -> &'a Handler{
self.0.handler
}
/// Emit the diagnostic. /// Emit the diagnostic.
pub fn emit(&mut self) { pub fn emit(&mut self) {
if self.cancelled() { if self.cancelled() {
return; return;
} }
self.handler.emit_db(&self); self.0.handler.emit_db(&self);
self.cancel(); self.cancel();
} }
@ -115,8 +128,8 @@ impl<'a> DiagnosticBuilder<'a> {
/// Buffers the diagnostic for later emission, unless handler /// Buffers the diagnostic for later emission, unless handler
/// has disabled such buffering. /// has disabled such buffering.
pub fn buffer(mut self, buffered_diagnostics: &mut Vec<Diagnostic>) { pub fn buffer(mut self, buffered_diagnostics: &mut Vec<Diagnostic>) {
if self.handler.flags.dont_buffer_diagnostics || if self.0.handler.flags.dont_buffer_diagnostics ||
self.handler.flags.treat_err_as_bug.is_some() self.0.handler.flags.treat_err_as_bug.is_some()
{ {
self.emit(); self.emit();
return; return;
@ -126,7 +139,7 @@ impl<'a> DiagnosticBuilder<'a> {
// implements `Drop`. // implements `Drop`.
let diagnostic; let diagnostic;
unsafe { unsafe {
diagnostic = std::ptr::read(&self.diagnostic); diagnostic = std::ptr::read(&self.0.diagnostic);
std::mem::forget(self); std::mem::forget(self);
}; };
// Logging here is useful to help track down where in logs an error was // Logging here is useful to help track down where in logs an error was
@ -144,7 +157,7 @@ impl<'a> DiagnosticBuilder<'a> {
span: Option<S>, span: Option<S>,
) -> &mut Self { ) -> &mut Self {
let span = span.map(|s| s.into()).unwrap_or_else(|| MultiSpan::new()); let span = span.map(|s| s.into()).unwrap_or_else(|| MultiSpan::new());
self.diagnostic.sub(level, message, span, None); self.0.diagnostic.sub(level, message, span, None);
self self
} }
@ -160,7 +173,7 @@ impl<'a> DiagnosticBuilder<'a> {
/// locally in whichever way makes the most sense. /// locally in whichever way makes the most sense.
pub fn delay_as_bug(&mut self) { pub fn delay_as_bug(&mut self) {
self.level = Level::Bug; self.level = Level::Bug;
self.handler.delay_as_bug(self.diagnostic.clone()); self.0.handler.delay_as_bug(self.0.diagnostic.clone());
self.cancel(); self.cancel();
} }
@ -171,7 +184,7 @@ impl<'a> DiagnosticBuilder<'a> {
/// then the snippet will just include that `Span`, which is /// then the snippet will just include that `Span`, which is
/// called the primary span. /// called the primary span.
pub fn span_label<T: Into<String>>(&mut self, span: Span, label: T) -> &mut Self { pub fn span_label<T: Into<String>>(&mut self, span: Span, label: T) -> &mut Self {
self.diagnostic.span_label(span, label); self.0.diagnostic.span_label(span, label);
self self
} }
@ -208,10 +221,10 @@ impl<'a> DiagnosticBuilder<'a> {
suggestion: Vec<(Span, String)>, suggestion: Vec<(Span, String)>,
applicability: Applicability, applicability: Applicability,
) -> &mut Self { ) -> &mut Self {
if !self.allow_suggestions { if !self.0.allow_suggestions {
return self return self
} }
self.diagnostic.multipart_suggestion( self.0.diagnostic.multipart_suggestion(
msg, msg,
suggestion, suggestion,
applicability, applicability,
@ -225,10 +238,10 @@ impl<'a> DiagnosticBuilder<'a> {
suggestion: Vec<(Span, String)>, suggestion: Vec<(Span, String)>,
applicability: Applicability, applicability: Applicability,
) -> &mut Self { ) -> &mut Self {
if !self.allow_suggestions { if !self.0.allow_suggestions {
return self return self
} }
self.diagnostic.tool_only_multipart_suggestion( self.0.diagnostic.tool_only_multipart_suggestion(
msg, msg,
suggestion, suggestion,
applicability, applicability,
@ -236,7 +249,6 @@ impl<'a> DiagnosticBuilder<'a> {
self self
} }
pub fn span_suggestion( pub fn span_suggestion(
&mut self, &mut self,
sp: Span, sp: Span,
@ -244,10 +256,10 @@ impl<'a> DiagnosticBuilder<'a> {
suggestion: String, suggestion: String,
applicability: Applicability, applicability: Applicability,
) -> &mut Self { ) -> &mut Self {
if !self.allow_suggestions { if !self.0.allow_suggestions {
return self return self
} }
self.diagnostic.span_suggestion( self.0.diagnostic.span_suggestion(
sp, sp,
msg, msg,
suggestion, suggestion,
@ -263,10 +275,10 @@ impl<'a> DiagnosticBuilder<'a> {
suggestions: impl Iterator<Item = String>, suggestions: impl Iterator<Item = String>,
applicability: Applicability, applicability: Applicability,
) -> &mut Self { ) -> &mut Self {
if !self.allow_suggestions { if !self.0.allow_suggestions {
return self return self
} }
self.diagnostic.span_suggestions( self.0.diagnostic.span_suggestions(
sp, sp,
msg, msg,
suggestions, suggestions,
@ -282,10 +294,10 @@ impl<'a> DiagnosticBuilder<'a> {
suggestion: String, suggestion: String,
applicability: Applicability, applicability: Applicability,
) -> &mut Self { ) -> &mut Self {
if !self.allow_suggestions { if !self.0.allow_suggestions {
return self return self
} }
self.diagnostic.span_suggestion_short( self.0.diagnostic.span_suggestion_short(
sp, sp,
msg, msg,
suggestion, suggestion,
@ -301,10 +313,10 @@ impl<'a> DiagnosticBuilder<'a> {
suggestion: String, suggestion: String,
applicability: Applicability, applicability: Applicability,
) -> &mut Self { ) -> &mut Self {
if !self.allow_suggestions { if !self.0.allow_suggestions {
return self return self
} }
self.diagnostic.span_suggestion_hidden( self.0.diagnostic.span_suggestion_hidden(
sp, sp,
msg, msg,
suggestion, suggestion,
@ -320,10 +332,10 @@ impl<'a> DiagnosticBuilder<'a> {
suggestion: String, suggestion: String,
applicability: Applicability, applicability: Applicability,
) -> &mut Self { ) -> &mut Self {
if !self.allow_suggestions { if !self.0.allow_suggestions {
return self return self
} }
self.diagnostic.tool_only_span_suggestion( self.0.diagnostic.tool_only_span_suggestion(
sp, sp,
msg, msg,
suggestion, suggestion,
@ -336,7 +348,7 @@ impl<'a> DiagnosticBuilder<'a> {
forward!(pub fn code(&mut self, s: DiagnosticId) -> &mut Self); forward!(pub fn code(&mut self, s: DiagnosticId) -> &mut Self);
pub fn allow_suggestions(&mut self, allow: bool) -> &mut Self { pub fn allow_suggestions(&mut self, allow: bool) -> &mut Self {
self.allow_suggestions = allow; self.0.allow_suggestions = allow;
self self
} }
@ -359,19 +371,18 @@ impl<'a> DiagnosticBuilder<'a> {
/// Creates a new `DiagnosticBuilder` with an already constructed /// Creates a new `DiagnosticBuilder` with an already constructed
/// diagnostic. /// diagnostic.
pub fn new_diagnostic(handler: &'a Handler, diagnostic: Diagnostic) pub fn new_diagnostic(handler: &'a Handler, diagnostic: Diagnostic) -> DiagnosticBuilder<'a> {
-> DiagnosticBuilder<'a> { DiagnosticBuilder(Box::new(DiagnosticBuilderInner {
DiagnosticBuilder {
handler, handler,
diagnostic, diagnostic,
allow_suggestions: true, allow_suggestions: true,
} }))
} }
} }
impl<'a> Debug for DiagnosticBuilder<'a> { impl<'a> Debug for DiagnosticBuilder<'a> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.diagnostic.fmt(f) self.0.diagnostic.fmt(f)
} }
} }
@ -381,7 +392,7 @@ impl<'a> Drop for DiagnosticBuilder<'a> {
fn drop(&mut self) { fn drop(&mut self) {
if !panicking() && !self.cancelled() { if !panicking() && !self.cancelled() {
let mut db = DiagnosticBuilder::new( let mut db = DiagnosticBuilder::new(
self.handler, self.0.handler,
Level::Bug, Level::Bug,
"the following error was constructed but not emitted", "the following error was constructed but not emitted",
); );

2
src/librustc_errors/emitter.rs
View file

@ -385,7 +385,7 @@ impl Emitter for EmitterWriter {
&mut primary_span, &mut primary_span,
&mut children, &mut children,
&db.level, &db.level,
db.handler.flags.external_macro_backtrace); db.handler().flags.external_macro_backtrace);
self.emit_messages_default(&db.level, self.emit_messages_default(&db.level,
&db.styled_message(), &db.styled_message(),

7
src/libsyntax/parse/mod.rs
View file

@ -13,6 +13,8 @@ use crate::symbol::Symbol;
use errors::{Applicability, FatalError, Level, Handler, ColorConfig, Diagnostic, DiagnosticBuilder}; use errors::{Applicability, FatalError, Level, Handler, ColorConfig, Diagnostic, DiagnosticBuilder};
use rustc_data_structures::fx::{FxHashSet, FxHashMap}; use rustc_data_structures::fx::{FxHashSet, FxHashMap};
#[cfg(target_arch = "x86_64")]
use rustc_data_structures::static_assert_size;
use rustc_data_structures::sync::{Lrc, Lock, Once}; use rustc_data_structures::sync::{Lrc, Lock, Once};
use syntax_pos::{Span, SourceFile, FileName, MultiSpan}; use syntax_pos::{Span, SourceFile, FileName, MultiSpan};
use syntax_pos::edition::Edition; use syntax_pos::edition::Edition;
@ -38,6 +40,11 @@ crate mod unescape_error_reporting;
pub type PResult<'a, T> = Result<T, DiagnosticBuilder<'a>>; 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`.)
#[cfg(target_arch = "x86_64")]
static_assert_size!(PResult<'_, bool>, 16);
/// Collected spans during parsing for places where a certain feature was /// Collected spans during parsing for places where a certain feature was
/// used and should be feature gated accordingly in `check_crate`. /// used and should be feature gated accordingly in `check_crate`.
#[derive(Default)] #[derive(Default)]