Skip to content

Commit c5201fe

Browse files
committed
fix(core): every ArgIterator member throws, as .NET's does (#2276, SR-AUD-112)
The ticket asked whether the unreachable instance members should become reachable, become static, or stay. They STAY instance members, because .NET's are, and making them static would diverge from the very shape this stub exists to present. But ArgIterator.cs:10-58 settled something the ticket had not asked, and it is the larger half: EVERY member of .NET's portable ArgIterator throws PlatformNotSupportedException -- constructors, End, Equals, GetHashCode and the rest alike. THREE MEMBERS HERE RETURNED QUIETLY: End() was a silent no-op, Equals() returned false, GetHashCode() returned 0, and all three were noexcept. A caller who reached one received a PLAUSIBLE ANSWER where .NET reports an unsupported platform -- the worse of the two failures. Three further divergences the ticket did not name and the reference exposed: the exception type was NotSupportedException where .NET uses PlatformNotSupportedException; GetNextArgType() returned TypedReference where .NET returns RuntimeTypeHandle, which ONLY a type assertion can catch because the body throws either way; and the GetNextArg(RuntimeTypeHandle) overload was missing entirely. The messages were this port's own invention, one variant per member. .NET uses one sentence for every door, and it is transcribed. A test asserts the DERIVED exception type specifically, because PlatformNotSupportedException derives from NotSupportedException and a test catching only the base would pass either way -- which is exactly how the old type survived this long. Gate 17,283 -> 17,278: nine narrow cases replaced by four that assert more. Nothing disabled, weakened or skipped. Downstream: zero sites in either consumer.
1 parent 33e8fdd commit c5201fe

6 files changed

Lines changed: 199 additions & 72 deletions

File tree

CLAUDE.md

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

audit/AUDIT_FINDINGS_INDEX.md

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
<!-- Copyright (c) Robert Vokac and contributors -->
3+
4+
# Migration — every `ArgIterator` member throws, as .NET's does (ticket #2276)
5+
6+
*2026-08-18.* Three `ArgIterator` members returned quietly where .NET reports an unsupported
7+
platform, the exception type was wrong, one overload was missing, and one return type diverged.
8+
9+
Landed under `docs/StandingApprovals.md` **SA-9**, with **SA-10** for the `noexcept` drops and the
10+
return-type change, under SA-2's five conditions. **This decreases the test count by 5** — nine
11+
narrow cases replaced by four broader ones.
12+
13+
---
14+
15+
## 1. The open question, and how the reference answered it
16+
17+
The ticket asked whether the unreachable instance members should become **reachable**, become
18+
**static**, or **stay as they are**.
19+
20+
They stay instance members, because .NET's are, and making them `static` would diverge from the
21+
very shape this stub exists to present.
22+
23+
But `ArgIterator.cs:10-58` settled something the ticket had not asked: **every member of .NET's
24+
portable `ArgIterator` throws `PlatformNotSupportedException`** — constructors, `End`, `Equals`,
25+
`GetHashCode` and the rest alike.
26+
27+
## 2. What changed
28+
29+
| Member | Was | Is |
30+
|---|---|---|
31+
| `End()` | `noexcept`, **silent no-op** | throws `PlatformNotSupportedException` |
32+
| `Equals(const ArgIterator&)` | `noexcept`, returned **`false`** | throws |
33+
| `GetHashCode()` | `noexcept`, returned **`0`** | throws |
34+
| both constructors, `GetNextArg()`, `GetRemainingCount()` | threw `NotSupportedException` | throw **`PlatformNotSupportedException`** |
35+
| `GetNextArgType()` | returned `TypedReference` | returns **`RuntimeTypeHandle`** |
36+
| `GetNextArg(RuntimeTypeHandle)` | **absent** | present, throws |
37+
| the message | this port's own, one variant per member | **.NET's single sentence** |
38+
39+
The three quiet members are the important half: a caller who reached one received a **plausible
40+
answer** where .NET reports an unsupported platform, which is the worse of the two failures.
41+
42+
## 3. The exception type change is narrower than it looks
43+
44+
`PlatformNotSupportedException` derives from `NotSupportedException`, so existing `catch` blocks
45+
on the base still catch it. Only code that caught `NotSupportedException` **and inspected the
46+
message** is affected — and the message changed too, from this port's invented per-member text to
47+
.NET's single `SR.PlatformNotSupported_ArgIterator` sentence:
48+
49+
> ArgIterator is not supported on this platform.
50+
51+
A test asserts the **derived** type specifically, because a test that only caught the base would
52+
pass either way — which is exactly how the old type survived this long.
53+
54+
## 4. To migrate
55+
56+
Nothing constructs an `ArgIterator` successfully, in this port or in .NET, so there is no working
57+
code to migrate. If you caught `NotSupportedException` around one of the three formerly-quiet
58+
members without expecting a throw, you now get one — which is the point.
59+
60+
## 5. The test-count decrease
61+
62+
The gate moves **17,283 → 17,278**. Nine narrow cases (one per member, plus two message pins) were
63+
replaced by four that assert more: every door's exception, the derived type specifically, the
64+
added overload, and the two return types. Nothing was disabled, weakened or skipped.
65+
66+
## 6. Downstream, measured
67+
68+
Neither `cna` nor `mobile-eggbert` mentions `ArgIterator`**zero sites in both**. Neither
69+
repository was modified.

