Skip to content

Commit 4d5e897

Browse files
robertvokacrobertvokac
committed
fix(runtime): MarshalAsAttribute gets .NET's field types and the COM enums (#1980, group G-5)
Rule-14 sweep. Field types are the contract here for the same reason G-2's values were: these types exist to preserve managed metadata, and a field typed as a loose integer where .NET types it as an enum lets any number be stored where only a marshalling kind is meaningful. Value becomes get-only (SA-8), ArraySubType becomes an UnmanagedType, SizeParamIndex becomes a short, SafeArraySubType and IidParameterIndex arrive, the class is sealed, and VarEnum / ComInterfaceType / ClassInterfaceType are added. Both new enum-typed fields default to 0, which is not a declared enumerator -- the same deliberate choice G-2 recorded for the CharSet defaults, because .NET's fields have no initializer either. One member stays absent deliberately: .NET types SafeArrayUserDefinedSubType as Type?, which is reflection, and inventing a second string would look like parity while storing something else. MarshalTypeRef survives as a name-carrying string only because it already did. Six mutations, all caught, four at compile time. M4 is the one worth keeping: .NET's VarEnum jumps from VT_DECIMAL = 14 to VT_I1 = 16, and asserting those neighbours catches a renumbering but NOT an insertion -- adding VT_UNUSED15 = 15 went uncaught, because C++ cannot enumerate an enum's members. The fix is an exhaustive switch with no default: the build runs -Wall -Wextra -Werror, so -Wswitch turns any unhandled enumerator into "error: enumeration value 'VT_UNUSED15' not handled in switch". That pins membership, which no value assertion can, and it fails the other way too. Fixture set 42/219 -> 43/223; site 2 is the assignment to Value that let a caller retarget an attribute after construction. First-party migration was one site, found by the compiler. Downstream measured: 0 sites in both. #1980 now has G-3 alone left. Gate: 17,515 run, 17,515 passed, 0 failed, 0 skipped across 38 executables (+9 on 17,506; SharpRuntimeTests_Runtime 189 -> 198; no other executable moved). Module graph unchanged at 41/93.
1 parent 0bd261c commit 4d5e897

6 files changed

Lines changed: 449 additions & 9 deletions

File tree

‎CLAUDE.md‎

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

