Skip to content

Commit ac40187

Browse files
robertvokacrobertvokac
committed
refactor(runtime)!: reparent AmbiguousImplementationException and give the platform attributes a base (#1980 G-3, SA-15.3)
THE FIRST CHANGE TO LAND UNDER SA-15.3, which lifted SA-3's exclusion of vtable and base-class changes. IT CLOSES #1980: G-1, G-2, G-4 and G-5 landed 2026-08-19. AmbiguousImplementationException is `public sealed class ... : Exception` in .NET and derived from SystemException here, unsealed, without .NET's (message, inner) constructor (SR-AUD-158). All three are fixed. SA-15.3'S FOURTH CONDITION IS THE PART NO LAYOUT ASSERTION CAN SEE, and it was discharged by measurement rather than by assertion. The clause whose meaning moves is `catch (const System::SystemException&)` around code that can throw this type -- and THERE ARE ZERO SUCH CLAUSES ANYWHERE. The 17 first-party catch (SystemException) sites are exception-hierarchy tests for other types; cna's single one catches its own NoAudioHardwareException; and neither consumer names this type at all. The pin asserts ALL THREE rows -- SystemException, Exception, and the type itself -- so a later reparenting cannot quietly take the two that did not move. BEFORE G-3, FIVE OF .NET'S SIX PLATFORM ATTRIBUTES DERIVED FROM System::Attribute DIRECTLY AND EACH CARRIED ITS OWN COPY of platformName_ and its own getPlatformNameProperty() -- five duplicates of one fact, and no type through which a caller could handle "any platform attribute" at all. That is SR-AUD-163. OSPlatformAttribute is introduced (abstract, protected constructor, get-only PlatformName) and all five derive from it and are sealed, as .NET seals all six. protected, NOT private protected: C++ has no equivalent of C#'s "derived classes in the same assembly only", and this port has no assembly boundary to express the second half. protected keeps the base unconstructible from outside the hierarchy, which is the half that carries meaning; the assembly restriction is NOT EXPRESSIBLE AND IS NOT PRETENDED. TargetPlatformAttribute is .NET's sixth derived type and is absent here, stated so that five is not mistaken for the whole set. LAYOUTS MEASURED AFTER THE CHANGE RATHER THAN PREDICTED BEFORE IT (#1958's lesson), and NOTHING GREW. The exception stays 168 because SystemException adds no members of its own over Exception, so the reparenting moves it SIDEWAYS. The attributes stay put because platformName_ moved INTO the new base rather than being duplicated beside it -- which is the point of having a base. THE REBUILD IS REQUIRED BY THE VTABLE, NOT THE SIZE, and the pins assert relationships (sizeof(Supported...) == sizeof(OSPlatformAttribute)) rather than only literals, so a later member cannot hide behind a hand-updated number. Six mutations, ALL CAUGHT, five of them at compile time -- the only way C++ reports a shape, and the reason the pins are static_asserts. Reverting the base is caught TWICE OVER: the constructor delegations stop compiling and the static_assert fires. ONE CORRECTION OF MY OWN IS RECORDED RATHER THAN QUIETLY FIXED: a first version of the ticket note said G-4 was still outstanding. It was copied from the review plan's original table instead of from the ticket's own record, and corrected on re-reading it -- setIsOptionalProperty is gone and the header says so in terms. That is the same premise error this programme keeps correcting in other people's records. Downstream, measured: zero AmbiguousImplementationException and zero OSPlatformAttribute-family sites in cna and in mobile-eggbert; cna's one catch (SystemException) cannot receive this type. Recorded as #2413. A FULL CONSUMER REBUILD IS STILL REQUIRED, because both halves move a vtable. Gate 17,676 / 38 executables: 17,676 run, 17,676 passed, 0 failed, 0 skipped, recounted from the per-executable logs. +5 on 17,671, in SharpRuntimeTests_Runtime (199 -> 204); no other executable moved. Zero build warnings at --parallel 2; module boundaries green at 41/94.
1 parent d3b594e commit ac40187

8 files changed

Lines changed: 316 additions & 33 deletions

File tree

‎CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

‎NEXT.md‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33

44
# NEXT.md
55

