31 KiB
Generating sync methods from async ones — migration plan
SharpCompress hand-maintains a sync twin beside most async methods, usually split as Foo.cs +
Foo.Async.cs. That means every bug fix has to be made twice, and they drift: the extraction
overloads converted in commit c303856c had already diverged (the async path honoured
ExtractionOptions.BufferSize, the sync path did not).
Zomp.SyncMethodGenerator removes the duplication
by generating the sync method from the async one. It is already referenced for every project
(Directory.Packages.props, GlobalPackageReference), so it runs in all six TFMs with no per-project
setup.
A normalised scan (strip await, .ConfigureAwait(false), the Async suffix, CancellationToken;
map Task<T>→T) over ~130 .cs/.Async.cs pairs found 626 XAsync/X method pairs, 298 of
them byte-identical and another 147 at ≥85% similarity — roughly 2,100 deletable lines.
This document is the execution plan for the remaining work. The conventions themselves are
summarised in AGENTS.md ("Generating sync methods from async ones"); this file is the batch list,
the verification workflow, and the record of what is blocked and why.
Status
| Batch | Area | Status |
|---|---|---|
| 0 | Archives/IArchiveEntryExtensions (5 methods) |
done — c303856c |
| 1 | Compressors/Filters/Filter + 6 XZ branch filters + Lzma2Filter (9 methods, 147 lines) |
done — verified identical on net48 + net10.0 |
| 2 | IO/ leaf stream shims + Common/EntryStream (17 methods, 190 lines) |
done — verified identical on net48 + net10.0 |
| 3–5 | below | to do |
| 6 | ZIP parts & header factories | 6a done; header factories remain |
| 7 | Concrete writers | ZIP done; Tar, SevenZip, and remaining writers to do |
| 8 | 7-Zip | to do |
| 9 | Streaming compressors | Deflate, Zlib, and Deflate64 done; remaining compressors to do |
| 10–11 | below | to do |
| 12 | Providers | registry done; provider implementations remain |
| Rar reader unification | below | to do, needs its own design review |
The recipe
Proven on batch 1. Per file pair:
- Map the signature by hand for each async method: drop the
Asyncsuffix;Task/ValueTask→void,Task<T>/ValueTask<T>→T; dropCancellationTokenandIProgress<T>(unlessPreserveCancellationToken/PreserveProgress);Memory<T>→Span<T>,ReadOnlyMemory<T>→ReadOnlySpan<T>; modifiers (public,override,virtual,static,sealed) copied verbatim. - Attribute only when that mapped signature already exists by hand. Generating a member that
did not exist before is a behaviour change, not a deduplication — a generated
Read(Span<byte>)replacesStream's default rent-and-copy shim. Keep those out of a deletion batch; if they are wanted, add them in a separate, clearly-labelled commit. - Add
[Zomp.SyncMethodGenerator.CreateSyncVersion]per method, never on the type. A type-level attribute also generates the members that must not exist, so you would need more[SkipSyncVersion]than[CreateSyncVersion]. - Delete exactly those sync twins. Missing one is
CS0111on every TFM — lean on that. - Remove usings that the deleted sync body was the only consumer of (batch 1:
using SharpCompress.Compressors.Filters;in each branch filter's.cs). - Keep the class
partial, keep the async method where it lives, and do not renameFoo.Async.cs— renames destroygit blame/--followon exactly the algorithm code that most needs history, and the name becomes more accurate after conversion (what remains in it is genuinely async-only). DeleteFoo.Async.csonly if it ends up empty. - XML docs: keep one comment, on the async method, phrased tense-neutrally ("Extract entry to the specified stream.") — it is emitted onto both copies. If the two comments disagree on substance, that is a converge item, not something to resolve silently while deleting.
dotnet csharpier format .—check-formatis the first step of the default build target.
One commit per concern, at most two per batch: commit 1 "converge" (behaviour alignment, itemised), commit 2 "generate" (attribute + delete, provably no-op). Never mix them — mixing destroys the "expected diff is empty" invariant that makes these PRs reviewable.
Keep each batch to one cohesive area, ≤ ~12 attributed methods or ≤ ~400 deleted lines.
Verification
1. Prove the generated code equals the deleted code
The generator only sees the active syntax for the current compilation — code inside
#if !LEGACY_DOTNET is disabled trivia on net48/ns2.0/ns2.1 — so check both #if worlds:
foreach ($tfm in 'net48','net10.0') {
dotnet build src/SharpCompress/SharpCompress.csproj -c Release -f $tfm -p:EmitCompilerGeneratedFiles=true
}
Generated source lands in
src/SharpCompress/obj/Release/<tfm>/generated/Zomp.SyncMethodGenerator/Zomp.SyncMethodGenerator.SyncMethodSourceGenerator/<Namespace>.<Type>.<Method>.g.cs
— already TFM-scoped, already gitignored, and invisible to csharpier.
For each attributed method, compare the generated body against the body deleted from a baseline commit
(git show <rev>:<path>). Batches 1 and 2 were verified this way with a throwaway Python script that
walks the generated directory, parses each generated declaration, finds the same-signature method in
the baseline and diffs the bodies — 48/48 identical across both TFMs. Expected result is an empty
diff; itemise any non-empty hunk in the commit message.
Four things that will otherwise produce false results, all learned the hard way:
obj/is not cleaned by an incremental build. If nothing changed the generator does not re-run, so you compare against stale.g.csfrom a previous attempt. Use-t:Rebuild, or deleteobj/Release/<tfm>/generatedfirst.- The baseline is per batch. Once a batch is committed
HEADno longer contains the code it deleted, so earlier batches must be compared against the commit before that batch. - The generator fully qualifies types. It emits
new global::System.ObjectDisposedException(...), so stripglobal::and type qualifiers when canonicalising before comparison. Version 2.0.42 preserves fluent-call layout, so no reflow-specific normalisation is needed. - Extension invocations become static calls.
this.Skip()is emitted asStreamExtensions.Skip(this). Canonicalise one form to the other before comparing.
A generated method whose signature is absent from the baseline is the important signal: the batch created a new member instead of replacing a duplicate. Report those separately and fail on them.
If a dump has to survive clean, override CompilerGeneratedFilesOutputPath — but build one TFM at
a time (it is a global property with no $(TargetFramework) expansion, so a multi-TFM build races all
six inner builds into one directory) and point it outside the repo tree so csharpier never sees it.
2. The repo gate
dotnet run --project build/build.csproj
check-format → restore --locked-mode → build all six TFMs → test on net10.0 and net48 → pack.
Note ns2.0/ns2.1/net6.0/net8.0 are compile-verified only, which is why step 1 matters.
3. AOT
dotnet publish tests/SharpCompress.AotSmoke/SharpCompress.AotSmoke.csproj -c Release --runtime linux-x64 --self-contained true --output artifacts/aot-smoke
The trim/AOT analyzers (net8.0/net10.0 set IsTrimmable/IsAotCompatible) run during the managed
compile, so an ordinary build already covers them; the native link step needs a C++ toolchain and is
covered by CI. A --runtime <rid> publish rewrites packages.lock.json with RID-specific sections —
revert that churn before committing.
4. Benchmarks
performance-benchmarks.yml flags >25% moves against tests/SharpCompress.Performance/baseline-results.md.
Batches 4, 9, 10 and 11 replace hand-optimised sync paths with code derived from allocation-tolerant
async bodies. A regression there is the signal to add a SYNC_ONLY site or leave that method
hand-written — not noise to override.
Remaining batches
Ordered by (duplication removed) / risk.
2 — IO/ leaf stream shims — done, 17 methods, 190 lines
ReadOnlySubStream, BufferedSubStream, SourceStream, SeekableSharpCompressStream,
ProgressReportingStream, CountingStream, Common/EntryStream.
What it removed: Read(byte[],int,int) from all seven; Read(Span<byte>) from CountingStream,
ProgressReportingStream and SeekableSharpCompressStream; Write(byte[],int,int) and
Write(ReadOnlySpan<byte>) from SeekableSharpCompressStream/CountingStream; Flush() from those
two; BufferedSubStream.RefillCache(); EntryStream.SkipEntry().
Notes from doing it:
IO/CountingStreamneededpartialadded (it has no.Async.cs— both halves live in one file).Flush/FlushAsynccan be generated when the body is a pure delegation (SeekableSharpCompressStream,CountingStream); the "leave hand-written" rule below is about pairs whose semantics differ, not about the method name.ReadOnlySubStream.Read(Span<byte>)stays hand-written — see the generator limitation below. ItsRead(byte[],int,int)is generated.- Not attributed, deliberately: every
DisposeAsync;SeekableSharpCompressStream.CopyToAsync(no hand-writtenCopyTo(Stream,int)twin); and theMemory<byte>overloads ofBufferedSubStream,SourceStreamandEntryStream, which have noRead(Span<byte>)twin. EntryStream: the informative doc comment lived on the syncSkipEntry; it moved toSkipEntryAsync, which is now the single source of truth.SharpCompressStreamwas left for later (stateful, 274-line async partial; also needs theReadAsyncCorerename and theReadAsync(Memory<byte>)rewrite listed below).
3 — XZ reader family · ~120 lines · low-med risk
XZIndexRecord, XZIndex, XZFooter, XZHeader, MultiByteIntegers, XZBlock.
Needs a converge commit first: XZFooter.ProcessAsync reads the CRC via
_reader.BaseStream.ReadLittleEndianUInt32Async(...) while the sync twin (XZFooter.cs:34) uses
_reader.ReadLittleEndianUInt32(). Those are different implementations (BinaryUtils.cs:19 goes
through ReadBytes(4), :33 through ReadFully) and throw different exception types on truncated
input (ArgumentOutOfRangeException vs IncompleteArchiveException). Same split at
XZIndex.Async.cs:78 vs XZIndex.cs:84, and XZHeader.Async.cs:33 vs XZHeader.cs:37. Also
MultiByteIntegers names the parameter MaxBytes in sync and maxBytes in async — generation
renames it (harmless, internal, but say so).
Xz/BinaryUtils stays hand-written: the sync version uses stackalloc byte[4] + ReadFully(Span),
the async one new byte[4], and the method is 4 lines — SYNC_ONLY would be most of it.
4 — LZMA internals · ~140 lines · med risk
LZ/LzOutWindow (9 pairs, all ≥0.96), RangeCoder/RangeCoderBitTree (6 pairs, all identical),
LzmaDecoder (8 pairs incl. CodeAsync 181 lines), RangeCoder/RangeCoder.
DisposeAsync→Dispose()is legal here —OutWindowandDecoderareIDisposable, notStream.RangeCoderBitneeds a rewrite first:BitEncoder.EncodeAsyncis non-async(return encoder.ShiftLowAsync(ct);/return default;) andBitDecoder.DecodeAsynccallsDecodeAsyncHelper(noAsyncsuffix, so no rewrite). Make both realasync/awaitand drop the helper. These were deliberately non-async — measure the LZMA hot path before and after.- Perf caveats to check in the generated output: it uses 1-byte
Read(buf,0,1)/Write(buf,0,1)where the hand-written sync code usedReadByte/WriteByte, and[MethodImpl(AggressiveInlining)]on the hotNormalize2is not carried over.SYNC_ONLYis the fix if the benchmarks move.
5 — ACE + ARJ parsing · ~200 lines · low risk
Ace/Headers/AceFileHeader (ReadAsync 94 identical lines), AceMainHeader (63), AceHeader,
Arj/HuffmanTree, Arj/BitReader, Arj/LhaStream (11 pairs, all ≥0.99), Arj/LHDecoderStream,
Squeezed/BitReader. Best win per line of review — pure parsing, all clean.
6 — Zip parts & header factories · ~250 lines · med risk · in progress
6a: ZipFilePart (incl. GetCryptoStreamAsync), SeekableZipFilePart, StreamingZipFilePart,
Zip/Headers/ZipFileEntry, PkwareTraditionalCryptoStream, and WinzipAesCryptoStream — done.
GZipFilePart was completed in the prior GZip batch.
6b: ZipHeaderFactory (LoadHeaderAsync identical), SeekableZipHeaderFactory.
7 — Concrete writers · ~250 lines · med risk · in progress
ZipWriter and ZipWritingStream (GetWriteStreamAsync is a 131-line duplicate) — done.
TarWriter and SevenZipWriter remain. GZipWriter was completed in the prior GZip batch.
This is the reachable Writers win: AbstractWriter already implements both IWriter and
IAsyncWriter (AbstractWriter.cs:11), so ZipWriter.Async.cs:52 WriteAsync(string,Stream,DateTime?,ct) maps exactly onto ZipWriter.cs:80 Write(string,Stream,DateTime?). Do not attribute AbstractWriter's abstract declarations, and do not
attempt IWriterExtensions (see Blocked).
8 — 7-Zip · ~200 lines · med-high risk
Common/SevenZip/ArchiveReader — ReadHeaderAsync (230 identical lines), ReadDatabaseAsync (106),
ReadAndDecodePackedStreamsAsync (72) — and ArchiveDatabase.GetFolderStreamAsync,
SevenZipFilePart, SevenZipSignatureHeader.
Clean: these use the sync DataReader, not an async-only reader type. Keep the file split
(ArchiveReader.cs is 1,377 lines).
9 — Streaming compressors · ~300 lines · med-high risk · in progress
Deflate64Stream, Deflate/ZlibBaseStream (ReadAsync, WriteAsync, FlushAsync),
DeflateStream, and ZlibStream — done. GZipStream was completed in the prior GZip batch.
ZStandard/*, Reduce, Explode, Lzw/LzwStream (ReadAsync 208 lines), RLE90, Shrink,
and ArcLzw remain.
ZlibBaseStreamis partial. Deflate-familyDisposeAsync, byte methods, unmatched memory overloads, GZip-header parsing, and finalization remain hand-written.RunLength90Streamis the canonicalSYNC_ONLYcase (see below).GZipCompressionProvider.cs:41returnsnew ValueTask<Stream>(...)from a non-asyncmethod — not a Zomp rewrite; make itasyncfirst. (Non-asyncmethods that just return anXAsync(...)call are fine —Lzma2Filterproved that in batch 1.)
10 — BZip2 · ~500 lines · high risk · two separate PRs
CBZip2InputStream (18 pairs, incl. RecvDecodingTablesAsync 142 identical lines and
GetAndMoveToFrontDecodeAsync 315) then, separately, CBZip2OutputStream (SendMTFValuesAsync 388
lines, EndBlockAsync, WriteRunAsync, …).
Largest mechanical win in the repo. [SkipSyncVersion] on ReadByteAsync/WriteByteAsync — the
generated ReadByte()/WriteByte() are emitted without override and would hide Stream's
(CS0108) — and on DisposeAsync. Watch the benchmark job.
11 — Rar UnpackV1 · ~400 lines · high risk
Unpack20 (unpack20Async 152 identical lines), Unpack15, Unpack (Unpack29Async 304,
UnpWriteBufAsync 203), Unpack50 (Unpack5Async 180), UnpackV2017/Unpack.
Fully clean — these use raw Stream, not the async reader types. The least readable diffs in the
repo, so do them last of the mechanical work.
12 — Providers · ~80 lines · low risk · in progress
CompressionProviderRegistry (4 identical pairs) — done. Default/* providers
(XStream.CreateAsync(...) → Create(...) maps cleanly), CompressionProviderBase, and
ContextRequiredDecompressionProviderBase remain. The GZipCompressionProvider rewrite was
completed in the prior GZip batch.
Cross-batch fixes that unlock files
Do these in the relevant batch's converge commit:
- Add
partial:IO/CountingStream,Compressors/Deflate/ZlibBaseStream. - Rename so
Asyncis actually a suffix (Zomp only strips a trailingAsync, so these currently generate a sync method that still says "Async" and never finds its twin):IEntryExtensions.WriteEntryToDirectoryAsyncCore→WriteEntryToDirectoryCoreAsync;SharpCompressStream.ReadAsyncCore→ReadCoreAsync. - Rewrite as real
async/await(Zomp rewritesawait Task.FromResult(x), but notnew ValueTask<T>(x)):RangeCoderBit.BitEncoder.EncodeAsync,BitDecoder.DecodeAsync,GZipCompressionProvider.CreateDecompressStreamAsync,SharpCompressStream.ReadAsync(Memory<byte>). Utility.SkipvsPolyfills/StreamExtensions.Skip: the generatedUtility.Skip(this Stream,long)would beCS0121-ambiguous with the polyfill. Merge or delete one first. DoUtilitylast — highest fan-in file in the library, only 2 near-matches to win. ItsReadFullyAsync(byte[])would also overwrite the#if NET8_0_OR_GREATERReadExactlyfast path, so that needsSYNC_ONLY.Common/IEntryExtensionsrelies onFunc<..,ValueTask>→Action<..>conversion (undocumented but implemented —IArchiveEntryExtensionsalready depends on it) plus the…CoreAsyncrename.
SYNC_ONLY — when, and the budget
Use it when the difference is a localised I/O idiom with identical semantics and ≥80% of the body
is shared algorithm. Canonical case Compressors/RLE90/RunLength90Stream: sync uses
_stream.ReadByte() with a -1 sentinel (.cs:86-91), async allocates byte[1] per byte
(.Async.cs:47-55), and the other ~55 lines are identical.
Constraints: works in statements, parameter lists and argument lists; cannot nest with !SYNC_ONLY
(ZSMGEN001); cannot combine with other symbols in one condition (ZSMGEN002); no #elif
(ZSMGEN003); contents are copied verbatim, so fully qualify names. ZSMGEN004 tells you to use
it when the async method awaits several operations at once.
Budget: at most two SYNC_ONLY sites per method, and never more than ~15% of its lines. Past
that, two honest files beat one half-preprocessor file.
Known generator limitations
Memory<T>overloads only convert when the buffer is passed through unmodified. Translatingstream.ReadAsync(memory, ct)tostream.Read(span)involves appending.Spanto the argument; if the async body slices first (_stream.ReadAsync(buffer.Slice(0, n), ct)), the generated code is_stream.Read(buffer.Slice(0, n).Span)— andbufferis already aSpan<byte>once the parameter is converted, so it fails withCS1061: 'Span<byte>' does not contain a definition for 'Span'. That is whyReadOnlySubStream.Read(Span<byte>)stays hand-written (with a comment saying so), while the direct-pass-through cases (CountingStream,ProgressReportingStream,SeekableSharpCompressStream) generate fine. It is a compile error, not a silent miscompile.- Only a trailing
Asyncis stripped —FooAsyncCoreandOpenAsyncReaderare not renamed. - Task-returning expressions that aren't awaited are not rewritten:
new ValueTask<T>(x)andValueTask.FromResult(x)need anasync/awaitrewrite first. A non-asyncmethod that simply returns anXAsync(...)call is fine (Lzma2FilterandSeekableSharpCompressStreamprove it).
Leave hand-written
Record the decision once, as a comment at the method, so it is not re-litigated.
Dispose/DisposeAsync— on aStreamthe generatedDispose()cannot override the non-virtualStream.Dispose()(CS0506); the real override isDispose(bool). (On a plainIDisposablesuch asOutWindowor LZMA'sDecoder,DisposeAsync→Dispose()is legal.)ReadByteAsync/WriteByteAsyncon aStream— generated withoutoverride, hidesStream.ReadByte/WriteByte(CS0108).- Pairs whose difference is an optimisation spread over >2 sites or >20% of the body — e.g.
Xz/BinaryUtils. Memory/ReadOnlyMemoryoverloads with no sync twin — see the symmetry question below.Flush/FlushAsyncandCopyTo/CopyToAsynconly where the two bodies genuinely differ. Pure delegations convert cleanly and were converted in batch 2.
Should the sync and async surfaces match?
Worth settling deliberately, because the answer decides whether some of the remaining asymmetries are
"leave alone" or "a batch of their own". Stream's base-class defaults mean an asymmetric type is not
neutral:
| Not overridden | What the base class does |
|---|---|
ReadAsync(byte[],int,int,ct) |
BeginRead/EndRead, i.e. runs the sync Read on a thread-pool thread |
FlushAsync(ct) |
runs the sync Flush on a thread-pool thread |
ReadAsync(Memory<byte>,ct) |
delegates to the byte[] overload, renting + copying when the memory is not array-backed |
Read(Span<byte>) |
rents an array, calls Read(byte[],int,int), copies back |
DisposeAsync() |
calls the sync Dispose() |
CopyToAsync(Stream,int,ct) |
generic ReadAsync/WriteAsync loop, skipping any inner fast path |
So: for wrapper/delegating streams the surfaces should match, and the generator makes that nearly free — the async body is written once and the sync twin is emitted. Two distinct asymmetries exist today:
- Async present, sync missing —
ReadAsync(Memory<byte>)with noRead(Span<byte>)(Filter,BufferedSubStream,SourceStream,EntryStream, all six XZ branch filters, and more). Attributing these creates the missingRead(Span<byte>), replacing the base rent-and-copy shim. That is a real improvement and makes the surfaces symmetric, but it is not a deduplication: it changes behaviour, so it does not belong in a batch advertised as a no-op. Do it as its own "symmetry pass" commit, one type at a time, and let the benchmark job see it. - Sync present, async missing — the async path then blocks a thread-pool thread. This is the only
case where new async code should be written as part of this campaign: write the async override,
attribute it, delete the sync one. An audit of all 75
Streamsubclasses found 12 real instances, listed below.
DisposeAsync is the deliberate exception in both directions: it cannot be generated on a Stream,
so its symmetry stays hand-maintained.
The 12 sync-only overrides (audited, verified by hand)
Only members that do real work are listed. A Write that throws NotSupportedException (or calls a
Throw* helper) needs no async twin, and neither does an empty Flush() { } — 22 types have one, and
overriding FlushAsync there just queues a no-op to the pool. Those are excluded.
Missing ReadAsync — the sync decode runs on a pool thread for every async read:
| Type | Sync member |
|---|---|
BCJ2Filter |
Read(byte[],int,int) — Compressors/Filters/BCJ2Filter.cs:92, the type has no async member at all, so async extraction of BCJ2-filtered 7z entries is fully synchronous |
DataDescriptorStream |
Read(byte[],int,int) — IO/DataDescriptorStream.cs:83 |
Missing WriteAsync:
| Type | Sync member |
|---|---|
FolderUnpackStream |
Write(byte[],int,int) — Common/SevenZip/ArchiveReader.cs:1190 (nested private class); fans one folder's bytes out across entry streams, all synchronously |
LZipStream |
Write(ReadOnlySpan<byte>) — Compressors/LZMA/LZipStream.cs:214; the type does override WriteAsync(byte[],int,int,ct) (.Async.cs:214) but not the ReadOnlyMemory<byte> overload |
Missing FlushAsync where Flush does real work — mostly one-line delegations, so these are the
cheapest to fix and the most mechanical (write FlushAsync, attribute it, delete Flush):
| Type | Flush() body |
|---|---|
ZipWritingStream |
writeStream.Flush() — Writers/Zip/ZipWritingStream.cs:452; on the writer path, and the type already overrides both WriteAsync overloads, so this is the sharpest inconsistency |
ProgressReportingStream |
_baseStream.Flush() |
SourceStream |
Current.Flush() |
LzwStream |
baseInputStream.Flush() |
LZipStream |
_stream.Flush() |
Deflate64Stream |
EnsureNotDisposed() |
BZip2Stream |
delegates to the underlying BZip2 stream |
CBZip2OutputStream |
flushes encoder state |
The audit is reproducible: walk every class in src/SharpCompress, union its members across partial
files, resolve base types within the repo, and for each (sync, async) pair report types that declare
the sync member but where neither the type nor any repo ancestor declares the async one. Classify the
sync body as throws / empty / real and only report real. Note that "an ancestor declares the async
one" is a worse finding than "nobody does" — it means async callers bypass the override entirely —
but the audit found no instances of that.
Hard-blocked
| Area | Blocker |
|---|---|
Writers/IWriterExtensions.cs |
Sync is extension(IWriter), async is extension(IAsyncWriter). IWriter : IDisposable and IAsyncWriter : IAsyncDisposable are public interfaces with different base contracts; merging them breaks every external implementor for ~50 lines of thin sugar. Leave hand-written permanently — take batch 7 instead. |
Readers/IAsyncReaderExtensions.cs, Archives/IAsyncArchiveExtensions.cs |
Same interface split, and the sync twins live in different classes (IReaderExtensions, IArchiveExtensions). Plus method-vs-property mismatches: IsSolidAsync() vs IArchive.IsSolid, TotalUncompressedSizeAsync() vs the property, EntriesAsync vs Entries. |
Factories/*Factory.cs |
OpenAsyncArchive/OpenAsyncReader/OpenAsyncWriter don't end in Async, so no rename happens; Factory.TryOpenReaderAsync returns ValueTask<IAsyncReader?> and cannot override internal virtual bool TryOpenReader(...); bodies use target-typed return new(...) on ValueTask. |
Archives/AbstractArchive*, ArchiveFactory |
Use AsyncEnumerableEx.Empty<T>() / volumes.ToListAsync() (Polyfills/AsyncEnumerableExtensions.cs) — no sync equivalent. |
| Rar header family | Async-only mirror types — see below. |
Two worries that turned out not to be real: the #if LEGACY_DOTNET byte[]-vs-Memory<byte>
overload groups cannot collide (Memory<T>→Span<T> yields a distinct signature), and where a
#if/#else pair declares the same shape twice you put the attribute inside each branch since only
one is ever active. Collisions that are possible (two async overloads differing only by
CancellationToken/IProgress) are caught by ZSMGEN005, not by review.
Warnings to expect
TreatWarningsAsErrors is on. Analyzers skip .g.cs, but compiler CS warnings do not: CS0162
unreachable (a return/break that vanishes from the sync copy), CS0219/CS0168 unused local (a
result only checked on the async side), CS8602/CS8604 nullable (the generator emits
#nullable enable unconditionally — OmitNullableDirective = true is the escape hatch),
CS0108/CS0114 hiding. Fix in the async source, or #pragma warning disable/restore there — it
is copied into the generated file. CS0111/CS0534 are the desired safety net for "did I delete
the right thing".
Rar reader unification (own PR, own review)
The Rar header family scores zero exact matches despite ~1,150 duplicated lines, purely because
the sync methods take RarCrcBinaryReader/MarkingBinaryReader and the async ones take
AsyncRarCrcBinaryReader/AsyncMarkingBinaryReader. Zomp cannot change parameter types. Unifying
the reader unlocks FileHeader.Async.cs (~340 lines: ReadFromReaderV5Async 169,
ReadFromReaderV4Async 171), RarHeaderFactory.Async.cs (~200), MarkHeader (~130), RarHeader
(~115), ArchiveHeader, EndArchiveHeader, ProtectHeader, ArchiveCryptHeader, Rar5CryptoInfo,
RarVolume.
Do not try to generate the sync side of today's MarkingBinaryReader: it derives from
BinaryReader and gets its whole API by overrideing ReadByte/ReadBytes/ReadUInt16/… .
Modifiers are copied from the async method, which cannot be override because BinaryReader has no
async virtuals — so you get CS0108/CS0114.
The async side is already the right shape. Common/Rar/AsyncMarkingBinaryReader does not derive
from BinaryReader; it wraps IO/AsyncBinaryReader over a raw Stream. So:
- Make
IO/AsyncBinaryReaderpartialand attribute its async primitives (ReadByteAsync,ReadUInt16/32/64Async,ReadBytesAsync,SkipAsync) to generate the sync twins. It ispublic sealed— keep the name (renaming breaks public API) and keep it sealed; generation is unaffected.Dispose/DisposeAsyncare already hand-written; leave them. - Make
AsyncMarkingBinaryReader,AsyncRarCrcBinaryReader,AsyncRarCryptoBinaryReaderpartialand attribute their async methods. Thevirtual/overridechain survives verbatim: basepublic virtual async ValueTask<byte> ReadByteAsync→public virtual byte ReadByte(), and the CRC subclass'soverride→public override byte ReadByte(). - Rename
AsyncRarCryptoBinaryReader.Create→CreateAsync: it is a static async factory that does not end inAsync, so the generated syncCreatewould differ from it only by return type (CS0111). - Delete
IO/MarkingBinaryReader.cs,Common/Rar/RarCrcBinaryReader.cs,Common/Rar/RarCryptoBinaryReader.cs. Drop theAsyncprefix from the three remainingCommon/Rar/Async*BinaryReader.cstypes (allinternal, so free) and update the ~15 Rar header call sites. - Then attribute the header methods and delete their sync twins, batch by batch:
RarHeader+ the small headers first,FileHeaderalone,RarHeaderFactoryalone.
This is a behaviour change, not a deletion. Name the deltas in the PR:
- Truncation exception type differs today: sync
MarkingBinaryReader.ReadByte→BinaryReader→EndOfStreamException; the unified path →Stream.ReadExact→IncompleteArchiveException. ReadBytestruncation message differs —"Requested: {0} Read: {1}"(MarkingBinaryReader.cs:47) vs"Requested: {0}"(AsyncMarkingBinaryReader.cs:54). Pick one.MarkingBinaryReader'sNotSupportedExceptionguards (Read(),ReadChar,ReadDouble,ReadSingle,ReadString, …) disappear with theBinaryReaderbase. Confirm nothing depends on them.RemainingHeaderBytes(MarkingBinaryReader)(RarHeader.cs:109) andRemainingHeaderBytesAsync(AsyncMarkingBinaryReader)(:112) — the latter isn't async at all; after unification they are the same signature, so delete one.
Prerequisite: pin today's byte-counting/CRC behaviour with tests first —
Mark()/CurrentReadByteCount through both RarCrcBinaryReader and RarCryptoBinaryReader, and
RAR5 encrypted-header reads. The comment at IO/MarkingBinaryReader.cs:26-34 warns that the CRC and
crypto subclasses depend on which methods call the base directly; that dependency is what the tests
must lock down.
Ranking: below batches 10–11. BZip2 (~500 lines) and Rar UnpackV1 (~400) deliver comparable
savings as provable no-ops, whereas this one touches CRC and RAR5 crypto byte counting.