fix(core): MarshalByRefObject's constructor is protected, and GetLifetimeService returns (#2297)

Two of the ticket's three parts, under SA-8 and SA-9. The third is split out as
#2374 because it is a VTABLE change, which SA-3 excludes explicitly.

(1) The constructor is protected, matching .NET's 'protected
MarshalByRefObject()' -- C# rejects the equivalent with CS0144 -- AND so are the
copy and move members, closing the slicing route the review did not enumerate.
Two in-repo construction sites migrated, in two different executables as the
review predicted.

(2) GetLifetimeService() is added and throws PlatformNotSupportedException with
.NET's own sentence. Its absence turned an OBSERVABLE RUNTIME DIAGNOSTIC INTO A
COMPILE ERROR at an unrelated place, which is the review's own point. It is NOT
virtual in .NET, so adding it costs no vtable slot -- exactly why this half could
land and the other could not.

(3) InitializeLifetimeService() is deliberately still absent (#2374). .NET's is
virtual, and this class already has a vtable, so adding it inserts a slot here
and in both derived classes. IT MUST NOT BE ADDED NON-VIRTUALLY AS A WORKAROUND:
a non-virtual member of that name would silently defeat the one thing .NET's
exists for -- letting a derived type override the lease policy -- while looking
correct at every call site. Its absence is pinned through a detection idiom, so
#2374's arrival is visible rather than silent.

MemberwiseClone(bool) stays absent: System::Object declares no MemberwiseClone,
measured, so adding it would be an invention rather than a port. No
[[deprecated]] either -- that is #2289's undecided question.

Fixture set: 29 fixtures / 174 sites. Gate 17,280 run, 0 failed. Downstream:
zero sites in either consumer.

Author: robertvokac

CLAUDE.md

@@ -0,0 +1,68 @@
+<!-- SPDX-License-Identifier: MIT -->
+<!-- Copyright (c) Robert Vokac and contributors -->
+
+# Migration — `MarshalByRefObject` is no longer directly constructible (ticket #2297)
+
+*2026-08-18.* `System::MarshalByRefObject obj;` compiled. .NET declares the class `abstract` with
+a `protected` constructor, so C# rejects the equivalent with `CS0144`.
+
+Landed under `docs/StandingApprovals.md` **SA-8** and **SA-9** with SA-2's five conditions.
+**Two of the ticket's three parts landed; the third needs a vtable approval** — see §3.
+
+---
+
+## 1. What changed
+
+| Spelling | Was | Is |
+|---|---|---|
+| `System::MarshalByRefObject obj;` | compiled | **rejected** |
+| `new System::MarshalByRefObject()` | compiled | **rejected** |
+| `MarshalByRefObject sliced = derived;` | compiled | **rejected** |
+| `GetLifetimeService()` | **absent** | present, throws `PlatformNotSupportedException` |
+| `class X : public MarshalByRefObject {}` | — | **unchanged**, and the only intended use |
+| `AppDomain`, `ContextBoundObject` | — | **unchanged** |
+
+`GetLifetimeService()`'s absence turned an **observable runtime diagnostic into a compile error at
+an unrelated place** — .NET keeps it precisely so a caller gets
+`PlatformNotSupportedException("Remoting is not supported on this platform.")`. It is **not**
+`virtual` in .NET, so adding it costs no vtable slot.
+
+## 2. To migrate
+
+Derive:
+
+```cpp
+// before
+System::MarshalByRefObject obj;
+
+// after
+class MyRemotable : public System::MarshalByRefObject {};
+MyRemotable obj;
+```
+
+A reference or pointer to the base is unaffected.
+
+## 3. What did NOT land, and why
+
+**`InitializeLifetimeService()` is still absent.** .NET's is `virtual`, and this class already has
+a vtable (the destructor), so adding it inserts a **slot** — a vtable change here **and in both
+derived classes**, `AppDomain` and `ContextBoundObject`. `docs/StandingApprovals.md` SA-3 excludes
+vtable changes explicitly, so it needs its own approval: **ticket #2374**.
+
+It is left absent rather than added **non-virtually**, because a non-virtual member of that name
+would silently defeat the one thing the .NET member exists for — letting a derived type override
+the lease policy. A test asserts its absence through a detection idiom, so #2374's arrival is
+visible rather than silent.
+
+**`MemberwiseClone(bool)` is still absent.** .NET's calls `Object.MemberwiseClone()`, and
+`System::Object` in this port declares no such member — measured. Adding it would be an invention
+rather than a port, which SA-5 forbids.
+
+**No `[[deprecated]]`.** Both .NET members carry `[Obsolete(RemotingApisMessage)]`. Whether .NET's
+`Obsolete` becomes C++ `[[deprecated]]` anywhere in this repository is the undecided ticket
+**#2289**, and answering it incidentally here would settle it in the wrong place.
+
+## 4. Downstream, measured
+
+Neither `cna` nor `mobile-eggbert` mentions `MarshalByRefObject` — **zero sites in both**. Neither
+repository was modified.

audit/AUDIT_FINDINGS_INDEX.md

@@ -3,41 +3,81 @@
 // Portions based on .NET runtime API (MIT License, Copyright .NET Foundation and Contributors)
 #pragma once
 
+#include "System/PlatformNotSupportedException.hpp"
+
 namespace System {
 
     /**
      * @brief Enables access to objects across application domain boundaries in applications
      * that support remoting.
      *
-     * C++ counterpart of .NET System.MarshalByRefObject.
-     * In this port the class serves as a base-class marker; full remoting is not implemented.
+     * C++ counterpart of .NET `System.MarshalByRefObject`
+     * (`MarshalByRefObject.cs:10-33`), which is `public abstract` with a `protected`
+     * constructor and whose two lifetime members exist only to throw
+     * `PlatformNotSupportedException(SR.PlatformNotSupported_Remoting)`. Remoting is not
+     * implemented in this port and never will be.
      *
-     * @warning **This class's public shape is not .NET's, in two ways that are
-     * observable rather than cosmetic** (SR-AUD-128; the shape decision is
-     * ticket #2297 and neither divergence is repaired by this note).
+     * @par What ticket #2297 changed
+     * - **The constructor is `protected`**, matching .NET's, so `System::MarshalByRefObject obj;`
+     *   no longer compiles — C# rejects the equivalent with `CS0144`. The copy and move members
+     *   are protected with it, so the base cannot be reached by a slice either.
+     * - **`GetLifetimeService()` is present and throws**, as .NET's does. Its absence turned an
+     *   observable runtime diagnostic into a compile error at an unrelated place.
      *
-     * - **It is directly constructible.** `System::MarshalByRefObject obj;`
-     *   compiles here; .NET declares the class `abstract` with a `protected`
-     *   constructor, so the C# equivalent is rejected outright (`CS0144`). Two
-     *   in-repo tests construct the base today, so closing this is a public
-     *   source break, not a tidy-up.
-     * - **Three .NET members are absent**, not merely unimplemented:
-     *   `GetLifetimeService()`, the virtual `InitializeLifetimeService()` and
-     *   the protected `MemberwiseClone(bool)`. Current .NET keeps the first two
-     *   specifically so that a caller receives `PlatformNotSupportedException`;
-     *   removing them replaces that observable diagnostic with a compile error
-     *   at an unrelated place. Adding them back is additive but introduces a
-     *   virtual, which changes this class's vtable and that of every derived
-     *   class — `AppDomain` and `ContextBoundObject` in this repository.
+     * @par What ticket #2297 deliberately did NOT change, and why
+     * - **`InitializeLifetimeService()` is still absent.** .NET's is `virtual`, and this class
+     *   already has a vtable (the destructor), so adding it inserts a **slot** — a vtable change
+     *   in this class and in both derived ones, `AppDomain` and `ContextBoundObject`.
+     *   `docs/StandingApprovals.md` SA-3 excludes vtable changes explicitly, so this needs its own
+     *   approval and is ticket **#2374**. It is left absent rather than added non-virtually,
+     *   because a non-virtual member of that name would silently break the one thing the .NET
+     *   member is for: letting a derived type override the lease policy.
+     * - **`MemberwiseClone(bool)` is still absent.** .NET's calls `Object.MemberwiseClone()`, and
+     *   `System::Object` in this port declares no such member — measured. Adding it would be an
+     *   invention rather than a port, which SA-5 forbids.
+     * - **No `[[deprecated]]`.** Both .NET members carry `[Obsolete(RemotingApisMessage)]`;
+     *   whether .NET's `Obsolete` becomes C++ `[[deprecated]]` anywhere in this repository is the
+     *   undecided ticket #2289, and answering it incidentally here would settle it in the wrong
+     *   place.
      *
-     * What the port does provide is correct as far as it goes: the virtual
-     * destructor makes polymorphic deletion through this base well defined,
-     * which is what the derived types rely on.
+     * The virtual destructor makes polymorphic deletion through this base well defined, which is
+     * what the derived types rely on.
      */
     class MarshalByRefObject {
+    protected:
+        /**
+         * @brief Protected default constructor, matching .NET's `protected MarshalByRefObject()`.
+         *
+         * Ticket #2297. The copy and move members are protected with it so the base cannot be
+         * reached by a slice.
+         */
+        MarshalByRefObject() = default;
+        MarshalByRefObject(const MarshalByRefObject&) = default;
+        MarshalByRefObject(MarshalByRefObject&&) = default;
+        MarshalByRefObject& operator=(const MarshalByRefObject&) = default;
+        MarshalByRefObject& operator=(MarshalByRefObject&&) = default;
+
     public:
         /** @brief Virtual destructor to allow safe polymorphic deletion. */
         virtual ~MarshalByRefObject() = default;
+
+        /**
+         * @brief Always throws; remoting is not supported.
+         *
+         * C++ counterpart of .NET `MarshalByRefObject.GetLifetimeService()`
+         * (`MarshalByRefObject.cs:17-21`), which throws
+         * `PlatformNotSupportedException(SR.PlatformNotSupported_Remoting)`. It is **not**
+         * `virtual` there, so adding it here costs no vtable slot.
+         *
+         * Added by ticket #2297: its absence turned an observable runtime diagnostic into a
+         * compile error at an unrelated place.
+         *
+         * @return Never returns.
+         * @throws System::PlatformNotSupportedException always.
+         */
+        [[noreturn]] void* GetLifetimeService() const {
+            throw PlatformNotSupportedException("Remoting is not supported on this platform.");
+        }
     };
 
 } // namespace System

docs/Migration-MarshalByRefObjectProtectedConstructor.md

@@ -5,6 +5,7 @@
 //   MidpointRounding, UInt128, MarshalByRefObject, EventHandler,
 //   ReadOnlyMemory, Activator, ThreadStaticAttribute, FlagsAttribute
 #include <gtest/gtest.h>
+#include "System/PlatformNotSupportedException.hpp"
 #include <limits>
 #include <type_traits>
 #include <vector>
@@ -86,8 +87,52 @@ TEST(MarshalByRefObjectNewTests, PolymorphicDeletion_NoLeak) {
     std::unique_ptr<System::MarshalByRefObject> p = std::make_unique<Derived>();
     EXPECT_NE(p.get(), nullptr);
 }
-TEST(MarshalByRefObjectNewTests, DefaultCtor_DoesNotThrow) {
-    EXPECT_NO_THROW(System::MarshalByRefObject obj);
+namespace detail2297 {
+/// Detection idiom: true only if `T` has a callable `InitializeLifetimeService()`. Written as a
+/// concept over a dependent expression so that the member's ABSENCE is a value rather than a
+/// compile error -- which is what makes the assertion below expressible at all.
+template <typename T>
+concept HasInitializeLifetimeService = requires(T& t) { t.InitializeLifetimeService(); };
+}  // namespace detail2297
+
+TEST(MarshalByRefObjectNewTests, Fix2297_TheBaseIsNoLongerDirectlyConstructible) {
+    // #2297 made the constructor protected, matching .NET's `protected MarshalByRefObject()`;
+    // C# rejects the equivalent with CS0144. This test used to construct the base directly.
+    struct Derived : System::MarshalByRefObject {};
+    static_assert(!std::is_default_constructible_v<System::MarshalByRefObject>,
+                  "#2297: the constructor is protected");
+    static_assert(!std::is_copy_constructible_v<System::MarshalByRefObject>,
+                  "#2297: and the copy member went with it, so the base cannot be sliced out");
+    static_assert(std::is_default_constructible_v<Derived>,
+                  "a derived type is still constructible -- that is the point");
+    EXPECT_NO_THROW(Derived obj; (void)obj);
+}
+
+TEST(MarshalByRefObjectNewTests, Fix2297_GetLifetimeServiceThrowsAndTheVirtualOneIsStillAbsent) {
+    // GetLifetimeService() is added: .NET keeps it precisely so a caller receives
+    // PlatformNotSupportedException (`MarshalByRefObject.cs:17-21`), and its absence here turned
+    // that observable diagnostic into a compile error at an unrelated place. It is NOT virtual in
+    // .NET, so adding it costs no vtable slot.
+    struct Derived : System::MarshalByRefObject {};
+    Derived obj;
+    EXPECT_THROW((void)obj.GetLifetimeService(), System::PlatformNotSupportedException);
+    try {
+        (void)obj.GetLifetimeService();
+        ADD_FAILURE() << "expected PlatformNotSupportedException";
+    } catch (const System::PlatformNotSupportedException& e) {
+        EXPECT_STREQ(e.what(), "Remoting is not supported on this platform.");
+    }
+
+    // InitializeLifetimeService() is DELIBERATELY still absent: .NET's is virtual, and this class
+    // already has a vtable, so adding it inserts a SLOT -- a vtable change here and in both
+    // derived classes. SA-3 excludes those explicitly. Ticket #2374 carries the approval request,
+    // and this assertion is what makes its arrival visible rather than silent.
+    // The `requires` form needs the name to be findable at all, so it is written as a
+    // detection idiom over a dependent type instead -- an absent member must be expressible as
+    // absent, not as a hard error.
+    static_assert(!detail2297::HasInitializeLifetimeService<Derived>,
+                  "#2374 landed: InitializeLifetimeService() is present, so re-derive the vtable "
+                  "pins in AppDomain and ContextBoundObject");
 }
 
 // ---------------------------------------------------------------------------

modules/core/include/System/MarshalByRefObject.hpp

@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) Robert Vokac and contributors
+//
+// Negative compile fixture for ticket #2297 (SR-AUD-128).
+//
+// #2297 made System::MarshalByRefObject's constructor protected, matching
+// .NET's `protected MarshalByRefObject()` -- C# rejects the equivalent with
+// CS0144 -- and added GetLifetimeService(), which .NET keeps precisely so that
+// a caller receives PlatformNotSupportedException rather than a compile error
+// at an unrelated place.
+//
+// What it deliberately did NOT add is InitializeLifetimeService(). .NET's is
+// VIRTUAL, and this class already has a vtable (the destructor), so adding it
+// inserts a SLOT -- a vtable change here and in both derived classes, AppDomain
+// and ContextBoundObject. docs/StandingApprovals.md SA-3 excludes vtable
+// changes explicitly. Ticket #2374 carries that approval request.
+//
+// Records: docs/Migration-MarshalByRefObjectProtectedConstructor.md,
+// docs/NegativeConsumerFixtureValidation.md.
+//
+// NEGATIVE-FIXTURE: component=Core.Base
+#include <type_traits>
+
+#include "System/MarshalByRefObject.hpp"
+#include "System/PlatformNotSupportedException.hpp"
+
+#ifndef SHARP_RUNTIME_NEGATIVE_SITE
+#define SHARP_RUNTIME_NEGATIVE_SITE 0
+#endif
+
+using System::MarshalByRefObject;
+
+namespace {
+/// The migrated shape: derive, which is what a remotable type always had to do.
+class MyRemotable : public MarshalByRefObject {};
+}  // namespace
+
+int main() {
+#if SHARP_RUNTIME_NEGATIVE_SITE == 1
+    // NEGATIVE(marshalbyrefobject-direct-instantiation): is protected within this context
+    //     | protected
+    MarshalByRefObject direct;
+    (void)direct;
+#else
+    MyRemotable direct;
+    (void)direct;
+#endif
+
+#if SHARP_RUNTIME_NEGATIVE_SITE == 2
+    // NEGATIVE(marshalbyrefobject-heap-instantiation): is protected within this context
+    //     | protected
+    MarshalByRefObject* heap = new MarshalByRefObject();
+    delete heap;
+#else
+    MarshalByRefObject* heap = new MyRemotable();
+    delete heap;
+#endif
+
+#if SHARP_RUNTIME_NEGATIVE_SITE == 3
+    // NEGATIVE(marshalbyrefobject-copy-slice): is protected within this context
+    //     | protected
+    //     | use of deleted function
+    MyRemotable derived;
+    MarshalByRefObject sliced = derived;
+    (void)sliced;
+#else
+    MyRemotable derived;
+    const MarshalByRefObject& notSliced = derived;
+    (void)notSliced;
+#endif
+
+#if SHARP_RUNTIME_NEGATIVE_SITE == 4
+    // NEGATIVE(marshalbyrefobject-still-default-constructible): static assertion failed
+    //     | static_assert
+    // The shape that breaks a consumer SILENTLY: a trait query.
+    static_assert(std::is_default_constructible_v<MarshalByRefObject>,
+                  "MarshalByRefObject is expected to be default-constructible");
+#else
+    static_assert(!std::is_default_constructible_v<MarshalByRefObject>,
+                  "#2297: the constructor is protected, as .NET's is");
+    static_assert(std::is_default_constructible_v<MyRemotable>,
+                  "#2297: a derived type is still constructible -- that is the point");
+#endif
+
+    // UNCHANGED / ADDED, and asserted so the fixture proves what the ticket DID provide:
+    // GetLifetimeService() exists and is reachable from a derived type. It is not virtual in
+    // .NET, so adding it cost no vtable slot.
+    try {
+        (void)derived.GetLifetimeService();
+        return 1;   // must not be reached
+    } catch (const System::PlatformNotSupportedException&) {
+        return 0;
+    }
+}

modules/core/tests/System/Batch3TypeTests.cpp

@@ -1459,8 +1459,11 @@ TEST(PredicateTests, Predicate_IsEven) {
 // MarshalByRefObject
 // ===========================================================================
 
-TEST(MarshalByRefObjectTests, Instantiation_NoThrow) {
-    EXPECT_NO_THROW(System::MarshalByRefObject obj);
+TEST(MarshalByRefObjectTests, Fix2297_DerivedInstantiation_NoThrow) {
+    // #2297 made the base's constructor protected, matching .NET. Deriving is the only route,
+    // and the only one there ever should have been.
+    struct Derived : System::MarshalByRefObject {};
+    EXPECT_NO_THROW(Derived obj; (void)obj);
 }
 
 // ===========================================================================