Skip to content

Commit 0e9d740

Browse files
committed
feat(io-compression): add the three streams' ZLibCompressionOptions constructors (#2150)
Rule-14 sweep. The recorded gate was a classification -- "public surface addition; needs approval" -- and this repository's own design record already contradicted it: SystemIOCompressionNamespaceReviewPlan.md section 7 lists #2150 as signature "new ctors", vtable --, layout --, noexcept --, result "additive". That is the #1980-G-1 and #1997-A-1 shape, both of which landed as ordinary SA-5 work. The one substantive claim is false and is now asserted rather than argued. A new overload can change an existing call only if some argument binds to both parameter types. CompressionMode is a scoped enumeration and ZLibCompressionOptions has no converting constructor, so nothing converts to both -- pinned, so the claim must be re-made if either fact changes. Adding mangled symbols breaks nothing: no existing symbol was removed or changed. All three streams gain (Stream*, const ZLibCompressionOptions&, bool = false), implying Compress as .NET's does. Level, strategy and window log all reach zlib; memLevel follows .NET's rule of 7 at quality 0, otherwise 8. The format arithmetic went from three copies to one, not four. Each encoder carried its own in an anonymous namespace and the streams had none, so Detail::ResolveWindowBits is now shared and the encoders delegate to it -- a net removal, proven behaviour-preserving by all 103 pre-existing cases passing unchanged. Two asymmetries are transcribed rather than smoothed: Deflate and GZip clamp the window log to 9 and ZLib deliberately does not, and the sign and offset are the container. Six mutations, all caught. M3 only after a pin was added: every end-to-end case used a window log of 9 or 15 and never exercised the one input that separates clamping from not clamping. Asserting the resolver directly was necessary rather than convenient -- classic zlib upgrades windowBits 8 to 9 internally, so no end-to-end test could discriminate it. M6 was invalid as first written (-Werror on an orphaned constexpr) and was reformulated rather than counted. The compile-time absence pin is inverted in place, not deleted. Downstream measured: 0 sites in cna, 0 in mobile-eggbert. Gate: 17,444 run, 17,444 passed, 0 failed, 0 skipped across 38 executables (+10 on 17,434; SharpRuntimeTests_IO_Compression 103 -> 113; no other executable moved). Module graph unchanged at 41/93.
1 parent 0a08ad0 commit 0e9d740

16 files changed

