Skip to content

Commit e20bba6

Browse files
committed
feat(core,numerics,buffers): the virtual lifetime service, plus three declarations (#2374, #2392, #2058, #2381)
#2374 -- MarshalByRefObject gains the VIRTUAL InitializeLifetimeService() .NET has (MarshalByRefObject.cs:22-26). Its absence turned an observable runtime diagnostic into a compile error at an unrelated place. The class already has a vtable, so this inserts a SLOT here and in both derived classes, AppDomain and ContextBoundObject -- a silent binary break, granted per action on the measurement that all three types have zero sites in both consumers. `virtual` is the point rather than a detail: a non-virtual member of the same name satisfies a presence check, compiles at every call site, and silently defeats the override the .NET member exists for. That is why #2297 left it ABSENT rather than add it that way, and why the new case calls through a BASE REFERENCE and asserts DISPATCH. #2297's absence pin is inverted and renamed, and its detection idiom is KEPT, because that is what makes presence and absence expressible in the same form. Three mutations, all caught; a first run of the set was invalid (the restore step copied a pre-change backup) and is recorded rather than counted. #2392, #2058 and #2381 change no production statement. #2392 declares that TotalOrderIeee754Comparer::GetHashCode hashes the bit pattern where .NET hashes the value. The contract holds either way -- equality here IS bit-pattern identity -- so the choice is about distribution, not correctness. The pin also records that Double::GetHashCode ALREADY matches .NET, so a future reader does not fix the wrong one. #2058 declares ReadOnlySequence<T> single-segment only, citing .NET's segment constructor and its computed IsSingleSegment. Its premise was partly stale: the false class comment had already been removed. The single-segment case was widened from two constructors to all five. #2381 is the one instruction that was granted and could not be carried out. Measured: cna builds against develop, which has no DateTimeKind at all. This is #2366's trap without #2366's escape -- no expression means "preserve the kind" in a version that has no kind. Editing anyway would leave cna unbuildable, so the ticket records the exact merge-time diff and is blocked on the merge rather than on a decision. Gate: 17,561 run, 17,561 passed, 0 failed, 0 skipped across 38 executables (+2). Module graph 41/93. Build directory: build/ only, --parallel 2 throughout.
1 parent 2562ace commit e20bba6

9 files changed

Lines changed: 267 additions & 39 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
# Migration — `MarshalByRefObject::InitializeLifetimeService()` (#2374)
3+
4+
Ticket **#2374**, landed 2026-08-19 on an explicit per-action approval
5+
(`docs/StandingApprovals.md` SA-13 batch). **Decided against the recommendation on the record.**
6+
7+
## What changed
8+
9+
```cpp
10+
[[noreturn]] virtual void* InitializeLifetimeService(); // throws PlatformNotSupportedException
11+
```
12+
13+
matching .NET's `public virtual object InitializeLifetimeService()`
14+
(`MarshalByRefObject.cs:22-26`), which throws
15+
`PlatformNotSupportedException(SR.PlatformNotSupported_Remoting)`.
16+
17+
Its absence turned an **observable runtime diagnostic** into a **compile error at an unrelated
18+
place** for anyone writing the .NET call.
19+
20+
## Why it needed an ask, and why it had to be `virtual`
21+
22+
`System::MarshalByRefObject` already has a vtable (the virtual destructor), so adding a virtual
23+
**inserts a slot** — here and in **both** derived classes, `System::AppDomain` and
24+
`System::ContextBoundObject`. SA-3 authorises private data members and **excludes vtable changes
25+
explicitly**; SA-10 covers signatures and does not reach a vtable. Silent binary break: nothing
26+
fails to compile and **every consumer must rebuild**.
27+
28+
Measured at the time of the grant — **zero** sites for all three types in `cna` and **zero** in
29+
`mobile-eggbert`.
30+
31+
**It must not be added non-virtually as a shortcut.** A non-virtual member of the same name would
32+
compile, satisfy any presence check, look correct at every call site, and **silently defeat the one
33+
thing the .NET member exists for**: letting a derived type override the lease policy. #2297 left it
34+
*absent* rather than add it that way, and that judgement stands — the approval bought the slot, not
35+
a workaround.
36+
37+
## The return type is `void*`, deliberately
38+
39+
.NET returns `object`. This port's `System::Object` is an abstract class a lease object would have
40+
to derive from, which is surface this port does not have and #2374 does not invent. It matches the
41+
shape `GetLifetimeService()` already uses, and since the body never returns, **no caller can observe
42+
the difference**.
43+
44+
## Tests
45+
46+
`Fix2297_…TheVirtualOneIsStillAbsent` is **inverted** — it had said *"this assertion is what makes
47+
its arrival visible rather than silent"* — and renamed to
48+
`Fix2297_GetLifetimeServiceThrows_And2374_AddedTheVirtualOne`. The **detection idiom is kept**
49+
rather than replaced by a direct call, because it is what makes presence and absence expressible in
50+
the same form.
51+
52+
`Fix2374_ADerivedTypeCanOverrideTheLeasePolicy` is the case that matters: it calls through a **base
53+
reference**, so what is asserted is *dynamic dispatch*, not a same-name member found by static
54+
lookup. A non-virtual member runs the base body there and throws. It also asserts that a plain
55+
derived type **still throws**, so the override is a real replacement rather than the base having
56+
gone quiet.
57+
58+
## Mutation testing
59+
60+
Three mutations, all caught:
61+
62+
| # | Mutation | Caught by |
63+
|---|---|---|
64+
| M1 | make the member non-virtual | compile error — the `override` in the test's derived type |
65+
| M2 | remove the member again | compile error — the presence `static_assert` |
66+
| M3 | the base body stops throwing | two cases |
67+
68+
A first run of this set was **invalid and is recorded rather than counted**: the restore step
69+
copied a *pre-change* backup, so M2 and M3 reported ANCHOR MISSING against a file that no longer
70+
had the member. The baseline was re-taken and the set re-run.

modules/buffers/include/System/Buffers/ReadOnlySequenceSegment.hpp

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,28 @@ namespace System::Buffers {
2020
* `getIsSingleSegmentProperty()` is a hard-coded `true`. Linking nodes therefore builds a
2121
* list nothing consumes (SR-AUD-087).
2222
*
23-
* The node shape is retained so that ported C# code declaring segment types still
24-
* compiles. Implementing the segment-chain constructor is blocked ticket **#2058**: it
25-
* needs new public members on `ReadOnlySequence<T>` (an object-layout change) plus
26-
* multi-segment rewrites of `First`, all six `Slice` overloads, both `GetPosition`
27-
* overloads, `TryGet`, `ToArray`, `CopyTo` and the enumerator, and of
28-
* `SequenceReader<T>`'s single-segment snapshot. See
29-
* docs/BuffersNamespaceReviewPlan.md §4.8. The current limitation is pinned by a
30-
* permanent test.
23+
* @note **DECLARED LIMITATION (SR-AUD-087, ticket #2058, decided 2026-08-19).** This is not a
24+
* pending repair: **this port models single-segment sequences only**, and multi-segment support
25+
* was offered and declined.
26+
*
27+
* .NET's contract is the opposite and is worth stating exactly, so the gap is explicit rather than
28+
* inferred: `ReadOnlySequence<T>` has
29+
* `ReadOnlySequence(ReadOnlySequenceSegment<T> startSegment, int startIndex,
30+
* ReadOnlySequenceSegment<T> endSegment, int endIndex)`
31+
* (`ReadOnlySequence.cs:94`), and its `IsSingleSegment` is `_startObject == _endObject`
32+
* (`ReadOnlySequence.cs:41-45`) — a real question with a real answer. Here it is a hard-coded
33+
* `true`.
34+
*
35+
* **What it would have cost, measured**: new public members on `ReadOnlySequence<T>` — a public
36+
* object-layout change on a type consumers hold by value — plus multi-segment rewrites of `First`,
37+
* all six `Slice` overloads, both `GetPosition` overloads, `TryGet`, `ToArray`, `CopyTo` and the
38+
* enumerator, and of `SequenceReader<T>`'s single-segment snapshot. See
39+
* docs/BuffersNamespaceReviewPlan.md §4.8.
40+
*
41+
* **The node shape is retained deliberately**, so that ported C# code declaring segment types
42+
* still compiles: a chain can be built and walked through `Next`/`RunningIndex`/`Memory`, it
43+
* simply cannot be handed to a `ReadOnlySequence<T>`. Pinned by
44+
* `ReadOnlySequenceSegmentPinTests`, written to fail the moment such a constructor appears.
3145
*
3246
* @tparam T The element type.
3347
*/

modules/buffers/tests/System/Buffers/BuffersContractPinTests.cpp

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ TEST(Fix2057_EveryBufferBackedConstructionYieldsASegment, IncludingTheDegenerate
201201
}
202202

203203
// ===========================================================================
204-
// SR-AUD-087 — the segment chain builds nothing (blocked #2058)
204+
// SR-AUD-087 — the segment chain builds nothing (#2058: DECLARED, not blocked)
205205
// ===========================================================================
206206

207207
namespace {
@@ -229,21 +229,36 @@ TEST(ReadOnlySequenceSegmentPinTests, ANodeChainCanBeBuiltButNotConsumed) {
229229
EXPECT_EQ(first.getMemoryProperty().getLengthProperty(), 2);
230230

231231
// ...and there is nothing that turns it into a sequence. ReadOnlySequence<int> has
232-
// exactly three constructors -- default, std::vector and raw pointer/length -- and
233-
// none of them accepts a segment. #2058 would add one; until then this static_assert
234-
// is the pin, and it fails the moment such a constructor appears.
232+
// exactly three constructors -- default, std::vector and raw pointer/length -- and none of
233+
// them accepts a segment.
234+
//
235+
// #2058 DECIDED on 2026-08-19: this is a DECLARED LIMITATION, not a pending repair. .NET has
236+
// ReadOnlySequence(startSegment, startIndex, endSegment, endIndex) (ReadOnlySequence.cs:94);
237+
// adding it here would be a public object-layout change on a type consumers hold by value,
238+
// plus multi-segment rewrites of First, six Slice overloads, two GetPosition overloads,
239+
// TryGet, ToArray, CopyTo, the enumerator and SequenceReader's snapshot. That was offered and
240+
// declined. These static_asserts are the declaration and they fail the moment it is withdrawn.
235241
static_assert(!std::is_constructible_v<ReadOnlySequence<int>,
236242
ReadOnlySequenceSegment<int>*, intcs,
237243
ReadOnlySequenceSegment<int>*, intcs>,
238-
"a segment-chain constructor now exists -- #2058 has landed");
244+
"a segment-chain constructor now exists -- #2058's declared limitation "
245+
"has been withdrawn and the header note must be rewritten");
239246
static_assert(!std::is_constructible_v<ReadOnlySequence<int>,
240247
ReadOnlySequenceSegment<int>*>,
241-
"a segment constructor now exists -- #2058 has landed");
248+
"a segment constructor now exists -- see above");
242249
}
243250

244251
TEST(ReadOnlySequenceSegmentPinTests, EverySequenceReportsASingleSegment) {
252+
// In .NET this is a real question -- IsSingleSegment is `_startObject == _endObject`
253+
// (ReadOnlySequence.cs:41-45) -- and here it is a hard-coded true. #2058 declares that, so the
254+
// pin asserts it holds for EVERY constructor this port offers rather than for a sample: an
255+
// answer that is constant is only demonstrably constant if every door is tried.
256+
std::vector<int> raw{1, 2, 3};
245257
EXPECT_TRUE(ReadOnlySequence<int>().getIsSingleSegmentProperty());
246258
EXPECT_TRUE(ReadOnlySequence<int>(std::vector<int>{1, 2, 3}).getIsSingleSegmentProperty());
259+
EXPECT_TRUE(ReadOnlySequence<int>(std::vector<int>{}).getIsSingleSegmentProperty());
260+
EXPECT_TRUE(ReadOnlySequence<int>(raw.data(), 3).getIsSingleSegmentProperty());
261+
EXPECT_TRUE(ReadOnlySequence<int>(raw.data(), 0).getIsSingleSegmentProperty());
247262
}
248263

249264
// ===========================================================================

modules/core/include/System/MarshalByRefObject.hpp

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,16 @@ namespace System {
2424
* - **`GetLifetimeService()` is present and throws**, as .NET's does. Its absence turned an
2525
* observable runtime diagnostic into a compile error at an unrelated place.
2626
*
27-
* @par What ticket #2297 deliberately did NOT change, and why
28-
* - **`InitializeLifetimeService()` is still absent.** .NET's is `virtual`, and this class
29-
* already has a vtable (the destructor), so adding it inserts a **slot** — a vtable change
30-
* in this class and in both derived ones, `AppDomain` and `ContextBoundObject`.
31-
* `docs/StandingApprovals.md` SA-3 excludes vtable changes explicitly, so this needs its own
32-
* approval and is ticket **#2374**. It is left absent rather than added non-virtually,
27+
* @par What ticket #2374 added afterwards
28+
* - **`InitializeLifetimeService()` is present and `virtual`**, matching .NET
29+
* (`MarshalByRefObject.cs:22-26`). #2297 left it absent because a virtual inserts a vtable
30+
* **slot** in this class and in both derived ones, `AppDomain` and `ContextBoundObject`,
31+
* which SA-3 excludes; the approval was granted per action on 2026-08-19 on the measurement
32+
* that both downstream consumers have **zero** sites. It is `virtual` rather than plain
3333
* because a non-virtual member of that name would silently break the one thing the .NET
3434
* member is for: letting a derived type override the lease policy.
35+
*
36+
* @par What ticket #2297 deliberately did NOT change, and why
3537
* - **`MemberwiseClone(bool)` is still absent.** .NET's calls `Object.MemberwiseClone()`, and
3638
* `System::Object` in this port declares no such member — measured. Adding it would be an
3739
* invention rather than a port, which SA-5 forbids.
@@ -78,6 +80,41 @@ namespace System {
7880
[[noreturn]] void* GetLifetimeService() const {
7981
throw PlatformNotSupportedException("Remoting is not supported on this platform.");
8082
}
83+
84+
/**
85+
* @brief Obtains a lifetime service object to control the lifetime policy for this
86+
* instance.
87+
*
88+
* C++ counterpart of .NET `MarshalByRefObject.InitializeLifetimeService()`
89+
* (`MarshalByRefObject.cs:22-26`), which throws
90+
* `PlatformNotSupportedException(SR.PlatformNotSupported_Remoting)`.
91+
*
92+
* @note **`virtual`, and that is the whole point of the member** — ticket **#2374**,
93+
* approved per action on 2026-08-19. .NET declares it `public virtual` so a derived type
94+
* can override the lease policy; a **non-virtual** member of the same name would compile,
95+
* look correct at every call site, and silently defeat the one thing the member exists
96+
* for. Ticket #2297 left it **absent** rather than add it non-virtually, for exactly that
97+
* reason, and pinned the absence with a detection idiom.
98+
*
99+
* The cost, accepted with the approval: this class already has a vtable (the destructor),
100+
* so adding a virtual inserts a **slot** here **and in both derived classes**,
101+
* `System::AppDomain` and `System::ContextBoundObject`. That is a silent binary break —
102+
* nothing fails to compile and every consumer must rebuild. Measured at the time of the
103+
* grant: **zero** `MarshalByRefObject`, `AppDomain` and `ContextBoundObject` sites in
104+
* `cna` and **zero** in `mobile-eggbert`.
105+
*
106+
* @note The return type is `void*` rather than `System::Object*`, matching the shape
107+
* `GetLifetimeService()` already uses here: .NET returns `object`, and this port's
108+
* `System::Object` is an abstract class that a lease object would have to derive from —
109+
* which is surface this port does not have and #2374 does not invent. Since the body
110+
* never returns, no caller can observe the difference.
111+
*
112+
* @return Never returns.
113+
* @throws System::PlatformNotSupportedException always.
114+
*/
115+
[[noreturn]] virtual void* InitializeLifetimeService() {
116+
throw PlatformNotSupportedException("Remoting is not supported on this platform.");
117+
}
81118
};
82119

83120
} // namespace System

modules/core/tests/System/Batch3TypeTests.cpp

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ TEST(MarshalByRefObjectNewTests, Fix2297_TheBaseIsNoLongerDirectlyConstructible)
108108
EXPECT_NO_THROW(Derived obj; (void)obj);
109109
}
110110

111-
TEST(MarshalByRefObjectNewTests, Fix2297_GetLifetimeServiceThrowsAndTheVirtualOneIsStillAbsent) {
111+
TEST(MarshalByRefObjectNewTests, Fix2297_GetLifetimeServiceThrows_And2374_AddedTheVirtualOne) {
112112
// GetLifetimeService() is added: .NET keeps it precisely so a caller receives
113113
// PlatformNotSupportedException (`MarshalByRefObject.cs:17-21`), and its absence here turned
114114
// that observable diagnostic into a compile error at an unrelated place. It is NOT virtual in
@@ -123,16 +123,58 @@ TEST(MarshalByRefObjectNewTests, Fix2297_GetLifetimeServiceThrowsAndTheVirtualOn
123123
EXPECT_STREQ(e.what(), "Remoting is not supported on this platform.");
124124
}
125125

126-
// InitializeLifetimeService() is DELIBERATELY still absent: .NET's is virtual, and this class
127-
// already has a vtable, so adding it inserts a SLOT -- a vtable change here and in both
128-
// derived classes. SA-3 excludes those explicitly. Ticket #2374 carries the approval request,
129-
// and this assertion is what makes its arrival visible rather than silent.
130-
// The `requires` form needs the name to be findable at all, so it is written as a
131-
// detection idiom over a dependent type instead -- an absent member must be expressible as
126+
// INVERTED BY #2374 (2026-08-19). Its predecessor asserted that InitializeLifetimeService()
127+
// was ABSENT and said "this assertion is what makes its arrival visible rather than silent" --
128+
// the approval was granted per action, so it now records the arrival.
129+
//
130+
// The detection idiom is KEPT rather than replaced by a direct call, because it is what makes
131+
// presence and absence expressible in the same form: an absent member must be expressible as
132132
// absent, not as a hard error.
133-
static_assert(!detail2297::HasInitializeLifetimeService<Derived>,
134-
"#2374 landed: InitializeLifetimeService() is present, so re-derive the vtable "
135-
"pins in AppDomain and ContextBoundObject");
133+
static_assert(detail2297::HasInitializeLifetimeService<Derived>,
134+
"#2374: InitializeLifetimeService() is present");
135+
136+
// VIRTUAL, and that is the whole point -- a non-virtual member of the same name would satisfy
137+
// the detection idiom above, compile at every call site, and silently defeat the one thing
138+
// .NET's member exists for: letting a derived type override the lease policy. So presence is
139+
// not enough to assert; overridability is.
140+
static_assert(std::is_polymorphic_v<System::MarshalByRefObject>);
141+
EXPECT_THROW((void)obj.InitializeLifetimeService(), System::PlatformNotSupportedException);
142+
try {
143+
(void)obj.InitializeLifetimeService();
144+
ADD_FAILURE() << "expected PlatformNotSupportedException";
145+
} catch (const System::PlatformNotSupportedException& e) {
146+
EXPECT_STREQ(e.what(), "Remoting is not supported on this platform.");
147+
}
148+
}
149+
150+
namespace detail2374 {
151+
/// A derived type that OVERRIDES the lease policy -- the capability #2374 bought, and the one
152+
/// a non-virtual member would have silently withheld.
153+
struct LeaseOverriding : System::MarshalByRefObject {
154+
mutable bool called = false;
155+
void* InitializeLifetimeService() override {
156+
called = true;
157+
return nullptr; // deliberately does NOT throw
158+
}
159+
};
160+
}
161+
162+
TEST(MarshalByRefObjectTests, Fix2374_ADerivedTypeCanOverrideTheLeasePolicy) {
163+
// This is what the vtable slot was paid for. Called through a BASE reference, so it is
164+
// dynamic dispatch that is asserted rather than a same-name member found by static lookup --
165+
// a non-virtual member would run the BASE body here and throw.
166+
detail2374::LeaseOverriding derived;
167+
System::MarshalByRefObject& asBase = derived;
168+
EXPECT_NO_THROW((void)asBase.InitializeLifetimeService());
169+
EXPECT_TRUE(derived.called) << "#2374: the override must be dispatched through the base";
170+
171+
// And the base's own body still throws, so the override is a real replacement rather than the
172+
// base having stopped throwing.
173+
struct Plain : System::MarshalByRefObject {};
174+
Plain plain;
175+
System::MarshalByRefObject& plainAsBase = plain;
176+
EXPECT_THROW((void)plainAsBase.InitializeLifetimeService(),
177+
System::PlatformNotSupportedException);
136178
}
137179

138180
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)