6-
> **Test-count floor, 2026-08-20 — 17,671 / 38, AND THE GATE IS GREEN.** The complete
7-
> 38-executable gate reads **17,671 run: 17,671 passed, 0 failed, 0 skipped**, recounted from the
6+
> **Test-count floor, 2026-08-20 — 17,676 / 38, AND THE GATE IS GREEN.** The complete
7+
> 38-executable gate reads **17,676 run: 17,676 passed, 0 failed, 0 skipped**, recounted from the
88
> per-executable logs with every executable run separately and continuing past failures, zero build
99
> warnings at `--parallel 2`. Every checkpoint below this one ends with *"the gate is not green"*;
1010
> this one does not. Two of the three historical failure sources were environmental and are simply
@@ -126,9 +126,11 @@
126126
> `System::TimeZone::CurrentTimeZone()` is per-date — and that is exactly the zone these
127127
> conversions need, so the model was present all along.
128128
>
129-
> **Six `todo`**: #1942, #1943, #1944 (unblocked by phase 2), #1945, and #1980 / #1997 (unblocked by
130-
> SA-15.3). **Only three tickets remain blocked**, none of them on a decision this repository can
131-
> take: #1773 and #2381 wait on downstream/merge events, #1962 on `CAP_NET_RAW`.
129+
> **#1980 IS CLOSED** — G-3 landed as the first change under SA-15.3, and G-1/G-2/G-4/G-5 landed
130+
> 2026-08-19. **Four `todo`**: #1942, #1943, #1944 (unblocked by #1941 phase 2), #1945, plus #1997
131+
> (unblocked by SA-15.3, A-2/A-4 remaining). **Only three tickets remain blocked**, none of them on
132+
> a decision this repository can take: #1773 and #2381 wait on downstream or merge events, #1962 on
133+
> `CAP_NET_RAW`.
132134
>
133135
> **One thing #2406 deliberately did NOT do, so it is not mistaken for parity**: `DisplayAttribute`'s
134136
> eight fields stay public data members (correct — .NET's are `{ get; set; }`), but their

‎docs/Migration-RuntimeHierarchyG3.md‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
<!-- Copyright (c) Robert Vokac and contributors -->
3+
4+
# Migration — `System::Runtime` hierarchy changes (ticket #1980 group G-3, SA-15.3)
5+
6+
*2026-08-20.* The group SA-3 used to exclude. **SA-15.3 lifted that exclusion**, and this is the
7+
first change to land under it.
8+
9+
**FULL CONSUMER REBUILD REQUIRED.** Both halves move a **vtable**, even where `sizeof` does not
10+
change. The library ships as a static library built from source, so this forces a rebuild rather
11+
than breaking a distributed binary — but a stale object file is an ODR violation with no
12+
diagnostic.
13+
14+
**Downstream, measured**: **zero** `AmbiguousImplementationException` sites and **zero**
15+
`OSPlatformAttribute`-family sites in `cna` and in `mobile-eggbert`. Downstream record: **#2413**.
16+
17+
---
18+
19+
## 1. `AmbiguousImplementationException` — reparented and sealed
20+
21+
.NET's is `public sealed class AmbiguousImplementationException : Exception`. This port derived it
22+
from **`SystemException`** and left it open.
23+
24+
| | Was | Now (= .NET) |
25+
|---|---|---|
26+
| base | `System::SystemException` | `System::Exception` |
27+
| sealed | no | `final` |
28+
| `(message, inner)` | absent (SR-AUD-158) | present |
29+
30+
### 1.1 Which `catch` clauses change meaning — SA-15.3's fourth condition
31+
32+
This is the part **no layout assertion can see**, which is why the approval demands it be
33+
enumerated:
34+
35+
| Clause | Before | After |
36+
|---|---|---|
37+
| `catch (const System::SystemException&)` | **caught** this type | **does not** |
38+
| `catch (const System::Exception&)` | caught | caught |
39+
| `catch (const AmbiguousImplementationException&)` | caught | caught |
40+
41+
**Measured across the repository and both consumers: there are ZERO clauses whose behaviour
42+
actually changes.** The 17 first-party `catch (SystemException)` sites are exception-hierarchy tests
43+
for other types; `cna`'s single one catches its own `NoAudioHardwareException`; and **neither
44+
consumer names this type at all**. A future handler is warned by the header and by
45+
`RuntimeG3Tests.WhichCatchClausesChangedMeaning`, which asserts all three rows rather than the one
46+
that moved — so a later reparenting cannot quietly take the other two.
47+
48+
## 2. `OSPlatformAttribute` — one base instead of five duplicates
49+
50+
.NET declares `abstract class OSPlatformAttribute : Attribute` with a `private protected`
51+
constructor and a get-only `PlatformName`, and **six** sealed types derive from it. This port had
52+
**five of the six deriving from `System::Attribute` directly, each carrying its own copy of
53+
`platformName_` and its own `getPlatformNameProperty()`** — five duplicates of one fact, and **no
54+
type through which a caller could handle "any platform attribute"**. That is SR-AUD-163.
55+
56+
Now: `SupportedOSPlatformAttribute`, `UnsupportedOSPlatformAttribute`,
57+
`SupportedOSPlatformGuardAttribute`, `UnsupportedOSPlatformGuardAttribute` and
58+
`ObsoletedOSPlatformAttribute` all derive from `OSPlatformAttribute` and are `final`.
59+
60+
**`protected`, not `private protected`.** C++ has no equivalent of C#'s *"derived classes in the
61+
same assembly only"*, and this port has no assembly boundary to express the second half. `protected`
62+
keeps the base unconstructible from outside the hierarchy, which is the half that carries meaning;
63+
the assembly restriction is **not expressible and is not pretended**.
64+
65+
**`TargetPlatformAttribute` is .NET's sixth derived type and is absent here.** Stated so that five
66+
is not mistaken for the whole set; adding it is additive and outside G-3's wording.
67+
68+
## 3. Layouts — SA-15.3's first condition
69+
70+
Every figure measured **after** the change rather than predicted before it, which is #1958's lesson.
71+
72+
| Type | `sizeof` |
73+
|---|---|
74+
| `Exception`, `SystemException`, `AmbiguousImplementationException` | **168**, all three |
75+
| `Attribute` | 8 |
76+
| `OSPlatformAttribute` and the three simple derived ones | **40** |
77+
| `UnsupportedOSPlatformAttribute` | 72 (base + one `std::string`) |
78+
| `ObsoletedOSPlatformAttribute` | 104 |
79+
80+
**Nothing grew.** The exception stays 168 because `SystemException` adds no members of its own over
81+
`Exception`, so the reparenting moves the type **sideways**. The attributes stay put because
82+
`platformName_` moved **into** the new base rather than being duplicated beside it — which is the
83+
point of having a base. **The rebuild is required by the vtable, not by the size**, and the pins
84+
assert the relationships (`sizeof(Supported…) == sizeof(OSPlatformAttribute)`) rather than only the
85+
literals, so a later member cannot hide behind a hand-updated number.
86+
87+
## 4. What a caller changes
88+
89+
- `catch (const SystemException&)` intended to catch `AmbiguousImplementationException` becomes
90+
`catch (const Exception&)` or the type itself.
91+
- Deriving from any of the six affected types no longer compiles; .NET seals all of them.
92+
- Reading `getPlatformNameProperty()` is unchanged — it moved to the base, and every derived type
93+
still answers it.
94+
95+
## 5. Testing
96+
97+
Six mutations, **all caught, five of them at compile time** — which is the only way C++ reports a
98+
shape, and the reason the pins are `static_assert`s. Reverting the base is caught **twice over**:
99+
the constructor delegations stop compiling *and* the `static_assert` fires.

‎modules/runtime/include/System/Runtime/AmbiguousImplementationException.hpp‎

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,49 @@
22
// Copyright (c) Robert Vokac and contributors
33
// Portions based on .NET runtime API (MIT License, Copyright .NET Foundation and Contributors)
44
#pragma once
5+
#include <exception>
56
#include <string>
6-
#include "System/SystemException.hpp"
7+
#include "System/Exception.hpp"
78

89
namespace System::Runtime {
910

10-
class AmbiguousImplementationException : public System::SystemException {
11+
/**
12+
* @brief Thrown when the runtime cannot choose between two implementations.
13+
*
14+
* C++ counterpart of .NET `System.Runtime.AmbiguousImplementationException`
15+
* (`AmbiguousImplementationException.cs`), which is
16+
* `public sealed class AmbiguousImplementationException : Exception`.
17+
*
18+
* @note **#1980 G-3 reparented this from `SystemException` to `Exception` and sealed it, under
19+
* SA-15.3** — the approval that lifted SA-3's exclusion of vtable and base-class changes.
20+
*
21+
* @note **What that changes for a `catch`, enumerated rather than summarised**, which is
22+
* SA-15.3's fourth condition. The clause whose meaning moves is
23+
* `catch (const System::SystemException&)` around code that can throw this type: it used to
24+
* catch it and no longer does. **Measured across the repository and both consumers, there are
25+
* ZERO such clauses** — the 17 first-party `catch (SystemException)` sites are
26+
* exception-hierarchy tests for other types, and `cna`'s single one catches its own
27+
* `NoAudioHardwareException`; neither consumer names this type at all. So no existing handler
28+
* changes behaviour, and a future one is warned here. `catch (const System::Exception&)` and
29+
* `catch (const AmbiguousImplementationException&)` are unaffected.
30+
*
31+
* @note The third constructor is .NET's `(message, innerException)` overload
32+
* (`AmbiguousImplementationException.cs`), which SR-AUD-158 recorded as missing.
33+
*/
34+
class AmbiguousImplementationException final : public System::Exception {
1135
public:
1236
AmbiguousImplementationException()
13-
: System::SystemException("Ambiguous implementation found.") {
37+
: System::Exception("Ambiguous implementation found.") {
1438
setHResultProperty(static_cast<SharpRuntime::intcs>(0x8013106Au)); // COR_E_AMBIGUOUSIMPLEMENTATION
1539
}
1640
explicit AmbiguousImplementationException(const std::string& message)
17-
: System::SystemException(message) {
18-
setHResultProperty(static_cast<SharpRuntime::intcs>(0x8013106Au)); // COR_E_AMBIGUOUSIMPLEMENTATION
41+
: System::Exception(message) {
42+
setHResultProperty(static_cast<SharpRuntime::intcs>(0x8013106Au));
43+
}
44+
AmbiguousImplementationException(const std::string& message,
45+
std::exception_ptr innerException)
46+
: System::Exception(message, innerException) {
47+
setHResultProperty(static_cast<SharpRuntime::intcs>(0x8013106Au));
1948
}
2049
};
2150

‎modules/runtime/include/System/Runtime/Versioning/VersioningAttributes.hpp‎

Lines changed: 46 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -27,35 +27,67 @@ namespace System::Runtime::Versioning {
2727
};
2828

2929
/** Indicates that an API is supported on the specified OS platform. */
30-
class SupportedOSPlatformAttribute : public System::Attribute {
30+
/**
31+
* @brief The base every platform-name attribute shares. `PlatformAttributes.cs:1-18`.
32+
*
33+
* C++ counterpart of .NET `System.Runtime.Versioning.OSPlatformAttribute`, which is
34+
* `abstract class OSPlatformAttribute : Attribute` with a `private protected` constructor and
35+
* a get-only `public string PlatformName`.
36+
*
37+
* @note **Introduced by #1980 G-3 under SA-15.3**, the approval that lifted SA-3's exclusion of
38+
* base-class changes. Before it, the five platform attributes each derived from
39+
* `System::Attribute` directly and **each carried its own copy of `platformName_` and its own
40+
* `getPlatformNameProperty()`** — five duplicates of one fact, and no type through which a
41+
* caller could handle "any platform attribute" at all, which is what SR-AUD-163 named.
42+
*
43+
* @note **`protected`, not `private protected`.** C++ has no equivalent of C#'s
44+
* `private protected` (accessible to derived classes *in the same assembly only*), and this
45+
* port has no assembly boundary to express the second half of it. `protected` keeps the class
46+
* unconstructible from outside the hierarchy, which is the part that carries meaning here; the
47+
* assembly restriction is not expressible and is not pretended.
48+
*
49+
* @note **`TargetPlatformAttribute` is .NET's sixth derived type and is absent here**, and that
50+
* is stated so a later reader does not mistake five for the whole set. Adding it is additive
51+
* and outside G-3, whose wording is "introduce `OSPlatformAttribute` and reparent five
52+
* attributes".
53+
*/
54+
class OSPlatformAttribute : public System::Attribute {
3155
std::string platformName_;
56+
57+
protected:
58+
explicit OSPlatformAttribute(const std::string& platformName)
59+
: platformName_(platformName) {}
60+
61+
public:
62+
/** @return The platform name this attribute names. `PlatformAttributes.cs:17`. */
63+
[[nodiscard]] const std::string& getPlatformNameProperty() const { return platformName_; }
64+
};
65+
66+
class SupportedOSPlatformAttribute final : public OSPlatformAttribute {
3267
public:
3368
/** @param platformName Platform identifier (e.g. "windows", "linux10.0"). */
3469
explicit SupportedOSPlatformAttribute(const std::string& platformName)
35-
: platformName_(platformName) {}
70+
: OSPlatformAttribute(platformName) {}
3671

3772
/** @return The platform identifier. */
38-
[[nodiscard]] const std::string& getPlatformNameProperty() const { return platformName_; }
3973
};
4074

4175
/** Indicates that an API is not supported on the specified OS platform. */
42-
class UnsupportedOSPlatformAttribute : public System::Attribute {
43-
std::string platformName_;
76+
class UnsupportedOSPlatformAttribute final : public OSPlatformAttribute {
4477
std::string message_;
4578
public:
4679
/** @param platformName Platform identifier (e.g. "windows"). */
4780
explicit UnsupportedOSPlatformAttribute(const std::string& platformName)
48-
: platformName_(platformName) {}
81+
: OSPlatformAttribute(platformName) {}
4982

5083
/**
5184
* @param platformName Platform identifier (e.g. "windows").
5285
* @param message Optional message explaining the lack of support.
5386
*/
5487
UnsupportedOSPlatformAttribute(const std::string& platformName, const std::string& message)
55-
: platformName_(platformName), message_(message) {}
88+
: OSPlatformAttribute(platformName), message_(message) {}
5689

5790
/** @return The platform identifier. */
58-
[[nodiscard]] const std::string& getPlatformNameProperty() const { return platformName_; }
5991

6092
/** @return The explanatory message, or empty if not provided. */
6193
[[nodiscard]] const std::string& getMessageProperty() const { return message_; }
@@ -65,35 +97,30 @@ namespace System::Runtime::Versioning {
6597
* Annotates a custom guard field, property, or method with a supported platform name, for use
6698
* in conditionals/asserts that guard calls to platform-specific APIs.
6799
*/
68-
class SupportedOSPlatformGuardAttribute : public System::Attribute {
69-
std::string platformName_;
100+
class SupportedOSPlatformGuardAttribute final : public OSPlatformAttribute {
70101
public:
71102
/** @param platformName Platform identifier the guard indicates support for. */
72103
explicit SupportedOSPlatformGuardAttribute(const std::string& platformName)
73-
: platformName_(platformName) {}
104+
: OSPlatformAttribute(platformName) {}
74105

75106
/** @return The platform identifier. */
76-
[[nodiscard]] const std::string& getPlatformNameProperty() const { return platformName_; }
77107
};
78108

79109
/**
80110
* Annotates a custom guard field, property, or method with an unsupported platform name, for
81111
* use in conditionals/asserts that guard against calling unsupported platform-specific APIs.
82112
*/
83-
class UnsupportedOSPlatformGuardAttribute : public System::Attribute {
84-
std::string platformName_;
113+
class UnsupportedOSPlatformGuardAttribute final : public OSPlatformAttribute {
85114
public:
86115
/** @param platformName Platform identifier the guard indicates lack of support for. */
87116
explicit UnsupportedOSPlatformGuardAttribute(const std::string& platformName)
88-
: platformName_(platformName) {}
117+
: OSPlatformAttribute(platformName) {}
89118

90119
/** @return The platform identifier. */
91-
[[nodiscard]] const std::string& getPlatformNameProperty() const { return platformName_; }
92120
};
93121

94122
/** Indicates that an API has been obsoleted on the specified OS platform. */
95-
class ObsoletedOSPlatformAttribute : public System::Attribute {
96-
std::string platformName_;
123+
class ObsoletedOSPlatformAttribute final : public OSPlatformAttribute {
97124
std::string message_;
98125
std::string url_;
99126
public:
@@ -110,10 +137,9 @@ namespace System::Runtime::Versioning {
110137
*/
111138
explicit ObsoletedOSPlatformAttribute(const std::string& platformName,
112139
const std::string& message = {})
113-
: platformName_(platformName), message_(message) {}
140+
: OSPlatformAttribute(platformName), message_(message) {}
114141

115142
/** @return The platform identifier. */
116-
[[nodiscard]] const std::string& getPlatformNameProperty() const { return platformName_; }
117143

118144
/** @return The deprecation message, or empty if not provided. */
119145
[[nodiscard]] const std::string& getMessageProperty() const { return message_; }

0 commit comments

Comments
 (0)