Lines changed: 672 additions & 21 deletions

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
<!-- Copyright (c) Robert Vokac and contributors -->
3+
4+
# Migration — the three compression streams gained their options constructors (ticket #2150)
5+
6+
*2026-08-19.* `System::IO::Compression::DeflateStream`, `GZipStream` and `ZLibStream` each gained
7+
8+
```cpp
9+
Stream(System::IO::Stream* stream, const ZLibCompressionOptions& options, bool leaveOpen = false);
10+
```
11+
12+
matching .NET's `(Stream, ZLibCompressionOptions, bool)` constructor on all three types.
13+
14+
**Purely additive.** Nothing that compiled before compiles differently, no existing call changes
15+
meaning, no symbol changed, and no object grew. Landed under `docs/StandingApprovals.md` **SA-5**.
16+
17+
---
18+
19+
## 1. The ticket's classification was wrong, and the repository's own record already said so
20+
21+
#2150 was recorded as *"blocked — public surface addition, approval required"*, on the grounds
22+
that *"new overloads change source overload resolution and add mangled symbols"*.
23+
24+
**The design record already contradicted that.** `docs/SystemIOCompressionNamespaceReviewPlan.md`
25+
§7 lists #2150 as: signature *new ctors*, vtable —, layout —, `noexcept` —, accepted input —,
26+
observable result **additive**.
27+
28+
**And the overload-resolution claim is false here, measurably.** A new overload can change the
29+
meaning of an existing call only if some argument can bind to both. Two facts prevent that:
30+
31+
| Fact | Consequence |
32+
|---|---|
33+
| `CompressionMode` is a **scoped** enumeration | no integer, `bool` or other enum converts to it, and it converts to nothing |
34+
| `ZLibCompressionOptions` has **no converting constructor** — only a defaulted default one | nothing implicitly converts *to* it |
35+
36+
With no type convertible to both parameter types, no existing call can rebind and no new call can
37+
bind to the old overload. This is asserted, not argued, by
38+
`Decl2150_TheAdditionCannotRebindAnExistingCall` — and if either fact stops holding, that test
39+
fails and the additive claim must be re-made.
40+
41+
*Adding* mangled symbols breaks nothing: no existing symbol was removed or changed, so an
42+
already-linked consumer is unaffected and a recompiling one sees only new declarations.
43+
44+
This is the same shape as #1980's G-1 and #1997's A-1 earlier in this programme: a sub-item whose
45+
own record calls it additive is ordinary SA-5 work.
46+
47+
## 2. What the constructor does
48+
49+
It implies **`CompressionMode::Compress`** — .NET's options overload carries no mode, for the
50+
same reason its `CompressionLevel` overloads are commented *"Implies mode = Compress"*: every
51+
option the type holds describes compression.
52+
53+
Every option is honoured:
54+
55+
| Option | Becomes |
56+
|---|---|
57+
| `CompressionLevel` | zlib's `level` |
58+
| `CompressionStrategy` | zlib's `strategy`, via the shared `Detail::ResolveZLibStrategy` |
59+
| `WindowLog` | zlib's `windowBits`, via the shared `Detail::ResolveWindowBits` |
60+
| *(derived from the level)* | zlib's `memLevel` — **7** at quality 0, otherwise **8**, as .NET does |
61+
62+
## 3. One definition of the format arithmetic, not four
63+
64+
`Detail::ResolveWindowBits(windowLog, CompressionFormat)` and `Detail::ResolveDeflateMemLevel`
65+
are new, and they are transcribed from .NET's `CompressionFormatHelper.ResolveWindowBits`
66+
(`CompressionFormat.cs:30-48`) and `DeflateEncoder` (`DeflateEncoder.cs:75-77, 95-97`).
67+
68+
Before this ticket, each of the three **encoders** carried its own copy of the format arithmetic
69+
in an anonymous namespace, and the three **streams** had none — because they had no options
70+
constructor to need one. Rather than add three more copies, the three encoders now delegate to
71+
the shared resolver too. That is a net removal, and it is proven behaviour-preserving: all 103
72+
pre-existing `SharpRuntimeTests_IO_Compression` cases pass unchanged.
73+
74+
**Two asymmetries are transcribed rather than smoothed away**, and both are pinned:
75+
76+
1. **Deflate and GZip clamp the window log to a minimum of 9; ZLib does not.** .NET's own comment
77+
gives the reason — *"zlib-ng rejects windowBits 8 for raw deflate and gzip; classic zlib
78+
silently upgrades to 9"* — so the clamp makes the two zlib implementations agree. A resolver
79+
that clamped uniformly would change what a `WindowLog` of 8 means for `ZLibStream`.
80+
2. **The sign and offset are the container**: raw deflate is negative, zlib positive, gzip
81+
positive plus 16.
82+
83+
## 4. Evidence
84+
85+
Six mutations, **all caught**:
86+
87+
| Mutation | Caught by |
88+
|---|---|
89+
| M1 — the stream constructor drops the strategy | `Fix2150_TheStrategyReachesZlibInAllThreeStreams` |
90+
| M2 — the window log is ignored and the default used | `Fix2150_TheWindowLogReachesZlibInAllThreeStreams` |
91+
| M3 — the ZLib arm clamps to 9 as well | `Fix2150_ResolveWindowBitsTranscribesBothAsymmetries` — **only after that test was added**, see below |
92+
| M4 — the deflate window-bits sign is lost | six cases, four of them **pre-existing** |
93+
| M5 — the gzip `+16` offset is lost | five cases, three of them **pre-existing** |
94+
| M6 — the `memLevel` boundary moves | `Fix2150_ResolveDeflateMemLevelIsSevenOnlyAtQualityZero` |
95+
96+
**M3 is the one worth recording.** It went uncaught at first because every end-to-end case used a
97+
`WindowLog` of 9 or 15, so the single input that distinguishes clamping from not clamping — a
98+
`WindowLog` of 8 — was never exercised. The fix was to assert the **resolver** directly rather
99+
than to hunt for the difference in emitted bytes, and that is not merely more convenient: classic
100+
zlib silently upgrades a `windowBits` of 8 to 9 internally, so an end-to-end test could not have
101+
discriminated this reliably at all.
102+
103+
M6 was invalid as first written — removing the quality-0 arm left a `constexpr` unused and
104+
`-Werror` rejected it — and was reformulated (moving the boundary to `<= 1`) rather than counted.
105+
106+
Gate: **17,444 run, 17,444 passed, 0 failed, 0 skipped** across 38 executables — `+10` on 17,434,
107+
exactly the ten new cases (`SharpRuntimeTests_IO_Compression` 103 → 113). No other executable
108+
moved. Module graph
109+
unchanged at 41/93.
110+
111+
## 5. To use it
112+
113+
```cpp
114+
ZLibCompressionOptions options;
115+
options.setCompressionLevelProperty(9);
116+
options.setCompressionStrategyProperty(ZLibCompressionStrategy::RunLengthEncoding);
117+
118+
MemoryStream sink;
119+
GZipStream out(&sink, options, /*leaveOpen=*/true);
120+
out.Write(data.data(), 0, static_cast<intcs>(data.size()));
121+
out.Close();
122+
```
123+
124+
Nothing needs migrating. The `(Stream*, CompressionMode, bool)` constructor is untouched and
125+
still the only way to build a **decompressing** stream.
126+
127+
## 6. Downstream, measured
128+
129+
Per SA-2 condition 5 — recorded although SA-2 is not the approval this landed under, because an
130+
addition to a public type deserves the measurement anyway: `DeflateStream`, `GZipStream`,
131+
`ZLibStream` and `ZLibCompressionOptions` appear in **zero** places in `cna` and **zero** in
132+
`mobile-eggbert`. Neither repository was modified.