‎docs/Migration-MarshalAsFieldTypes.md‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
<!-- SPDX-License-Identifier: MIT -->
2+
<!-- Copyright (c) Robert Vokac and contributors -->
3+
4+
# Migration — `MarshalAsAttribute` gets .NET's field types, and the two COM enums arrive (ticket #1980, group G-5)
5+
6+
*2026-08-19.* `MarshalAsAttribute::Value` becomes get-only, `ArraySubType` becomes an
7+
`UnmanagedType`, `SizeParamIndex` becomes a `short`, two absent fields arrive, and
8+
`VarEnum` / `ComInterfaceType` / `ClassInterfaceType` are added.
9+
10+
**This is a public source break**, landed under **SA-8** (the `Value` half) and **SA-5** (the
11+
types and the additions), with SA-2's five conditions discharged.
12+
13+
---
14+
15+
## 1. Why field *types* are the contract here
16+
17+
The same reason group G-2 gave for the values: P/Invoke is a declared permanent deviation, so
18+
these types exist to **preserve the managed metadata**. A field typed as a loose integer where
19+
.NET types it as an enum is therefore not a stylistic choice — it lets **any number** be stored
20+
where only a marshalling kind is meaningful, which is the whole reason the enum exists.
21+
22+
## 2. What changed
23+
24+
| Member | Was | Is | Reference |
25+
|---|---|---|---|
26+
| `Value` | public **mutable** field | **get-only**, via `getValueProperty()` | `MarshalAsAttribute.cs:18` (`{ get; }`) |
27+
| `ArraySubType` | `intcs` | **`UnmanagedType`** | `:29` |
28+
| `SizeParamIndex` | `intcs` | **`shortcs`** | `:30` |
29+
| `SafeArraySubType` | **absent** | `VarEnum` | `:21` |
30+
| `IidParameterIndex` | **absent** | `intcs` | `:25` |
31+
| the class | not sealed | **`final`** | `public sealed partial class` |
32+
| `VarEnum` | absent | added | `VarEnum.cs` |
33+
| `ComInterfaceType` | absent | added | `ComInterfaceType.cs` |
34+
| `ClassInterfaceType` | absent | added | `ClassInterfaceType.cs` |
35+
36+
`ArraySubType` and `SafeArraySubType` default to **0**, which is **not a declared enumerator** in
37+
either enum (`UnmanagedType` starts at `Bool = 2`). That is deliberate and is the same reasoning
38+
G-2 recorded for the two `CharSet` defaults: .NET's fields have no initializer either.
39+
40+
## 3. Two members stay out of scope, and one of them stays *absent*
41+
42+
.NET types **`MarshalTypeRef`** and **`SafeArrayUserDefinedSubType`** as `Type?`, and
43+
`System::Type` is reflection — a declared permanent deviation of this port.
44+
45+
* `MarshalTypeRef` survives as a `std::string` holding the type's **name**. It is the closest
46+
available shape and it already existed.
47+
* `SafeArrayUserDefinedSubType` is **absent**, not invented as a second string. A member that
48+
cannot carry what .NET's carries is worse than no member — it would look like parity while
49+
storing something else.
50+
51+
Both are stated in the header rather than left implicit.
52+
53+
## 4. To migrate
54+
55+
```cpp
56+
attr.Value; // was — read
57+
attr.getValueProperty(); // now
58+
59+
attr.Value = UnmanagedType::I4; // was — write
60+
attr = MarshalAsAttribute(UnmanagedType::I4); // now: the type is fixed at construction
61+
62+
attr.ArraySubType = 7; // was
63+
attr.ArraySubType = UnmanagedType::I4; // now
64+
```
65+
66+
**First-party migration was one site**, a test, and the compiler found it.
67+
68+
## 5. Evidence
69+
70+
Six mutations, **all caught**, four at compile time:
71+
72+
| Mutation | Caught by |
73+
|---|---|
74+
| M1 — `Value` public and mutable again | the migrated test (compile time) |
75+
| M2 — `ArraySubType` back to `intcs` | `Fix1980G5_ArraySubTypeIsAnUnmanagedTypeNotAnInt` (compile time) |
76+
| M3 — `SizeParamIndex` back to `intcs` | `Fix1980G5_SizeParamIndexIsAShortNotAnInt` (compile time) |
77+
| M4 — a `VT_UNUSED15 = 15` enumerator inserted | `VarEnumCensus` (compile time) — **only after that census was written** |
78+
| M5 — a `VarEnum` flag value wrong | `Fix1980G5_VarEnumCarriesItsHoleAndItsFlags` |
79+
| M6 — a `ComInterfaceType` value wrong | `Fix1980G5_TheTwoComEnumsExist` |
80+
81+
**M4 is the one worth recording.** .NET's `VarEnum` jumps from `VT_DECIMAL = 14` straight to
82+
`VT_I1 = 16` — there is no member with value 15. Asserting that 14 and 16 are the neighbours
83+
catches a **renumbering** but *not* an **insertion**, and M4 went uncaught at first for exactly
84+
that reason: C++ offers no way to enumerate an enum's members.
85+
86+
The fix is an **exhaustive `switch` with no `default:` label**. The build runs with
87+
`-Wall -Wextra -Werror`, so gcc's `-Wswitch` turns any unhandled enumerator into a compile error:
88+
89+
```
90+
error: enumeration value 'VT_UNUSED15' not handled in switch [-Werror=switch]
91+
```
92+
93+
That pins the enum's **membership**, which no value assertion can. It also fails in the other
94+
direction — removing an enumerator breaks the census's own reference to it.
95+
96+
**`VarEnum`'s flag values** are the other row a transcription gets wrong: `VT_VECTOR`, `VT_ARRAY`
97+
and `VT_BYREF` sit at `0x1000`, `0x2000` and `0x4000`, far above the rest.
98+
99+
Negative consumer fixture: `test/consumer/runtime_marshalas_shape_negative.cpp`, four sites, all
100+
rejected. Fixture set grows to **43 fixtures / 223 sites**. Site 2 is the spelling this ticket
101+
exists for — assigning to `Value`, which let a caller retarget an attribute after construction.
102+
103+
Gate: **17,515 run, 17,515 passed, 0 failed, 0 skipped** across 38 executables — `+9` on 17,506
104+
(`SharpRuntimeTests_Runtime` 189 → 198; one pin migrated in place, nine cases added). No other
105+
executable moved. Module graph unchanged at 41/93.
106+
107+
## 6. Downstream, measured
108+
109+
`MarshalAsAttribute` appears in **zero** places in `cna` and **zero** in `mobile-eggbert`. Neither
110+
repository was modified.
111+
112+
## 7. Scope
113+
114+
This closes **G-5** of #1980. G-1, G-2 and G-4 landed earlier the same day. **Only G-3 remains** —
115+
reparenting `AmbiguousImplementationException` and five attributes and sealing them, a vtable
116+
*and* layout change that SA-3 explicitly excludes.

