From bb02cc47c49ee3cc1e913c243d69ee7cb23df598 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 16 Jun 2022 16:00:25 +1000 Subject: [PATCH] Move `finish` out of the `Encoder` trait. This simplifies things, but requires making `CacheEncoder` non-generic. (This was previously merged as commit 4 in #94732 and then was reverted in #97905 because it caused a perf regression.) --- compiler/rustc_codegen_ssa/src/lib.rs | 2 +- .../rustc_incremental/src/persist/save.rs | 2 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 9 +- .../rustc_query_impl/src/on_disk_cache.rs | 92 ++++++------------- compiler/rustc_query_impl/src/plumbing.rs | 3 +- .../src/dep_graph/serialized.rs | 2 +- compiler/rustc_serialize/src/opaque.rs | 28 +++--- compiler/rustc_serialize/src/serialize.rs | 12 +-- compiler/rustc_serialize/tests/opaque.rs | 4 +- src/librustdoc/scrape_examples.rs | 2 +- .../deriving-encodable-decodable-box.rs | 2 +- ...riving-encodable-decodable-cell-refcell.rs | 2 +- src/test/ui-fulldeps/issue-14021.rs | 2 +- 13 files changed, 51 insertions(+), 111 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/lib.rs b/compiler/rustc_codegen_ssa/src/lib.rs index 750432b0b2653..6c30923bc3d73 100644 --- a/compiler/rustc_codegen_ssa/src/lib.rs +++ b/compiler/rustc_codegen_ssa/src/lib.rs @@ -211,7 +211,7 @@ impl CodegenResults { encoder.emit_raw_bytes(&RLINK_VERSION.to_be_bytes()); encoder.emit_str(RUSTC_VERSION.unwrap()); Encodable::encode(codegen_results, &mut encoder); - encoder.finish().unwrap() + encoder.finish() } pub fn deserialize_rlink(data: Vec) -> Result { diff --git a/compiler/rustc_incremental/src/persist/save.rs b/compiler/rustc_incremental/src/persist/save.rs index 9341a742925d9..79836d66011a2 100644 --- a/compiler/rustc_incremental/src/persist/save.rs +++ b/compiler/rustc_incremental/src/persist/save.rs @@ -3,7 +3,7 @@ use rustc_data_structures::sync::join; use rustc_middle::dep_graph::{DepGraph, SerializedDepGraph, WorkProduct, WorkProductId}; use rustc_middle::ty::TyCtxt; use rustc_serialize::opaque::{FileEncodeResult, FileEncoder}; -use rustc_serialize::{Encodable as RustcEncodable, Encoder}; +use rustc_serialize::Encodable as RustcEncodable; use rustc_session::Session; use std::fs; diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 91ca4ecf3e132..2db0800190aa8 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -94,9 +94,6 @@ macro_rules! encoder_methods { } impl<'a, 'tcx> Encoder for EncodeContext<'a, 'tcx> { - type Ok = ::Ok; - type Err = ::Err; - encoder_methods! { emit_usize(usize); emit_u128(u128); @@ -119,10 +116,6 @@ impl<'a, 'tcx> Encoder for EncodeContext<'a, 'tcx> { emit_str(&str); emit_raw_bytes(&[u8]); } - - fn finish(self) -> Result { - self.opaque.finish() - } } impl<'a, 'tcx, T> Encodable> for LazyValue { @@ -2216,7 +2209,7 @@ fn encode_metadata_impl(tcx: TyCtxt<'_>) -> EncodedMetadata { // culminating in the `CrateRoot` which points to all of it. let root = ecx.encode_crate_root(); - let mut result = ecx.opaque.finish().unwrap(); + let mut result = ecx.opaque.finish(); // Encode the root position. let header = METADATA_HEADER.len(); diff --git a/compiler/rustc_query_impl/src/on_disk_cache.rs b/compiler/rustc_query_impl/src/on_disk_cache.rs index d7327ca4bc602..0fb1d72829361 100644 --- a/compiler/rustc_query_impl/src/on_disk_cache.rs +++ b/compiler/rustc_query_impl/src/on_disk_cache.rs @@ -25,6 +25,7 @@ use rustc_span::hygiene::{ use rustc_span::source_map::{SourceMap, StableSourceFileId}; use rustc_span::CachingSourceMapView; use rustc_span::{BytePos, ExpnData, ExpnHash, Pos, SourceFile, Span}; +use std::io; use std::mem; const TAG_FILE_FOOTER: u128 = 0xC0FFEE_C0FFEE_C0FFEE_C0FFEE_C0FFEE; @@ -807,21 +808,10 @@ impl_ref_decoder! {<'tcx> //- ENCODING ------------------------------------------------------------------- -pub trait OpaqueEncoder: Encoder { - fn position(&self) -> usize; -} - -impl OpaqueEncoder for FileEncoder { - #[inline] - fn position(&self) -> usize { - FileEncoder::position(self) - } -} - /// An encoder that can write to the incremental compilation cache. -pub struct CacheEncoder<'a, 'tcx, E: OpaqueEncoder> { +pub struct CacheEncoder<'a, 'tcx> { tcx: TyCtxt<'tcx>, - encoder: E, + encoder: FileEncoder, type_shorthands: FxHashMap, usize>, predicate_shorthands: FxHashMap, usize>, interpret_allocs: FxIndexSet, @@ -830,10 +820,7 @@ pub struct CacheEncoder<'a, 'tcx, E: OpaqueEncoder> { hygiene_context: &'a HygieneEncodeContext, } -impl<'a, 'tcx, E> CacheEncoder<'a, 'tcx, E> -where - E: OpaqueEncoder, -{ +impl<'a, 'tcx> CacheEncoder<'a, 'tcx> { fn source_file_index(&mut self, source_file: Lrc) -> SourceFileIndex { self.file_to_file_index[&(&*source_file as *const SourceFile)] } @@ -852,32 +839,27 @@ where let end_pos = self.position(); ((end_pos - start_pos) as u64).encode(self); } + + fn finish(self) -> Result { + self.encoder.finish() + } } -impl<'a, 'tcx, E> Encodable> for SyntaxContext -where - E: OpaqueEncoder, -{ - fn encode(&self, s: &mut CacheEncoder<'a, 'tcx, E>) { +impl<'a, 'tcx> Encodable> for SyntaxContext { + fn encode(&self, s: &mut CacheEncoder<'a, 'tcx>) { rustc_span::hygiene::raw_encode_syntax_context(*self, s.hygiene_context, s); } } -impl<'a, 'tcx, E> Encodable> for ExpnId -where - E: OpaqueEncoder, -{ - fn encode(&self, s: &mut CacheEncoder<'a, 'tcx, E>) { +impl<'a, 'tcx> Encodable> for ExpnId { + fn encode(&self, s: &mut CacheEncoder<'a, 'tcx>) { s.hygiene_context.schedule_expn_data_for_encoding(*self); self.expn_hash().encode(s); } } -impl<'a, 'tcx, E> Encodable> for Span -where - E: OpaqueEncoder, -{ - fn encode(&self, s: &mut CacheEncoder<'a, 'tcx, E>) { +impl<'a, 'tcx> Encodable> for Span { + fn encode(&self, s: &mut CacheEncoder<'a, 'tcx>) { let span_data = self.data_untracked(); span_data.ctxt.encode(s); span_data.parent.encode(s); @@ -920,10 +902,7 @@ where } } -impl<'a, 'tcx, E> TyEncoder for CacheEncoder<'a, 'tcx, E> -where - E: OpaqueEncoder, -{ +impl<'a, 'tcx> TyEncoder for CacheEncoder<'a, 'tcx> { type I = TyCtxt<'tcx>; const CLEAR_CROSS_CRATE: bool = false; @@ -943,29 +922,20 @@ where } } -impl<'a, 'tcx, E> Encodable> for CrateNum -where - E: OpaqueEncoder, -{ - fn encode(&self, s: &mut CacheEncoder<'a, 'tcx, E>) { +impl<'a, 'tcx> Encodable> for CrateNum { + fn encode(&self, s: &mut CacheEncoder<'a, 'tcx>) { s.tcx.stable_crate_id(*self).encode(s); } } -impl<'a, 'tcx, E> Encodable> for DefId -where - E: OpaqueEncoder, -{ - fn encode(&self, s: &mut CacheEncoder<'a, 'tcx, E>) { +impl<'a, 'tcx> Encodable> for DefId { + fn encode(&self, s: &mut CacheEncoder<'a, 'tcx>) { s.tcx.def_path_hash(*self).encode(s); } } -impl<'a, 'tcx, E> Encodable> for DefIndex -where - E: OpaqueEncoder, -{ - fn encode(&self, _: &mut CacheEncoder<'a, 'tcx, E>) { +impl<'a, 'tcx> Encodable> for DefIndex { + fn encode(&self, _: &mut CacheEncoder<'a, 'tcx>) { bug!("encoding `DefIndex` without context"); } } @@ -979,13 +949,7 @@ macro_rules! encoder_methods { } } -impl<'a, 'tcx, E> Encoder for CacheEncoder<'a, 'tcx, E> -where - E: OpaqueEncoder, -{ - type Ok = E::Ok; - type Err = E::Err; - +impl<'a, 'tcx> Encoder for CacheEncoder<'a, 'tcx> { encoder_methods! { emit_usize(usize); emit_u128(u128); @@ -1008,30 +972,26 @@ where emit_str(&str); emit_raw_bytes(&[u8]); } - - fn finish(self) -> Result { - self.encoder.finish() - } } // This ensures that the `Encodable::encode` specialization for byte slices // is used when a `CacheEncoder` having an `opaque::FileEncoder` is passed to `Encodable::encode`. // Unfortunately, we have to manually opt into specializations this way, given how `CacheEncoder` // and the encoding traits currently work. -impl<'a, 'tcx> Encodable> for [u8] { - fn encode(&self, e: &mut CacheEncoder<'a, 'tcx, FileEncoder>) { +impl<'a, 'tcx> Encodable> for [u8] { + fn encode(&self, e: &mut CacheEncoder<'a, 'tcx>) { self.encode(&mut e.encoder); } } pub fn encode_query_results<'a, 'tcx, CTX, Q>( tcx: CTX, - encoder: &mut CacheEncoder<'a, 'tcx, FileEncoder>, + encoder: &mut CacheEncoder<'a, 'tcx>, query_result_index: &mut EncodedDepNodeIndex, ) where CTX: QueryContext + 'tcx, Q: super::QueryDescription, - Q::Value: Encodable>, + Q::Value: Encodable>, { let _timer = tcx .dep_context() diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 87aedc6542d8d..66f4508f6b497 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -12,7 +12,6 @@ use rustc_query_system::query::{QueryContext, QueryJobId, QueryMap, QuerySideEff use rustc_data_structures::sync::Lock; use rustc_data_structures::thin_vec::ThinVec; use rustc_errors::{Diagnostic, Handler}; -use rustc_serialize::opaque; use std::any::Any; use std::num::NonZeroU64; @@ -140,7 +139,7 @@ impl<'tcx> QueryCtxt<'tcx> { pub(super) fn encode_query_results( self, - encoder: &mut on_disk_cache::CacheEncoder<'_, 'tcx, opaque::FileEncoder>, + encoder: &mut on_disk_cache::CacheEncoder<'_, 'tcx>, query_result_index: &mut on_disk_cache::EncodedDepNodeIndex, ) { macro_rules! encode_queries { diff --git a/compiler/rustc_query_system/src/dep_graph/serialized.rs b/compiler/rustc_query_system/src/dep_graph/serialized.rs index d583b45698ac4..3b20ec70d73cb 100644 --- a/compiler/rustc_query_system/src/dep_graph/serialized.rs +++ b/compiler/rustc_query_system/src/dep_graph/serialized.rs @@ -20,7 +20,7 @@ use rustc_data_structures::profiling::SelfProfilerRef; use rustc_data_structures::sync::Lock; use rustc_index::vec::{Idx, IndexVec}; use rustc_serialize::opaque::{FileEncodeResult, FileEncoder, IntEncodedWithFixedSize, MemDecoder}; -use rustc_serialize::{Decodable, Decoder, Encodable, Encoder}; +use rustc_serialize::{Decodable, Decoder, Encodable}; use smallvec::SmallVec; use std::convert::TryInto; diff --git a/compiler/rustc_serialize/src/opaque.rs b/compiler/rustc_serialize/src/opaque.rs index f828c0b71063d..366efe9cfa519 100644 --- a/compiler/rustc_serialize/src/opaque.rs +++ b/compiler/rustc_serialize/src/opaque.rs @@ -24,6 +24,10 @@ impl MemEncoder { pub fn position(&self) -> usize { self.data.len() } + + pub fn finish(self) -> Vec { + self.data + } } macro_rules! write_leb128 { @@ -54,9 +58,6 @@ macro_rules! write_leb128 { const STR_SENTINEL: u8 = 0xC1; impl Encoder for MemEncoder { - type Ok = Vec; - type Err = !; - #[inline] fn emit_usize(&mut self, v: usize) { write_leb128!(self, v, usize, write_usize_leb128) @@ -150,10 +151,6 @@ impl Encoder for MemEncoder { fn emit_raw_bytes(&mut self, s: &[u8]) { self.data.extend_from_slice(s); } - - fn finish(self) -> Result { - Ok(self.data) - } } pub type FileEncodeResult = Result; @@ -389,6 +386,13 @@ impl FileEncoder { } } } + + pub fn finish(mut self) -> Result { + self.flush(); + + let res = std::mem::replace(&mut self.res, Ok(())); + res.map(|()| self.position()) + } } impl Drop for FileEncoder { @@ -426,9 +430,6 @@ macro_rules! file_encoder_write_leb128 { } impl Encoder for FileEncoder { - type Ok = usize; - type Err = io::Error; - #[inline] fn emit_usize(&mut self, v: usize) { file_encoder_write_leb128!(self, v, usize, write_usize_leb128) @@ -522,13 +523,6 @@ impl Encoder for FileEncoder { fn emit_raw_bytes(&mut self, s: &[u8]) { self.write_all(s); } - - fn finish(mut self) -> Result { - self.flush(); - - let res = std::mem::replace(&mut self.res, Ok(())); - res.map(|()| self.position()) - } } // ----------------------------------------------------------------------------- diff --git a/compiler/rustc_serialize/src/serialize.rs b/compiler/rustc_serialize/src/serialize.rs index 98bb18581f517..36585b8d77e0a 100644 --- a/compiler/rustc_serialize/src/serialize.rs +++ b/compiler/rustc_serialize/src/serialize.rs @@ -18,13 +18,10 @@ use std::sync::Arc; /// is pervasive and has non-trivial cost. Instead, impls of this trait must /// implement a delayed error handling strategy. If a failure occurs, they /// should record this internally, and all subsequent encoding operations can -/// be processed or ignored, whichever is appropriate. Then when `finish()` is -/// called, an error result should be returned to indicate the failure. If no -/// failures occurred, then `finish()` should return a success result. +/// be processed or ignored, whichever is appropriate. Then they should provide +/// a `finish` method that finishes up encoding. If the encoder is fallible, +/// `finish` should return a `Result` that indicates success or failure. pub trait Encoder { - type Ok; - type Err; - // Primitive types: fn emit_usize(&mut self, v: usize); fn emit_u128(&mut self, v: u128); @@ -64,9 +61,6 @@ pub trait Encoder { fn emit_fieldless_enum_variant(&mut self) { self.emit_usize(ID) } - - // Consume the encoder, getting the result. - fn finish(self) -> Result; } // Note: all the methods in this trait are infallible, which may be surprising. diff --git a/compiler/rustc_serialize/tests/opaque.rs b/compiler/rustc_serialize/tests/opaque.rs index 4eafb6fabde8e..3a695d0714ee1 100644 --- a/compiler/rustc_serialize/tests/opaque.rs +++ b/compiler/rustc_serialize/tests/opaque.rs @@ -2,7 +2,7 @@ use rustc_macros::{Decodable, Encodable}; use rustc_serialize::opaque::{MemDecoder, MemEncoder}; -use rustc_serialize::{Decodable, Encodable, Encoder as EncoderTrait}; +use rustc_serialize::{Decodable, Encodable}; use std::fmt::Debug; #[derive(PartialEq, Clone, Debug, Encodable, Decodable)] @@ -38,7 +38,7 @@ fn check_round_trip< Encodable::encode(value, &mut encoder); } - let data = encoder.finish().unwrap(); + let data = encoder.finish(); let mut decoder = MemDecoder::new(&data[..], 0); for value in values { diff --git a/src/librustdoc/scrape_examples.rs b/src/librustdoc/scrape_examples.rs index da09ae9dd0662..f6c599297fcd6 100644 --- a/src/librustdoc/scrape_examples.rs +++ b/src/librustdoc/scrape_examples.rs @@ -18,7 +18,7 @@ use rustc_middle::hir::nested_filter; use rustc_middle::ty::{self, TyCtxt}; use rustc_serialize::{ opaque::{FileEncoder, MemDecoder}, - Decodable, Encodable, Encoder, + Decodable, Encodable, }; use rustc_session::getopts; use rustc_span::{ diff --git a/src/test/ui-fulldeps/deriving-encodable-decodable-box.rs b/src/test/ui-fulldeps/deriving-encodable-decodable-box.rs index 4ad4ef60a5294..a4b911878e0d6 100644 --- a/src/test/ui-fulldeps/deriving-encodable-decodable-box.rs +++ b/src/test/ui-fulldeps/deriving-encodable-decodable-box.rs @@ -20,7 +20,7 @@ fn main() { let mut encoder = MemEncoder::new(); obj.encode(&mut encoder); - let data = encoder.finish().unwrap(); + let data = encoder.finish(); let mut decoder = MemDecoder::new(&data, 0); let obj2 = A::decode(&mut decoder); diff --git a/src/test/ui-fulldeps/deriving-encodable-decodable-cell-refcell.rs b/src/test/ui-fulldeps/deriving-encodable-decodable-cell-refcell.rs index 3ac3abae692af..580c85f9b7848 100644 --- a/src/test/ui-fulldeps/deriving-encodable-decodable-cell-refcell.rs +++ b/src/test/ui-fulldeps/deriving-encodable-decodable-cell-refcell.rs @@ -29,7 +29,7 @@ fn main() { let mut encoder = MemEncoder::new(); obj.encode(&mut encoder); - let data = encoder.finish().unwrap(); + let data = encoder.finish(); let mut decoder = MemDecoder::new(&data, 0); let obj2 = B::decode(&mut decoder); diff --git a/src/test/ui-fulldeps/issue-14021.rs b/src/test/ui-fulldeps/issue-14021.rs index b7b6e1b860d35..215dfaed7abbe 100644 --- a/src/test/ui-fulldeps/issue-14021.rs +++ b/src/test/ui-fulldeps/issue-14021.rs @@ -19,7 +19,7 @@ pub fn main() { let mut encoder = MemEncoder::new(); obj.encode(&mut encoder); - let data = encoder.finish().unwrap(); + let data = encoder.finish(); let mut decoder = MemDecoder::new(&data, 0); let obj2 = UnitLikeStruct::decode(&mut decoder);