modules/io-compression/include/System/IO/Compression/CompressionArgumentValidation.hpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,4 +219,45 @@ namespace System::IO::Compression::Detail {
219219
*/
220220
[[nodiscard]] intcs ResolveZLibStrategy(ZLibCompressionStrategy strategy);
221221

222+
/**
223+
* @brief The three zlib container formats this component encodes.
224+
*
225+
* C++ counterpart of .NET's internal `System.IO.Compression.CompressionFormat`
226+
* (`CompressionFormat.cs:9-17`). It exists for the same reason: window bits are the one
227+
* parameter whose meaning depends on the container, so resolving them needs the format as
228+
* an argument rather than three near-identical local functions.
229+
*/
230+
enum class CompressionFormat { Deflate, ZLib, GZip };
231+
232+
/**
233+
* @brief Resolves a `WindowLog` (8..15, or -1 for the default) to zlib's `windowBits`.
234+
*
235+
* Transcribed from .NET's `CompressionFormatHelper.ResolveWindowBits`
236+
* (`CompressionFormat.cs:30-48`), including both of its asymmetries:
237+
*
238+
* * `-1` resolves to **15**, the default window log;
239+
* * **Deflate and GZip clamp to a minimum of 9, and ZLib does not.** .NET's own comment
240+
* gives the reason — *"zlib-ng rejects windowBits 8 for raw deflate and gzip; classic zlib
241+
* silently upgrades to 9"* — so the clamp reproduces classic zlib's behaviour on both
242+
* implementations. A resolver that clamped uniformly would change what a `WindowLog` of 8
243+
* means for `ZLibStream`.
244+
*
245+
* The sign and offset are then the container: raw deflate is negative, zlib is positive,
246+
* gzip is positive plus 16.
247+
*
248+
* Defined once here so the three encoders and the three stream types share one definition.
249+
* Before ticket #2150 each encoder carried its own copy in an anonymous namespace, and the
250+
* streams had none at all because they had no options constructor to need one.
251+
*/
252+
[[nodiscard]] intcs ResolveWindowBits(intcs windowLog, CompressionFormat format);
253+
254+
/**
255+
* @brief Resolves the zlib `memLevel` for a compression quality.
256+
*
257+
* Transcribed from .NET's `DeflateEncoder` (`DeflateEncoder.cs:75-77, 95-97`): quality 0
258+
* ("no compression") uses memLevel **7**, every other quality uses **8**. The rule is the
259+
* same for all three containers, so it takes no format.
260+
*/
261+
[[nodiscard]] intcs ResolveDeflateMemLevel(intcs quality);
262+
222263
} // namespace System::IO::Compression::Detail