modules/core/include/System/ArgIterator.hpp

Lines changed: 61 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
// Portions based on .NET runtime API (MIT License, Copyright .NET Foundation and Contributors)
44
#pragma once
55

6-
#include "System/NotSupportedException.hpp"
6+
#include "System/PlatformNotSupportedException.hpp"
7+
#include "System/RuntimeTypeHandle.hpp"
78
#include "System/RuntimeArgumentHandle.hpp"
89
#include "System/TypedReference.hpp"
910

@@ -14,18 +15,30 @@ namespace System {
1415
*
1516
* C++ counterpart of .NET System.ArgIterator.
1617
*
17-
* **Status: STUB** — ArgIterator depends on the CLR `__arglist` keyword,
18-
* `RuntimeArgumentHandle`, and `TypedReference`, none of which exist in C++.
19-
* All methods throw NotSupportedException at runtime.
18+
* **Status: STUB, and .NET's own is one too.** `ArgIterator.cs:10-58` — the portable
19+
* implementation, which is the one a port must follow — throws
20+
* `PlatformNotSupportedException(SR.PlatformNotSupported_ArgIterator)` from **every single
21+
* member**, constructors included. The type depends on the CLR `__arglist` keyword, which
22+
* has no C++ counterpart, so there is nothing to implement on either side.
2023
*
21-
* @note Both constructors are `[[noreturn]]`, and declaring them suppresses the
22-
* implicit default constructor, so **no public construction of an ArgIterator can
23-
* succeed**: the instance members below are unreachable through the public API. That
24-
* is a consequence of the stub, not a separate contract, and whether those members
25-
* should become reachable, become `static`, or stay as they are is an open question
26-
* (ticket #2276). It is recorded here because a fixture that needs an instance has no
27-
* legitimate ordinary route to one, and reaching for raw storage instead is undefined
28-
* behaviour rather than a workaround (SR-AUD-112).
24+
* @par Ticket #2276 settled the open question, and the reference answered it
25+
* The question was whether the instance members should become reachable, become `static`,
26+
* or stay as they are. **They stay instance members**, because .NET's are, and making them
27+
* `static` would diverge from the very shape this stub exists to present. What #2276 *did*
28+
* change is that they now behave like .NET's:
29+
*
30+
* - `End()`, `Equals()` and `GetHashCode()` used to be `noexcept` and to return quietly
31+
* (a no-op, `false`, and `0`). .NET throws from all three, so **a caller who reached one
32+
* of them received a plausible answer where .NET reports an unsupported platform**;
33+
* - the exception is `PlatformNotSupportedException`, not `NotSupportedException`;
34+
* - `GetNextArgType()` returns `RuntimeTypeHandle`, as .NET's does, not `TypedReference`;
35+
* - the `GetNextArg(RuntimeTypeHandle)` overload was missing and is added.
36+
*
37+
* @note Both constructors are `[[noreturn]]`, and declaring them suppresses the implicit
38+
* default constructor, so **no public construction can succeed** — exactly as in .NET, where
39+
* both constructors throw. The instance members are therefore unreachable through ordinary
40+
* use in both. A fixture that needs an instance has no legitimate route to one, and reaching
41+
* for raw storage instead is undefined behaviour rather than a workaround (SR-AUD-112).
2942
*/
3043
struct ArgIterator {
3144
/**
@@ -35,8 +48,7 @@ namespace System {
3548
* @param arglist Handle to the variable argument list.
3649
*/
3750
[[noreturn]] explicit ArgIterator(RuntimeArgumentHandle /*arglist*/) {
38-
throw NotSupportedException(
39-
"ArgIterator requires CLR __arglist support and is not available in sharp-runtime.");
51+
throw PlatformNotSupportedException("ArgIterator is not supported on this platform.");
4052
}
4153

4254
/**
@@ -47,47 +59,66 @@ namespace System {
4759
* @param ptr Pointer to the first argument descriptor.
4860
*/
4961
[[noreturn]] ArgIterator(RuntimeArgumentHandle /*arglist*/, void* /*ptr*/) {
50-
throw NotSupportedException(
62+
throw PlatformNotSupportedException(
5163
"ArgIterator requires CLR __arglist support and is not available in sharp-runtime.");
5264
}
5365

5466
/**
5567
* @brief Concludes processing of the argument list.
5668
*
57-
* No-op in this implementation (there is no list to finalize).
69+
* Always throws, as .NET's does (`ArgIterator.cs:23-26`). It was a silent no-op until
70+
* ticket #2276.
5871
*/
59-
void End() noexcept {}
72+
[[noreturn]] void End() {
73+
throw PlatformNotSupportedException("ArgIterator is not supported on this platform.");
74+
}
6075

6176
/**
62-
* @brief Returns false — ArgIterator stubs are never equal.
63-
* @return false.
77+
* @brief Always throws, as .NET's override does (`ArgIterator.cs:28-31`).
78+
*
79+
* It returned `false` until ticket #2276 — a plausible answer where .NET reports an
80+
* unsupported platform.
6481
*/
65-
[[nodiscard]] bool Equals(const ArgIterator& /*other*/) const noexcept { return false; }
82+
[[noreturn]] bool Equals(const ArgIterator& /*other*/) const {
83+
throw PlatformNotSupportedException("ArgIterator is not supported on this platform.");
84+
}
6685

6786
/**
68-
* @brief Returns a hash code for this ArgIterator.
69-
* @return 0 (stub).
87+
* @brief Always throws, as .NET's override does (`ArgIterator.cs:33-36`).
88+
*
89+
* It returned `0` until ticket #2276.
7090
*/
71-
[[nodiscard]] int GetHashCode() const noexcept { return 0; }
91+
[[noreturn]] int GetHashCode() const {
92+
throw PlatformNotSupportedException("ArgIterator is not supported on this platform.");
93+
}
7294

7395
/**
7496
* @brief Returns the next argument in the variable-argument list.
7597
*
7698
* Always throws NotSupportedException.
7799
*/
78100
[[noreturn]] TypedReference GetNextArg() {
79-
throw NotSupportedException(
80-
"ArgIterator.GetNextArg requires CLR __arglist support and is not available in sharp-runtime.");
101+
throw PlatformNotSupportedException("ArgIterator is not supported on this platform.");
81102
}
82103

83104
/**
84105
* @brief Returns the next argument constrained to the specified runtime type.
85106
*
86-
* Always throws NotSupportedException.
107+
* Always throws. **Added by ticket #2276** — .NET has this overload
108+
* (`ArgIterator.cs:44-48`) and this port did not.
109+
*/
110+
[[noreturn]] TypedReference GetNextArg(RuntimeTypeHandle /*rth*/) {
111+
throw PlatformNotSupportedException("ArgIterator is not supported on this platform.");
112+
}
113+
114+
/**
115+
* @brief Returns the runtime type of the next argument.
116+
*
117+
* Always throws. **The return type changed from `TypedReference` to
118+
* `RuntimeTypeHandle` in ticket #2276**, matching `ArgIterator.cs:50-53`.
87119
*/
88-
[[noreturn]] TypedReference GetNextArgType() {
89-
throw NotSupportedException(
90-
"ArgIterator.GetNextArgType requires CLR __arglist support and is not available in sharp-runtime.");
120+
[[noreturn]] RuntimeTypeHandle GetNextArgType() {
121+
throw PlatformNotSupportedException("ArgIterator is not supported on this platform.");
91122
}
92123

93124
/**
@@ -97,8 +128,7 @@ namespace System {
97128
* @return Never returns.
98129
*/
99130
[[noreturn]] int GetRemainingCount() {
100-
throw NotSupportedException(
101-
"ArgIterator.GetRemainingCount requires CLR __arglist support and is not available in sharp-runtime.");
131+
throw PlatformNotSupportedException("ArgIterator is not supported on this platform.");
102132
}
103133
};
104134

modules/core/tests/System/Batch12ArgHandleTests.cpp

Lines changed: 67 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// Copyright (c) Robert Vokac and contributors
33
// Portions based on .NET runtime API (MIT License, Copyright .NET Foundation and Contributors)
44
#include <gtest/gtest.h>
5+
#include "System/PlatformNotSupportedException.hpp"
6+
#include "System/RuntimeTypeHandle.hpp"
57

68
#include <bit>
79
#include <string>
@@ -57,45 +59,69 @@ TEST(RuntimeArgumentHandleTests, IsCopyConstructible) {
5759
// ArgIterator
5860
// ===========================================================================
5961

60-
TEST(ArgIteratorTests, Constructor_Throws) {
62+
// #2276 SETTLED THE OPEN QUESTION, AND THE REFERENCE ANSWERED IT.
63+
//
64+
// The question was whether ArgIterator's unreachable instance members should become reachable,
65+
// become static, or stay as they are. They STAY INSTANCE MEMBERS, because .NET's are, and making
66+
// them static would diverge from the very shape this stub exists to present.
67+
//
68+
// What the reference settled is that EVERY member of .NET's portable ArgIterator throws
69+
// PlatformNotSupportedException (`ArgIterator.cs:10-58`) -- constructors, End, Equals,
70+
// GetHashCode and the rest alike. Three members here returned quietly instead: End() was a no-op,
71+
// Equals() returned false and GetHashCode() returned 0. A caller who reached one of those
72+
// received a PLAUSIBLE ANSWER where .NET reports an unsupported platform, which is the worse of
73+
// the two failures.
74+
75+
TEST(ArgIteratorTests, Fix2276_BothConstructorsThrowPlatformNotSupported) {
6176
System::RuntimeArgumentHandle h;
62-
EXPECT_THROW(System::ArgIterator it(h), System::NotSupportedException);
77+
EXPECT_THROW(System::ArgIterator it(h), System::PlatformNotSupportedException);
78+
EXPECT_THROW(System::ArgIterator it(h, nullptr), System::PlatformNotSupportedException);
6379
}
6480

65-
TEST(ArgIteratorTests, Constructor_WithPtr_Throws) {
66-
System::RuntimeArgumentHandle h;
67-
EXPECT_THROW(System::ArgIterator it(h, nullptr), System::NotSupportedException);
68-
}
69-
70-
TEST(ArgIteratorTests, End_DoesNotThrow) {
81+
TEST(ArgIteratorTests, Fix2276_EveryMemberThrowsIncludingTheThreeThatUsedToAnswer) {
7182
auto it = MakeArgIterator();
72-
EXPECT_NO_THROW(it.End());
73-
}
83+
auto other = MakeArgIterator();
7484

75-
TEST(ArgIteratorTests, GetHashCode_ReturnsZero) {
76-
auto it = MakeArgIterator();
77-
EXPECT_EQ(it.GetHashCode(), 0);
78-
}
85+
// The three that changed. Each used to return quietly.
86+
EXPECT_THROW(it.End(), System::PlatformNotSupportedException);
87+
EXPECT_THROW((void)it.GetHashCode(), System::PlatformNotSupportedException);
88+
EXPECT_THROW((void)it.Equals(other), System::PlatformNotSupportedException);
7989

80-
TEST(ArgIteratorTests, Equals_ReturnsFalse) {
81-
auto a = MakeArgIterator();
82-
auto b = MakeArgIterator();
83-
EXPECT_FALSE(a.Equals(b));
84-
}
90+
// The three that already threw, now with .NET's exception type rather than the base.
91+
EXPECT_THROW((void)it.GetNextArg(), System::PlatformNotSupportedException);
92+
EXPECT_THROW((void)it.GetNextArgType(), System::PlatformNotSupportedException);
93+
EXPECT_THROW((void)it.GetRemainingCount(), System::PlatformNotSupportedException);
8594

86-
TEST(ArgIteratorTests, GetNextArg_Throws) {
87-
auto it = MakeArgIterator();
88-
EXPECT_THROW(it.GetNextArg(), System::NotSupportedException);
95+
// The overload #2276 ADDED, which .NET has and this port did not.
96+
EXPECT_THROW((void)it.GetNextArg(System::RuntimeTypeHandle{}),
97+
System::PlatformNotSupportedException);
8998
}
9099

91-
TEST(ArgIteratorTests, GetNextArgType_Throws) {
100+
TEST(ArgIteratorTests, Fix2276_TheExceptionTypeIsPlatformNotSupportedNotItsBase) {
101+
// PlatformNotSupportedException derives from NotSupportedException, so a test that only
102+
// caught the base would pass either way -- which is exactly how the old type survived. This
103+
// row asserts the derived type is what arrives.
92104
auto it = MakeArgIterator();
93-
EXPECT_THROW(it.GetNextArgType(), System::NotSupportedException);
105+
try {
106+
(void)it.GetRemainingCount();
107+
ADD_FAILURE() << "expected PlatformNotSupportedException";
108+
} catch (const System::PlatformNotSupportedException& e) {
109+
EXPECT_STREQ(e.what(), "ArgIterator is not supported on this platform.");
110+
}
111+
static_assert(std::is_base_of_v<System::NotSupportedException,
112+
System::PlatformNotSupportedException>,
113+
"if this ever stops holding, the catch above is not the narrowing it claims");
94114
}
95115

96-
TEST(ArgIteratorTests, GetRemainingCount_Throws) {
97-
auto it = MakeArgIterator();
98-
EXPECT_THROW(it.GetRemainingCount(), System::NotSupportedException);
116+
TEST(ArgIteratorTests, Fix2276_GetNextArgTypeReturnsARuntimeTypeHandle) {
117+
// .NET's returns RuntimeTypeHandle (`ArgIterator.cs:50-53`); this port returned
118+
// TypedReference. The body throws either way, so ONLY a type assertion can catch this.
119+
static_assert(std::is_same_v<decltype(std::declval<System::ArgIterator&>().GetNextArgType()),
120+
System::RuntimeTypeHandle>,
121+
"#2276: GetNextArgType returns a RuntimeTypeHandle, as .NET's does");
122+
static_assert(std::is_same_v<decltype(std::declval<System::ArgIterator&>().GetNextArg()),
123+
System::TypedReference>,
124+
"...while GetNextArg still returns a TypedReference, also as .NET's does");
99125
}
100126

101127
// ===========================================================================
@@ -116,25 +142,27 @@ TEST(ArgIteratorContractTests, IsEmptyAndTriviallyCopyable) {
116142
EXPECT_EQ(sizeof(System::ArgIterator), 1u);
117143
}
118144

119-
TEST(ArgIteratorContractTests, HandleConstructor_MessageNamesTheUnsupportedFeature) {
145+
TEST(ArgIteratorContractTests, Fix2276_EveryDoorCarriesDotNetsOwnSentence) {
146+
// These two rows used to pin this port's INVENTED messages -- "ArgIterator requires CLR
147+
// __arglist support and is not available in sharp-runtime", and a per-member variant naming
148+
// GetRemainingCount. .NET uses ONE sentence for every door
149+
// (`Strings.resx:3305-3307`, SR.PlatformNotSupported_ArgIterator), so #2276 transcribes that
150+
// instead. A per-member message reads more helpfully and is not what the reference says.
120151
System::RuntimeArgumentHandle h;
152+
const std::string expected = "ArgIterator is not supported on this platform.";
121153
try {
122154
System::ArgIterator it(h);
123-
FAIL() << "expected NotSupportedException";
124-
} catch (const System::NotSupportedException& ex) {
125-
EXPECT_EQ(std::string(ex.getMessageProperty()),
126-
"ArgIterator requires CLR __arglist support and is not available in sharp-runtime.");
155+
FAIL() << "expected PlatformNotSupportedException";
156+
} catch (const System::PlatformNotSupportedException& ex) {
157+
EXPECT_EQ(std::string(ex.getMessageProperty()), expected);
127158
}
128-
}
129159

130-
TEST(ArgIteratorContractTests, GetRemainingCount_MessageNamesTheMember) {
131160
auto it = MakeArgIterator();
132161
try {
133162
(void)it.GetRemainingCount();
134-
FAIL() << "expected NotSupportedException";
135-
} catch (const System::NotSupportedException& ex) {
136-
EXPECT_EQ(std::string(ex.getMessageProperty()),
137-
"ArgIterator.GetRemainingCount requires CLR __arglist support and is not "
138-
"available in sharp-runtime.");
163+
FAIL() << "expected PlatformNotSupportedException";
164+
} catch (const System::PlatformNotSupportedException& ex) {
165+
EXPECT_EQ(std::string(ex.getMessageProperty()), expected)
166+
<< "one sentence for every door, as .NET has -- not a per-member variant";
139167
}
140168
}

plan.sqlite3

4 KB
Binary file not shown.

0 commit comments

Comments
 (0)