Skip to content

Commit 0423e06

Browse files
borsbors
committed
Auto merge of #98160 - nnethercote:mv-finish-out-of-Encoder, r=bjorn3
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.) r? `@ghost`
2 parents 3cf1275 + bb02cc4 commit 0423e06

File tree

13 files changed

+51
-111
lines changed

13 files changed

+51
-111
lines changed

‎compiler/rustc_codegen_ssa/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ impl CodegenResults {
211211
encoder.emit_raw_bytes(&RLINK_VERSION.to_be_bytes());
212212
encoder.emit_str(RUSTC_VERSION.unwrap());
213213
Encodable::encode(codegen_results, &mut encoder);
214-
encoder.finish().unwrap()
214+
encoder.finish()
215215
}
216216

217217
pub fn deserialize_rlink(data: Vec<u8>) -> Result<Self, String> {

‎compiler/rustc_incremental/src/persist/save.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use rustc_data_structures::sync::join;
33
use rustc_middle::dep_graph::{DepGraph, SerializedDepGraph, WorkProduct, WorkProductId};
44
use rustc_middle::ty::TyCtxt;
55
use rustc_serialize::opaque::{FileEncodeResult, FileEncoder};
6-
use rustc_serialize::{Encodable as RustcEncodable, Encoder};
6+
use rustc_serialize::Encodable as RustcEncodable;
77
use rustc_session::Session;
88
use std::fs;
99

‎compiler/rustc_metadata/src/rmeta/encoder.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,6 @@ macro_rules! encoder_methods {
9494
}
9595

9696
impl<'a, 'tcx> Encoder for EncodeContext<'a, 'tcx> {
97-
type Ok = <MemEncoder as Encoder>::Ok;
98-
type Err = <MemEncoder as Encoder>::Err;
99-
10097
encoder_methods! {
10198
emit_usize(usize);
10299
emit_u128(u128);
@@ -119,10 +116,6 @@ impl<'a, 'tcx> Encoder for EncodeContext<'a, 'tcx> {
119116
emit_str(&str);
120117
emit_raw_bytes(&[u8]);
121118
}
122-
123-
fn finish(self) -> Result<Self::Ok, Self::Err> {
124-
self.opaque.finish()
125-
}
126119
}
127120

128121
impl<'a, 'tcx, T> Encodable<EncodeContext<'a, 'tcx>> for LazyValue<T> {
@@ -2216,7 +2209,7 @@ fn encode_metadata_impl(tcx: TyCtxt<'_>) -> EncodedMetadata {
22162209
// culminating in the `CrateRoot` which points to all of it.
22172210
let root = ecx.encode_crate_root();
22182211

2219-
let mut result = ecx.opaque.finish().unwrap();
2212+
let mut result = ecx.opaque.finish();
22202213

22212214
// Encode the root position.
22222215
let header = METADATA_HEADER.len();

‎compiler/rustc_query_impl/src/on_disk_cache.rs

Lines changed: 26 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use rustc_span::hygiene::{
2525
use rustc_span::source_map::{SourceMap, StableSourceFileId};
2626
use rustc_span::CachingSourceMapView;
2727
use rustc_span::{BytePos, ExpnData, ExpnHash, Pos, SourceFile, Span};
28+
use std::io;
2829
use std::mem;
2930

3031
const TAG_FILE_FOOTER: u128 = 0xC0FFEE_C0FFEE_C0FFEE_C0FFEE_C0FFEE;
@@ -807,21 +808,10 @@ impl_ref_decoder! {<'tcx>
807808

808809
//- ENCODING -------------------------------------------------------------------
809810

810-
pub trait OpaqueEncoder: Encoder {
811-
fn position(&self) -> usize;
812-
}
813-
814-
impl OpaqueEncoder for FileEncoder {
815-
#[inline]
816-
fn position(&self) -> usize {
817-
FileEncoder::position(self)
818-
}
819-
}
820-
821811
/// An encoder that can write to the incremental compilation cache.
822-
pub struct CacheEncoder<'a, 'tcx, E: OpaqueEncoder> {
812+
pub struct CacheEncoder<'a, 'tcx> {
823813
tcx: TyCtxt<'tcx>,
824-
encoder: E,
814+
encoder: FileEncoder,
825815
type_shorthands: FxHashMap<Ty<'tcx>, usize>,
826816
predicate_shorthands: FxHashMap<ty::PredicateKind<'tcx>, usize>,
827817
interpret_allocs: FxIndexSet<interpret::AllocId>,
@@ -830,10 +820,7 @@ pub struct CacheEncoder<'a, 'tcx, E: OpaqueEncoder> {
830820
hygiene_context: &'a HygieneEncodeContext,
831821
}
832822

833-
impl<'a, 'tcx, E> CacheEncoder<'a, 'tcx, E>
834-
where
835-
E: OpaqueEncoder,
836-
{
823+
impl<'a, 'tcx> CacheEncoder<'a, 'tcx> {
837824
fn source_file_index(&mut self, source_file: Lrc<SourceFile>) -> SourceFileIndex {
838825
self.file_to_file_index[&(&*source_file as *const SourceFile)]
839826
}
@@ -852,32 +839,27 @@ where
852839
let end_pos = self.position();
853840
((end_pos - start_pos) as u64).encode(self);
854841
}
842+
843+
fn finish(self) -> Result<usize, io::Error> {
844+
self.encoder.finish()
845+
}
855846
}
856847

857-
impl<'a, 'tcx, E> Encodable<CacheEncoder<'a, 'tcx, E>> for SyntaxContext
858-
where
859-
E: OpaqueEncoder,
860-
{
861-
fn encode(&self, s: &mut CacheEncoder<'a, 'tcx, E>) {
848+
impl<'a, 'tcx> Encodable<CacheEncoder<'a, 'tcx>> for SyntaxContext {
849+
fn encode(&self, s: &mut CacheEncoder<'a, 'tcx>) {
862850
rustc_span::hygiene::raw_encode_syntax_context(*self, s.hygiene_context, s);
863851
}
864852
}
865853

866-
impl<'a, 'tcx, E> Encodable<CacheEncoder<'a, 'tcx, E>> for ExpnId
867-
where
868-
E: OpaqueEncoder,
869-
{
870-
fn encode(&self, s: &mut CacheEncoder<'a, 'tcx, E>) {
854+
impl<'a, 'tcx> Encodable<CacheEncoder<'a, 'tcx>> for ExpnId {
855+
fn encode(&self, s: &mut CacheEncoder<'a, 'tcx>) {
871856
s.hygiene_context.schedule_expn_data_for_encoding(*self);
872857
self.expn_hash().encode(s);
873858
}
874859
}
875860

876-
impl<'a, 'tcx, E> Encodable<CacheEncoder<'a, 'tcx, E>> for Span
877-
where
878-
E: OpaqueEncoder,
879-
{
880-
fn encode(&self, s: &mut CacheEncoder<'a, 'tcx, E>) {
861+
impl<'a, 'tcx> Encodable<CacheEncoder<'a, 'tcx>> for Span {
862+
fn encode(&self, s: &mut CacheEncoder<'a, 'tcx>) {
881863
let span_data = self.data_untracked();
882864
span_data.ctxt.encode(s);
883865
span_data.parent.encode(s);
@@ -920,10 +902,7 @@ where
920902
}
921903
}
922904

923-
impl<'a, 'tcx, E> TyEncoder for CacheEncoder<'a, 'tcx, E>
924-
where
925-
E: OpaqueEncoder,
926-
{
905+
impl<'a, 'tcx> TyEncoder for CacheEncoder<'a, 'tcx> {
927906
type I = TyCtxt<'tcx>;
928907
const CLEAR_CROSS_CRATE: bool = false;
929908

@@ -943,29 +922,20 @@ where
943922
}
944923
}
945924

946-
impl<'a, 'tcx, E> Encodable<CacheEncoder<'a, 'tcx, E>> for CrateNum
947-
where
948-
E: OpaqueEncoder,
949-
{
950-
fn encode(&self, s: &mut CacheEncoder<'a, 'tcx, E>) {
925+
impl<'a, 'tcx> Encodable<CacheEncoder<'a, 'tcx>> for CrateNum {
926+
fn encode(&self, s: &mut CacheEncoder<'a, 'tcx>) {
951927
s.tcx.stable_crate_id(*self).encode(s);
952928
}
953929
}
954930

955-
impl<'a, 'tcx, E> Encodable<CacheEncoder<'a, 'tcx, E>> for DefId
956-
where
957-
E: OpaqueEncoder,
958-
{
959-
fn encode(&self, s: &mut CacheEncoder<'a, 'tcx, E>) {
931+
impl<'a, 'tcx> Encodable<CacheEncoder<'a, 'tcx>> for DefId {
932+
fn encode(&self, s: &mut CacheEncoder<'a, 'tcx>) {
960933
s.tcx.def_path_hash(*self).encode(s);
961934
}
962935
}
963936

964-
impl<'a, 'tcx, E> Encodable<CacheEncoder<'a, 'tcx, E>> for DefIndex
965-
where
966-
E: OpaqueEncoder,
967-
{
968-
fn encode(&self, _: &mut CacheEncoder<'a, 'tcx, E>) {
937+
impl<'a, 'tcx> Encodable<CacheEncoder<'a, 'tcx>> for DefIndex {
938+
fn encode(&self, _: &mut CacheEncoder<'a, 'tcx>) {
969939
bug!("encoding `DefIndex` without context");
970940
}
971941
}
@@ -979,13 +949,7 @@ macro_rules! encoder_methods {
979949
}
980950
}
981951

982-
impl<'a, 'tcx, E> Encoder for CacheEncoder<'a, 'tcx, E>
983-
where
984-
E: OpaqueEncoder,
985-
{
986-
type Ok = E::Ok;
987-
type Err = E::Err;
988-
952+
impl<'a, 'tcx> Encoder for CacheEncoder<'a, 'tcx> {
989953
encoder_methods! {
990954
emit_usize(usize);
991955
emit_u128(u128);
@@ -1008,30 +972,26 @@ where
1008972
emit_str(&str);
1009973
emit_raw_bytes(&[u8]);
1010974
}
1011-
1012-
fn finish(self) -> Result<E::Ok, E::Err> {
1013-
self.encoder.finish()
1014-
}
1015975
}
1016976

1017977
// This ensures that the `Encodable<opaque::FileEncoder>::encode` specialization for byte slices
1018978
// is used when a `CacheEncoder` having an `opaque::FileEncoder` is passed to `Encodable::encode`.
1019979
// Unfortunately, we have to manually opt into specializations this way, given how `CacheEncoder`
1020980
// and the encoding traits currently work.
1021-
impl<'a, 'tcx> Encodable<CacheEncoder<'a, 'tcx, FileEncoder>> for [u8] {
1022-
fn encode(&self, e: &mut CacheEncoder<'a, 'tcx, FileEncoder>) {
981+
impl<'a, 'tcx> Encodable<CacheEncoder<'a, 'tcx>> for [u8] {
982+
fn encode(&self, e: &mut CacheEncoder<'a, 'tcx>) {
1023983
self.encode(&mut e.encoder);
1024984
}
1025985
}
1026986

1027987
pub fn encode_query_results<'a, 'tcx, CTX, Q>(
1028988
tcx: CTX,
1029-
encoder: &mut CacheEncoder<'a, 'tcx, FileEncoder>,
989+
encoder: &mut CacheEncoder<'a, 'tcx>,
1030990
query_result_index: &mut EncodedDepNodeIndex,
1031991
) where
1032992
CTX: QueryContext + 'tcx,
1033993
Q: super::QueryDescription<CTX>,
1034-
Q::Value: Encodable<CacheEncoder<'a, 'tcx, FileEncoder>>,
994+
Q::Value: Encodable<CacheEncoder<'a, 'tcx>>,
1035995
{
1036996
let _timer = tcx
1037997
.dep_context()

‎compiler/rustc_query_impl/src/plumbing.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use rustc_query_system::query::{QueryContext, QueryJobId, QueryMap, QuerySideEff
1212
use rustc_data_structures::sync::Lock;
1313
use rustc_data_structures::thin_vec::ThinVec;
1414
use rustc_errors::{Diagnostic, Handler};
15-
use rustc_serialize::opaque;
1615

1716
use std::any::Any;
1817
use std::num::NonZeroU64;
@@ -140,7 +139,7 @@ impl<'tcx> QueryCtxt<'tcx> {
140139

141140
pub(super) fn encode_query_results(
142141
self,
143-
encoder: &mut on_disk_cache::CacheEncoder<'_, 'tcx, opaque::FileEncoder>,
142+
encoder: &mut on_disk_cache::CacheEncoder<'_, 'tcx>,
144143
query_result_index: &mut on_disk_cache::EncodedDepNodeIndex,
145144
) {
146145
macro_rules! encode_queries {

‎compiler/rustc_query_system/src/dep_graph/serialized.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use rustc_data_structures::profiling::SelfProfilerRef;
2020
use rustc_data_structures::sync::Lock;
2121
use rustc_index::vec::{Idx, IndexVec};
2222
use rustc_serialize::opaque::{FileEncodeResult, FileEncoder, IntEncodedWithFixedSize, MemDecoder};
23-
use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
23+
use rustc_serialize::{Decodable, Decoder, Encodable};
2424
use smallvec::SmallVec;
2525
use std::convert::TryInto;
2626

‎compiler/rustc_serialize/src/opaque.rs

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ impl MemEncoder {
2424
pub fn position(&self) -> usize {
2525
self.data.len()
2626
}
27+
28+
pub fn finish(self) -> Vec<u8> {
29+
self.data
30+
}
2731
}
2832

2933
macro_rules! write_leb128 {
@@ -54,9 +58,6 @@ macro_rules! write_leb128 {
5458
const STR_SENTINEL: u8 = 0xC1;
5559

5660
impl Encoder for MemEncoder {
57-
type Ok = Vec<u8>;
58-
type Err = !;
59-
6061
#[inline]
6162
fn emit_usize(&mut self, v: usize) {
6263
write_leb128!(self, v, usize, write_usize_leb128)
@@ -150,10 +151,6 @@ impl Encoder for MemEncoder {
150151
fn emit_raw_bytes(&mut self, s: &[u8]) {
151152
self.data.extend_from_slice(s);
152153
}
153-
154-
fn finish(self) -> Result<Self::Ok, Self::Err> {
155-
Ok(self.data)
156-
}
157154
}
158155

159156
pub type FileEncodeResult = Result<usize, io::Error>;
@@ -389,6 +386,13 @@ impl FileEncoder {
389386
}
390387
}
391388
}
389+
390+
pub fn finish(mut self) -> Result<usize, io::Error> {
391+
self.flush();
392+
393+
let res = std::mem::replace(&mut self.res, Ok(()));
394+
res.map(|()| self.position())
395+
}
392396
}
393397

394398
impl Drop for FileEncoder {
@@ -426,9 +430,6 @@ macro_rules! file_encoder_write_leb128 {
426430
}
427431

428432
impl Encoder for FileEncoder {
429-
type Ok = usize;
430-
type Err = io::Error;
431-
432433
#[inline]
433434
fn emit_usize(&mut self, v: usize) {
434435
file_encoder_write_leb128!(self, v, usize, write_usize_leb128)
@@ -522,13 +523,6 @@ impl Encoder for FileEncoder {
522523
fn emit_raw_bytes(&mut self, s: &[u8]) {
523524
self.write_all(s);
524525
}
525-
526-
fn finish(mut self) -> Result<usize, io::Error> {
527-
self.flush();
528-
529-
let res = std::mem::replace(&mut self.res, Ok(()));
530-
res.map(|()| self.position())
531-
}
532526
}
533527

534528
// -----------------------------------------------------------------------------

‎compiler/rustc_serialize/src/serialize.rs

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,10 @@ use std::sync::Arc;
1818
/// is pervasive and has non-trivial cost. Instead, impls of this trait must
1919
/// implement a delayed error handling strategy. If a failure occurs, they
2020
/// should record this internally, and all subsequent encoding operations can
21-
/// be processed or ignored, whichever is appropriate. Then when `finish()` is
22-
/// called, an error result should be returned to indicate the failure. If no
23-
/// failures occurred, then `finish()` should return a success result.
21+
/// be processed or ignored, whichever is appropriate. Then they should provide
22+
/// a `finish` method that finishes up encoding. If the encoder is fallible,
23+
/// `finish` should return a `Result` that indicates success or failure.
2424
pub trait Encoder {
25-
type Ok;
26-
type Err;
27-
2825
// Primitive types:
2926
fn emit_usize(&mut self, v: usize);
3027
fn emit_u128(&mut self, v: u128);
@@ -64,9 +61,6 @@ pub trait Encoder {
6461
fn emit_fieldless_enum_variant<const ID: usize>(&mut self) {
6562
self.emit_usize(ID)
6663
}
67-
68-
// Consume the encoder, getting the result.
69-
fn finish(self) -> Result<Self::Ok, Self::Err>;
7064
}
7165

7266
// Note: all the methods in this trait are infallible, which may be surprising.

‎compiler/rustc_serialize/tests/opaque.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
use rustc_macros::{Decodable, Encodable};
44
use rustc_serialize::opaque::{MemDecoder, MemEncoder};
5-
use rustc_serialize::{Decodable, Encodable, Encoder as EncoderTrait};
5+
use rustc_serialize::{Decodable, Encodable};
66
use std::fmt::Debug;
77

88
#[derive(PartialEq, Clone, Debug, Encodable, Decodable)]
@@ -38,7 +38,7 @@ fn check_round_trip<
3838
Encodable::encode(value, &mut encoder);
3939
}
4040

41-
let data = encoder.finish().unwrap();
41+
let data = encoder.finish();
4242
let mut decoder = MemDecoder::new(&data[..], 0);
4343

4444
for value in values {

‎src/librustdoc/scrape_examples.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use rustc_middle::hir::nested_filter;
1818
use rustc_middle::ty::{self, TyCtxt};
1919
use rustc_serialize::{
2020
opaque::{FileEncoder, MemDecoder},
21-
Decodable, Encodable, Encoder,
21+
Decodable, Encodable,
2222
};
2323
use rustc_session::getopts;
2424
use rustc_span::{

‎src/test/ui-fulldeps/deriving-encodable-decodable-box.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ fn main() {
2020

2121
let mut encoder = MemEncoder::new();
2222
obj.encode(&mut encoder);
23-
let data = encoder.finish().unwrap();
23+
let data = encoder.finish();
2424

2525
let mut decoder = MemDecoder::new(&data, 0);
2626
let obj2 = A::decode(&mut decoder);

0 commit comments

Comments
 (0)