modules/io-compression/include/System/IO/Compression/DeflateStream.hpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#pragma once
55
#include "System/IO/Stream.hpp"
66
#include "System/IO/Compression/CompressionMode.hpp"
7+
#include "System/IO/Compression/ZLibCompressionOptions.hpp"
78
#include <memory>
89

910
// Note: .NET DeflateStream uses raw DEFLATE (no gzip wrapper), unlike GZipStream.
@@ -52,6 +53,36 @@ namespace System::IO::Compression {
5253
*/
5354
DeflateStream(Stream* stream, CompressionMode mode, bool leaveOpen = false);
5455

56+
/**
57+
* @brief Constructs a compressing DeflateStream using the given compression options.
58+
*
59+
* C++ counterpart of .NET's `DeflateStream(Stream, ZLibCompressionOptions, bool)`. **This
60+
* constructor implies `CompressionMode::Compress`** — .NET's own comment above the
61+
* `CompressionLevel` overloads says *"Implies mode = Compress"*, and the options
62+
* overload has no mode parameter for the same reason: every option it carries describes
63+
* compression.
64+
*
65+
* The options are honoured in full: `CompressionLevel` becomes zlib's level,
66+
* `CompressionStrategy` its strategy, and `WindowLog` its window size, resolved for
67+
* raw deflate (no header or trailer) by `Detail::ResolveWindowBits`. `memLevel` follows .NET's rule — 7 at
68+
* quality 0, otherwise 8.
69+
*
70+
* @param stream The stream to which compressed data is written.
71+
* @param options The compression options. Validated by `ZLibCompressionOptions`'
72+
* own setters, so an out-of-range value cannot reach here.
73+
* @param leaveOpen When @c true the inner stream is not closed on destruction.
74+
*
75+
* @throws System::ArgumentNullException if @p stream is null.
76+
* @throws System::IO::IOException if zlib initialisation fails.
77+
*
78+
* @note Ticket **#2150**. Adding this overload cannot change the meaning of any existing
79+
* call: `CompressionMode` is a scoped enumeration and `ZLibCompressionOptions` has no
80+
* converting constructor, so no argument can bind to both this and the
81+
* `(Stream*, CompressionMode, bool)` overload. That is asserted, not assumed, by
82+
* `CompressionOptionsConstructorTests.Decl2150_TheAdditionCannotRebindAnExistingCall`.
83+
*/
84+
DeflateStream(Stream* stream, const ZLibCompressionOptions& options, bool leaveOpen = false);
85+
5586
~DeflateStream() override;
5687

5788
/** @brief Returns @c true when mode is Decompress. */

modules/io-compression/include/System/IO/Compression/GZipStream.hpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#pragma once
55
#include "System/IO/Stream.hpp"
66
#include "System/IO/Compression/CompressionMode.hpp"
7+
#include "System/IO/Compression/ZLibCompressionOptions.hpp"
78
#include <memory>
89

910
namespace System::IO::Compression {
@@ -48,6 +49,36 @@ namespace System::IO::Compression {
4849
*/
4950
GZipStream(Stream* stream, CompressionMode mode, bool leaveOpen = false);
5051

52+
/**
53+
* @brief Constructs a compressing GZipStream using the given compression options.
54+
*
55+
* C++ counterpart of .NET's `GZipStream(Stream, ZLibCompressionOptions, bool)`. **This
56+
* constructor implies `CompressionMode::Compress`** — .NET's own comment above the
57+
* `CompressionLevel` overloads says *"Implies mode = Compress"*, and the options
58+
* overload has no mode parameter for the same reason: every option it carries describes
59+
* compression.
60+
*
61+
* The options are honoured in full: `CompressionLevel` becomes zlib's level,
62+
* `CompressionStrategy` its strategy, and `WindowLog` its window size, resolved for
63+
* the gzip container by `Detail::ResolveWindowBits`. `memLevel` follows .NET's rule — 7 at
64+
* quality 0, otherwise 8.
65+
*
66+
* @param stream The stream to which compressed data is written.
67+
* @param options The compression options. Validated by `ZLibCompressionOptions`'
68+
* own setters, so an out-of-range value cannot reach here.
69+
* @param leaveOpen When @c true the inner stream is not closed on destruction.
70+
*
71+
* @throws System::ArgumentNullException if @p stream is null.
72+
* @throws System::IO::IOException if zlib initialisation fails.
73+
*
74+
* @note Ticket **#2150**. Adding this overload cannot change the meaning of any existing
75+
* call: `CompressionMode` is a scoped enumeration and `ZLibCompressionOptions` has no
76+
* converting constructor, so no argument can bind to both this and the
77+
* `(Stream*, CompressionMode, bool)` overload. That is asserted, not assumed, by
78+
* `CompressionOptionsConstructorTests.Decl2150_TheAdditionCannotRebindAnExistingCall`.
79+
*/
80+
GZipStream(Stream* stream, const ZLibCompressionOptions& options, bool leaveOpen = false);
81+
5182
~GZipStream() override;
5283

5384
/** @brief Returns @c true when mode is Decompress. */

modules/io-compression/include/System/IO/Compression/ZLibStream.hpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#pragma once
55
#include "System/IO/Stream.hpp"
66
#include "System/IO/Compression/CompressionMode.hpp"
7+
#include "System/IO/Compression/ZLibCompressionOptions.hpp"
78
#include <memory>
89

910
namespace System::IO::Compression {
@@ -48,6 +49,36 @@ namespace System::IO::Compression {
4849
*/
4950
ZLibStream(Stream* stream, CompressionMode mode, bool leaveOpen = false);
5051

52+
/**
53+
* @brief Constructs a compressing ZLibStream using the given compression options.
54+
*
55+
* C++ counterpart of .NET's `ZLibStream(Stream, ZLibCompressionOptions, bool)`. **This
56+
* constructor implies `CompressionMode::Compress`** — .NET's own comment above the
57+
* `CompressionLevel` overloads says *"Implies mode = Compress"*, and the options
58+
* overload has no mode parameter for the same reason: every option it carries describes
59+
* compression.
60+
*
61+
* The options are honoured in full: `CompressionLevel` becomes zlib's level,
62+
* `CompressionStrategy` its strategy, and `WindowLog` its window size, resolved for
63+
* the zlib container by `Detail::ResolveWindowBits`. `memLevel` follows .NET's rule — 7 at
64+
* quality 0, otherwise 8.
65+
*
66+
* @param stream The stream to which compressed data is written.
67+
* @param options The compression options. Validated by `ZLibCompressionOptions`'
68+
* own setters, so an out-of-range value cannot reach here.
69+
* @param leaveOpen When @c true the inner stream is not closed on destruction.
70+
*
71+
* @throws System::ArgumentNullException if @p stream is null.
72+
* @throws System::IO::IOException if zlib initialisation fails.
73+
*
74+
* @note Ticket **#2150**. Adding this overload cannot change the meaning of any existing
75+
* call: `CompressionMode` is a scoped enumeration and `ZLibCompressionOptions` has no
76+
* converting constructor, so no argument can bind to both this and the
77+
* `(Stream*, CompressionMode, bool)` overload. That is asserted, not assumed, by
78+
* `CompressionOptionsConstructorTests.Decl2150_TheAdditionCannotRebindAnExistingCall`.
79+
*/
80+
ZLibStream(Stream* stream, const ZLibCompressionOptions& options, bool leaveOpen = false);
81+
5182
~ZLibStream() override;
5283

5384
/** @brief Returns @c true when mode is Decompress. */

modules/io-compression/src/System/IO/Compression/CompressionArgumentValidation.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: MIT
22
// Copyright (c) Robert Vokac and contributors
33
// Portions based on .NET runtime API (MIT License, Copyright .NET Foundation and Contributors)
4+
#include <algorithm>
45
#include "System/IO/Compression/CompressionArgumentValidation.hpp"
56
#include "System/ArgumentException.hpp"
67
#include "System/ArgumentNullException.hpp"
@@ -61,4 +62,30 @@ namespace System::IO::Compression::Detail {
6162
throw System::ArgumentOutOfRangeException("strategy", "Value must be between Default and Fixed.");
6263
}
6364

65+
namespace {
66+
// ZLibCompressionOptions publishes these as MinWindowLog/MaxWindowLog/DefaultWindowLog;
67+
// they are restated locally so this translation unit does not depend on that header for
68+
// two integers.
69+
constexpr intcs DefaultWindowLog = 15;
70+
constexpr intcs Deflate_DefaultMemLevel = 8;
71+
constexpr intcs Deflate_NoCompressionMemLevel = 7;
72+
}
73+
74+
intcs ResolveWindowBits(intcs windowLog, CompressionFormat format) {
75+
if (windowLog == -1) windowLog = DefaultWindowLog;
76+
// Deflate and GZip clamp; ZLib deliberately does not. See the header for .NET's reason.
77+
if (format != CompressionFormat::ZLib)
78+
windowLog = std::max(windowLog, static_cast<intcs>(9));
79+
switch (format) {
80+
case CompressionFormat::Deflate: return -windowLog;
81+
case CompressionFormat::ZLib: return windowLog;
82+
case CompressionFormat::GZip: return windowLog + 16;
83+
}
84+
throw System::ArgumentOutOfRangeException("format");
85+
}
86+
87+
intcs ResolveDeflateMemLevel(intcs quality) {
88+
return quality == 0 ? Deflate_NoCompressionMemLevel : Deflate_DefaultMemLevel;
89+
}
90+
6491
} // namespace System::IO::Compression::Detail

modules/io-compression/src/System/IO/Compression/DeflateEncoder.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,11 @@ namespace System::IO::Compression {
2323
constexpr intcs Deflate_DefaultMemLevel = 8;
2424
constexpr intcs Deflate_NoCompressionMemLevel = 7;
2525

26+
// #2150 replaced this function's body with the shared resolver. The three encoders each
27+
// carried their own copy of the format arithmetic; there is now one definition, in
28+
// Detail::ResolveWindowBits, which the three STREAM options constructors also use.
2629
intcs ResolveDeflateWindowBits(intcs windowLog) {
27-
if (windowLog == -1) windowLog = DefaultWindowLog;
28-
windowLog = std::max(windowLog, static_cast<intcs>(9));
29-
return -windowLog;
30+
return Detail::ResolveWindowBits(windowLog, Detail::CompressionFormat::Deflate);
3031
}
3132
}
3233

0 commit comments

Comments
 (0)