‎modules/runtime/include/System/Runtime/InteropServices/InteropAttributes.hpp‎

Lines changed: 106 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,52 @@ namespace System::Runtime::InteropServices {
109109
LPUTF8Str = 48
110110
};
111111

112+
/**
113+
* @brief COM automation variant types.
114+
*
115+
* #1980 group G-5 / SR-AUD-167: added because `MarshalAsAttribute::SafeArraySubType` is a
116+
* `VarEnum` in .NET (`MarshalAsAttribute.cs:21`) and neither the field nor its type existed
117+
* here. Transcribed from `VarEnum.cs`, including the three flag values above 0x1000.
118+
*/
119+
enum class VarEnum : SharpRuntime::intcs {
120+
VT_EMPTY = 0, VT_NULL = 1, VT_I2 = 2, VT_I4 = 3,
121+
VT_R4 = 4, VT_R8 = 5, VT_CY = 6, VT_DATE = 7,
122+
VT_BSTR = 8, VT_DISPATCH = 9, VT_ERROR = 10, VT_BOOL = 11,
123+
VT_VARIANT = 12, VT_UNKNOWN = 13, VT_DECIMAL = 14,
124+
// 15 is deliberately absent: .NET's enum has no member with that value.
125+
VT_I1 = 16, VT_UI1 = 17, VT_UI2 = 18, VT_UI4 = 19,
126+
VT_I8 = 20, VT_UI8 = 21, VT_INT = 22, VT_UINT = 23,
127+
VT_VOID = 24, VT_HRESULT = 25, VT_PTR = 26, VT_SAFEARRAY = 27,
128+
VT_CARRAY = 28, VT_USERDEFINED = 29, VT_LPSTR = 30, VT_LPWSTR = 31,
129+
VT_RECORD = 36, VT_FILETIME = 64, VT_BLOB = 65, VT_STREAM = 66,
130+
VT_STORAGE = 67, VT_STREAMED_OBJECT = 68, VT_STORED_OBJECT = 69,
131+
VT_BLOB_OBJECT = 70, VT_CF = 71, VT_CLSID = 72,
132+
VT_VECTOR = 0x1000, VT_ARRAY = 0x2000, VT_BYREF = 0x4000
133+
};
134+
135+
/**
136+
* @brief How a COM interface is exposed to COM clients.
137+
*
138+
* #1980 group G-5 / SR-AUD-167. Transcribed from `ComInterfaceType.cs`.
139+
*/
140+
enum class ComInterfaceType : SharpRuntime::intcs {
141+
InterfaceIsDual = 0,
142+
InterfaceIsIUnknown = 1,
143+
InterfaceIsIDispatch = 2,
144+
InterfaceIsIInspectable = 3
145+
};
146+
147+
/**
148+
* @brief The kind of class interface generated for a class.
149+
*
150+
* #1980 group G-5 / SR-AUD-167. Transcribed from `ClassInterfaceType.cs`.
151+
*/
152+
enum class ClassInterfaceType : SharpRuntime::intcs {
153+
None = 0,
154+
AutoDispatch = 1,
155+
AutoDual = 2
156+
};
157+
112158
/** Specifies the calling convention of an unmanaged entry point. */
113159
enum class CallingConvention : SharpRuntime::intcs {
114160
Winapi = 1, ///< Platform default (stdcall on Windows).
@@ -162,21 +208,74 @@ namespace System::Runtime::InteropServices {
162208
};
163209

164210
/** Indicates how a managed member should be marshalled to/from unmanaged code. */
165-
class MarshalAsAttribute : public System::Attribute {
211+
class MarshalAsAttribute final : public System::Attribute {
212+
UnmanagedType value_;
166213
public:
167-
UnmanagedType Value; ///< The unmanaged type to marshal as.
168-
SharpRuntime::intcs ArraySubType = 0; ///< Element type for array marshalling.
214+
/**
215+
* Element type for array marshalling.
216+
*
217+
* #1980 group G-5 / SR-AUD-167: this was `intcs`. .NET declares
218+
* `public UnmanagedType ArraySubType;` (`MarshalAsAttribute.cs:29`) -- an
219+
* `UnmanagedType`, not a loose integer. The weaker type let any number be stored where
220+
* only a marshalling kind is meaningful, which is the whole reason the enum exists.
221+
*
222+
* The default is `0`, which is **not a declared enumerator** (`UnmanagedType` starts at
223+
* `Bool = 2`) -- deliberately, because .NET's field has no initializer either, the same
224+
* reasoning group G-2 recorded for the two `CharSet` defaults.
225+
*/
226+
UnmanagedType ArraySubType = static_cast<UnmanagedType>(0);
227+
/**
228+
* The COM variant type of a SafeArray's elements.
229+
*
230+
* #1980 G-5 / SR-AUD-167: absent before. .NET: `public VarEnum SafeArraySubType;`
231+
* (`MarshalAsAttribute.cs:21`).
232+
*/
233+
VarEnum SafeArraySubType = static_cast<VarEnum>(0);
234+
/**
235+
* Parameter index of the IID for an `IUnknown`/`IDispatch` marshal.
236+
*
237+
* #1980 G-5 / SR-AUD-167: absent before. .NET: `public int IidParameterIndex;`
238+
* (`MarshalAsAttribute.cs:25`).
239+
*/
240+
SharpRuntime::intcs IidParameterIndex = 0;
169241
std::string MarshalType; ///< Fully qualified name of a custom marshaller.
170-
std::string MarshalTypeRef; ///< Type reference for a custom marshaller.
242+
/**
243+
* Type reference for a custom marshaller.
244+
*
245+
* @note **A permanent deviation, stated rather than left implicit.** .NET declares this
246+
* as `public Type? MarshalTypeRef;` (`MarshalAsAttribute.cs:35`), and `System::Type` is
247+
* reflection -- out of scope for this port by explicit decision. A `std::string` holding
248+
* the type's name is the closest available shape. The same applies to
249+
* `SafeArrayUserDefinedSubType`, which .NET also types as `Type?` and which is therefore
250+
* **absent here** rather than invented as another string: adding a member that cannot
251+
* carry what .NET's carries would be worse than not having it.
252+
*/
253+
std::string MarshalTypeRef;
171254
std::string MarshalCookie; ///< Extra string passed to the custom marshaller.
172255
SharpRuntime::intcs SizeConst = 0; ///< Fixed array/string size for ByValArray/ByValTStr.
173-
SharpRuntime::intcs SizeParamIndex = 0; ///< Parameter index supplying the array size.
256+
/**
257+
* Parameter index supplying the array size.
258+
*
259+
* #1980 G-5 / SR-AUD-167: this was `intcs`. .NET declares
260+
* `public short SizeParamIndex;` (`MarshalAsAttribute.cs:30`) -- a **short**, because the
261+
* value is a parameter position and the metadata encoding is 16-bit.
262+
*/
263+
SharpRuntime::shortcs SizeParamIndex = 0;
174264

175265
/** @param t The unmanaged marshalling type. */
176-
explicit MarshalAsAttribute(UnmanagedType t) : Value(t) {}
266+
explicit MarshalAsAttribute(UnmanagedType t) : value_(t) {}
177267

178268
/** Integer overload — @param t is cast to UnmanagedType. */
179-
explicit MarshalAsAttribute(SharpRuntime::shortcs t) : Value(static_cast<UnmanagedType>(t)) {}
269+
explicit MarshalAsAttribute(SharpRuntime::shortcs t) : value_(static_cast<UnmanagedType>(t)) {}
270+
271+
/**
272+
* @return The unmanaged type to marshal as.
273+
*
274+
* #1980 G-5 / SR-AUD-167: `Value` was a public **mutable** data member. .NET's is
275+
* `public UnmanagedType Value { get; }` (`MarshalAsAttribute.cs:18`) -- get-only, set
276+
* once by the constructor. Landed under SA-8, whose first bullet is exactly this shape.
277+
*/
278+
[[nodiscard]] UnmanagedType getValueProperty() const noexcept { return value_; }
180279
};
181280

182281
/** Specifies the DLL entry point and calling options for a P/Invoke method. */

0 commit comments

Comments
 (0)