fix(runtime): the interop metadata values match .NET (#1980, group G-2)

Rule-14 sweep. P/Invoke is a declared permanent deviation, so these types
exist to preserve the managed metadata values rather than to produce a native
effect -- getting the numbers right is the whole of their contract.

The LPStruct finding was understated. The plan recorded "48 -> 43", a wrong
number; 48 is LPUTF8Str's value, so the two enumerators were
INDISTINGUISHABLE -- LPStruct == LPUTF8Str was true, and a switch over
UnmanagedType could not carry both arms. A test asserts the inequality, not
just the two numbers.

Two more divergences the plan never named were found by measuring the
reference alongside the four it did: BOTH CharSet fields. .NET declares them
as plain fields with no initializer, so the default is 0 -- which is not a
declared enumerator (None is 1). Reproducing an unnamed default is deliberate
and follows from the types' purpose; a test states the reasoning so it is not
later "corrected" back.

Also repaired: Currency and IDispatch were absent; Pack was 8 where .NET's
plain `public int Pack;` gives 0; PreserveSig and BestFitMapping were true
where .NET's plain `public bool` gives false -- and the port had already got
the other three booleans right, which is what makes those two a divergence
rather than a policy.

Every other UnmanagedType value already matched, asserted across a spread so
the three repairs cannot be mistaken for a renumbering.

Five mutations, all caught; M2 at compile time, since removing an enumerator
breaks the test naming it and that is the only way C++ reports it. The plan
predicted the one pin that had to be inverted (DefaultPack_IsEight), and it
was the only pre-existing test this group touched.

Downstream measured: 0 sites in cna, 0 in mobile-eggbert.

#1980 stays open for G-3 (vtable), G-4 (mandatory migration) and G-5.

Gate: 17,503 run, 17,503 passed, 0 failed, 0 skipped across 38 executables
(+5 on 17,498; SharpRuntimeTests_Runtime 181 -> 186; no other executable
moved). Module graph unchanged at 41/93.

Author: robertvokac

CLAUDE.md

@@ -0,0 +1,97 @@
+<!-- SPDX-License-Identifier: MIT -->
+<!-- Copyright (c) Robert Vokac and contributors -->
+
+# Migration — the interop metadata values match .NET (ticket #1980, group G-2)
+
+*2026-08-19.* `System::Runtime::InteropServices`' `UnmanagedType`, `StructLayoutAttribute` and
+`DllImportAttribute` carried wrong constants and wrong defaults. All of them now match the
+reference.
+
+Landed under **SA-5**. No layout, vtable, signature or `noexcept` change — only *values*.
+
+---
+
+## 1. Why the numbers are the whole contract
+
+P/Invoke and interop are a **declared permanent deviation**: these types exist so ported
+declarations keep compiling and so the **managed metadata values are preserved**, not to produce
+a native effect. The header says so itself. Getting the numbers right is therefore not cosmetic —
+it is the entire purpose of the types.
+
+## 2. What changed
+
+| Declaration | Was | Is | Reference |
+|---|---|---|---|
+| `UnmanagedType::LPStruct` | **48** | **43** | `UnmanagedType.cs:55` (`0x2b`) |
+| `UnmanagedType::Currency` | **absent** | **15** | `UnmanagedType.cs:22` (`0xf`) |
+| `UnmanagedType::IDispatch` | **absent** | **26** | `UnmanagedType.cs:30` (`0x1a`) |
+| `StructLayoutAttribute::Pack` | **8** | **0** | `StructLayoutAttribute.cs:21` |
+| `StructLayoutAttribute::CharSet` | `Ansi` (2) | **0** | `StructLayoutAttribute.cs:23` |
+| `DllImportAttribute::CharSet` | `None` (1) | **0** | plain field, no initializer |
+| `DllImportAttribute::PreserveSig` | **true** | **false** | `DllImportAttribute.cs:22` |
+| `DllImportAttribute::BestFitMapping` | **true** | **false** | `DllImportAttribute.cs:21` |
+
+## 3. The `LPStruct` finding was understated
+
+The plan recorded *"`LPStruct` 48 → 43"* — a wrong number. Measured, it is worse than that: **48
+is `LPUTF8Str`'s value**, so the two enumerators were **indistinguishable**.
+`UnmanagedType::LPStruct == UnmanagedType::LPUTF8Str` was **true**, and a `switch` over
+`UnmanagedType` could not carry both arms — it would not compile.
+
+A test now asserts the inequality, not just the two numbers.
+
+## 4. Two divergences the plan did not name
+
+G-2's list named four items. Measuring the reference alongside them found a fifth and sixth: both
+`CharSet` fields.
+
+.NET declares `public CharSet CharSet;` on both attributes **with no initializer**, so the default
+is `0` — and `CharSet` has **no enumerator with that value** (`None` is 1). This port defaulted
+them to named values instead.
+
+**Reproducing an unnamed default is deliberate**, and it follows directly from §1: the header
+exists to preserve the managed metadata, and .NET's metadata really does carry an unset `CharSet`
+here. A test states the reasoning so it is not later "corrected" back to a named value.
+
+## 5. What did *not* change, and why that matters
+
+Every other `UnmanagedType` value already matched .NET exactly — `Bool`, the whole `I1`…`U8`
+block, `BStr`, `IUnknown`, `Struct`, `LPArray`, `CustomMarshaler`, `HString` and the rest. A test
+asserts a representative spread of them, so the three repairs above cannot be mistaken for a
+wholesale renumbering.
+
+The same applies to `DllImportAttribute`'s booleans: `SetLastError`, `ExactSpelling` and
+`ThrowOnUnmappableChar` were **already** `false`. That the port got three right and two wrong is
+what makes `PreserveSig`/`BestFitMapping` a divergence rather than a deliberate policy.
+
+## 6. Evidence
+
+Five mutations, **all caught** — one per repaired value, plus the `CharSet` default:
+
+| Mutation | Caught by |
+|---|---|
+| M1 — `LPStruct` back to 48 | `Fix1980G2_LPStructNoLongerCollidesWithLPUTF8Str` |
+| M2 — `Currency` removed | **at compile time** — `error: 'Currency' is not a member of 'UnmanagedType'`, which is the only way C++ can report a missing enumerator |
+| M3 — `Pack` back to 8 | `Fix1980G2_DefaultPackIsZeroNotEight` |
+| M4 — `PreserveSig` back to true | `Fix1980G2_DllImportBoolDefaultsAreAllFalse` |
+| M5 — `CharSet` named again | `Decl1980G2_BothCharSetDefaultsAreUnsetNotNamed` |
+
+**One pre-existing test was inverted, and the plan predicted exactly that**:
+`StructLayoutAttributeTests.DefaultPack_IsEight` pinned the wrong value, and the plan's own G-2
+row said it *"would have to be rewritten"*. It is the only pre-existing test this group touches.
+
+Gate: **17,503 run, 17,503 passed, 0 failed, 0 skipped** across 38 executables — `+5` on 17,498
+(`SharpRuntimeTests_Runtime` 181 → 186; one pin inverted in place, five cases added). No other
+executable moved. Module graph unchanged at 41/93.
+
+## 7. Downstream, measured
+
+`UnmanagedType`, `StructLayoutAttribute`, `DllImportAttribute` and `MarshalAsAttribute` appear in
+**zero** places in `cna` and **zero** in `mobile-eggbert`. Neither repository was modified.
+
+## 8. Scope
+
+This closes **G-2** of #1980. Remaining: **G-3** (reparenting and sealing — a vtable *and* layout
+change SA-3 excludes), **G-4** (removing `setIsOptionalProperty`, moving `Url` to a settable
+property — mandatory migration), and **G-5** / SR-AUD-167 (retyping `MarshalAs` fields, adding
+`ComInterfaceType`/`ClassInterfaceType`). G-1 landed earlier the same day.

docs/Migration-InteropMetadataValues.md

@@ -34,11 +34,15 @@ namespace System::Runtime::InteropServices {
      * Stated explicitly because the audit found this header to be the one interop-adjacent file
      * that described effects it cannot produce **without saying so**, unlike the compiler-service
      * marker headers alongside it (ticket #1978, the disclosure half of SR-AUD-168). The
-     * remaining divergences in the *values* this header declares — `UnmanagedType::LPStruct`,
-     * `StructLayoutAttribute::Pack`, `DllImportAttribute::PreserveSig`/`BestFitMapping`, the
-     * omitted `MarshalAsAttribute` fields and the absent `ComInterfaceType`/`ClassInterfaceType`
-     * enums — are SR-AUD-165/166/167 and are tracked separately by the approval-gated ticket
-     * #1980, because changing them changes declarations a consumer can already name.
+     * The *value* divergences this header used to declare — `UnmanagedType::LPStruct`, the
+     * missing `Currency`/`IDispatch`, `StructLayoutAttribute::Pack`, both `CharSet` defaults and
+     * `DllImportAttribute::PreserveSig`/`BestFitMapping` — were **#1980 group G-2** and are now
+     * fixed; see docs/Migration-InteropMetadataValues.md. What remains of #1980 here is
+     * **SR-AUD-167** (group G-5): the omitted `MarshalAsAttribute` fields and the absent
+     * `ComInterfaceType`/`ClassInterfaceType` enums.
+     *
+     * Because these types exist to preserve managed metadata rather than to produce an effect,
+     * getting the *numbers* right is the whole of their contract.
      */
 
     /** Specifies the memory layout of a managed class or struct. */
@@ -69,6 +73,10 @@ namespace System::Runtime::InteropServices {
         U8         = 10,
         R4         = 11,
         R8         = 12,
+        // #1980 G-2 / SR-AUD-165: Currency and IDispatch were absent. .NET declares
+        // `Currency = 0xf` and `IDispatch = 0x1a` (UnmanagedType.cs:22,30).
+        Currency   = 15,
+        IDispatch  = 26,
         LPStr      = 20,
         LPWStr     = 21,
         LPTStr     = 22,
@@ -88,7 +96,12 @@ namespace System::Runtime::InteropServices {
         FunctionPtr= 38,
         AsAny      = 40,
         LPArray    = 42,
-        LPStruct   = 48,
+        // #1980 G-2 / SR-AUD-165. This was 48, which is not merely the wrong number: 48 is
+        // LPUTF8Str's value, so the two enumerators COLLIDED -- `LPStruct == LPUTF8Str` was true,
+        // and a switch over UnmanagedType could not carry both arms. .NET declares
+        // `LPStruct = 0x2b` (UnmanagedType.cs:55). The plan recorded this as a wrong value; the
+        // collision is what measurement added.
+        LPStruct   = 43,
         CustomMarshaler = 44,
         Error      = 45,
         IInspectable = 46,
@@ -109,9 +122,28 @@ namespace System::Runtime::InteropServices {
     class StructLayoutAttribute : public System::Attribute {
     public:
         LayoutKind Value;  ///< The layout kind.
-        SharpRuntime::intcs Pack = 8; ///< Packing alignment in bytes.
+        /**
+         * Packing alignment in bytes.
+         *
+         * #1980 G-2 / SR-AUD-166: this defaulted to **8**. .NET declares `public int Pack;` with
+         * no initializer (`StructLayoutAttribute.cs:21`), so a default-constructed attribute
+         * reports **0** -- which in the metadata means "use the runtime's default packing", a
+         * different statement from "pack to 8".
+         */
+        SharpRuntime::intcs Pack = 0;
         SharpRuntime::intcs Size = 0; ///< Minimum size in bytes (0 = no minimum).
-        ::System::Runtime::InteropServices::CharSet CharSet = ::System::Runtime::InteropServices::CharSet::Ansi; ///< Character set used for embedded strings.
+        /**
+         * Character set used for embedded strings.
+         *
+         * #1980 G-2: this defaulted to `CharSet::Ansi`. .NET declares `public CharSet CharSet;`
+         * with no initializer (`StructLayoutAttribute.cs:23`), so the default is **0** -- and
+         * `CharSet` has no enumerator with that value (`None` is 1). **That is deliberate, not a
+         * slip**: this header exists to preserve the managed metadata values exactly, and .NET's
+         * metadata really does carry an unset CharSet here. The plan's G-2 list did not name this
+         * field; it was found by measuring the reference alongside the four it did name.
+         */
+        ::System::Runtime::InteropServices::CharSet CharSet =
+            static_cast<::System::Runtime::InteropServices::CharSet>(0);
 
         /** @param layout The desired memory layout kind. */
         explicit StructLayoutAttribute(LayoutKind layout) : Value(layout) {}
@@ -152,12 +184,34 @@ namespace System::Runtime::InteropServices {
     public:
         std::string Value;         ///< The name of the DLL to import from.
         std::string EntryPoint;    ///< Name of the exported function; empty means use the method name.
-        ::System::Runtime::InteropServices::CharSet            CharSet           = ::System::Runtime::InteropServices::CharSet::None;   ///< String marshalling character set.
+        /**
+         * String marshalling character set.
+         *
+         * #1980 G-2: this defaulted to `CharSet::None` (1). .NET declares
+         * `public CharSet CharSet;` with no initializer, so the default is **0**, which is not a
+         * declared enumerator. Same reasoning as `StructLayoutAttribute::CharSet` above.
+         */
+        ::System::Runtime::InteropServices::CharSet            CharSet           =
+            static_cast<::System::Runtime::InteropServices::CharSet>(0);
         ::System::Runtime::InteropServices::CallingConvention  CallingConvention = ::System::Runtime::InteropServices::CallingConvention::Winapi; ///< Calling convention.
         bool               SetLastError      = false; ///< Capture GetLastError after the call.
         bool               ExactSpelling     = false; ///< Disable automatic A/W suffix probing.
-        bool               PreserveSig       = true;  ///< Preserve the signature (no HRESULT transformation).
-        bool               BestFitMapping    = true;  ///< Enable best-fit character mapping.
+        /**
+         * Preserve the signature (no HRESULT transformation).
+         *
+         * #1980 G-2 / SR-AUD-166: this defaulted to **true**. .NET declares `public bool
+         * PreserveSig;` with no initializer (`DllImportAttribute.cs:22`), so the field reads
+         * **false** on a default-constructed attribute -- the same as every other bool on the
+         * type, none of which this port had got wrong.
+         */
+        bool               PreserveSig       = false;
+        /**
+         * Enable best-fit character mapping.
+         *
+         * #1980 G-2 / SR-AUD-166: this defaulted to **true**; .NET's is a plain
+         * `public bool BestFitMapping;` (`DllImportAttribute.cs:21`), i.e. **false**.
+         */
+        bool               BestFitMapping    = false;
         bool               ThrowOnUnmappableChar = false; ///< Throw on unmappable Unicode characters.
 
         /** @param dllName The name of the native DLL. */

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

@@ -496,9 +496,13 @@ TEST(StructLayoutAttributeTests, Constructor_StoresLayout) {
     EXPECT_EQ(attr.Value, LayoutKind::Sequential);
 }
 
-TEST(StructLayoutAttributeTests, DefaultPack_IsEight) {
+TEST(StructLayoutAttributeTests, Fix1980G2_DefaultPackIsZeroNotEight) {
+    // INVERTED by #1980 group G-2. The plan predicted this exact pin would have to be rewritten,
+    // and it is the only pre-existing test the group touches. .NET declares `public int Pack;`
+    // with no initializer (StructLayoutAttribute.cs:21), so the default is 0 -- which in the
+    // metadata means "use the runtime's default packing", a different statement from "pack to 8".
     StructLayoutAttribute attr(LayoutKind::Explicit);
-    EXPECT_EQ(attr.Pack, 8);
+    EXPECT_EQ(attr.Pack, 0);
 }
 
 TEST(StructLayoutAttributeTests, DefaultSize_IsZero) {
@@ -1036,3 +1040,63 @@ TEST(ConditionalWeakTableEnumeratorTests, Decl1981_TheTablesOwnLayoutIsUnchanged
     static_assert(alignof(WeakTable) == 8);
     EXPECT_EQ(sizeof(WeakTable), 72u);
 }
+
+// =============================================================================================
+// Ticket #1980 group G-2 (SR-AUD-165/166) — the interop metadata VALUES.
+//
+// These types exist to preserve managed metadata, not to produce an effect (P/Invoke and interop
+// are a declared permanent deviation), so getting the numbers right is the whole of their
+// contract. Landed under SA-5: every value below is transcribed from the reference.
+// =============================================================================================
+
+TEST(InteropMetadataValueTests, Fix1980G2_LPStructNoLongerCollidesWithLPUTF8Str) {
+    // THE FINDING WAS UNDERSTATED. LPStruct was 48 -- which is LPUTF8Str's value, so the two
+    // enumerators were INDISTINGUISHABLE: the comparison below was true, and a switch over
+    // UnmanagedType could not carry both arms. .NET: LPStruct = 0x2b (UnmanagedType.cs:55).
+    EXPECT_EQ(static_cast<int>(UnmanagedType::LPStruct), 43);
+    EXPECT_EQ(static_cast<int>(UnmanagedType::LPUTF8Str), 48);
+    EXPECT_NE(UnmanagedType::LPStruct, UnmanagedType::LPUTF8Str);
+}
+
+TEST(InteropMetadataValueTests, Fix1980G2_CurrencyAndIDispatchExist) {
+    // Both were absent. .NET: Currency = 0xf, IDispatch = 0x1a (UnmanagedType.cs:22,30).
+    EXPECT_EQ(static_cast<int>(UnmanagedType::Currency), 15);
+    EXPECT_EQ(static_cast<int>(UnmanagedType::IDispatch), 26);
+}
+
+TEST(InteropMetadataValueTests, Decl1980G2_EveryOtherUnmanagedTypeValueWasAlreadyRight) {
+    // Asserted so the two repairs above are not mistaken for a wholesale renumbering: the rest of
+    // the enum already matched .NET exactly, which is why only these three rows moved.
+    EXPECT_EQ(static_cast<int>(UnmanagedType::Bool), 2);
+    EXPECT_EQ(static_cast<int>(UnmanagedType::U8), 10);
+    EXPECT_EQ(static_cast<int>(UnmanagedType::BStr), 19);
+    EXPECT_EQ(static_cast<int>(UnmanagedType::IUnknown), 25);
+    EXPECT_EQ(static_cast<int>(UnmanagedType::Struct), 27);
+    EXPECT_EQ(static_cast<int>(UnmanagedType::LPArray), 42);
+    EXPECT_EQ(static_cast<int>(UnmanagedType::CustomMarshaler), 44);
+    EXPECT_EQ(static_cast<int>(UnmanagedType::HString), 47);
+}
+
+TEST(InteropMetadataValueTests, Fix1980G2_DllImportBoolDefaultsAreAllFalse) {
+    // PreserveSig and BestFitMapping defaulted to TRUE. .NET declares every one of these as a
+    // plain `public bool` with no initializer (DllImportAttribute.cs:18-23), so all five read
+    // false -- and the port had already got the other three right, which is what made the two
+    // outliers a divergence rather than a policy.
+    DllImportAttribute attr("lib");
+    EXPECT_FALSE(attr.PreserveSig);
+    EXPECT_FALSE(attr.BestFitMapping);
+    EXPECT_FALSE(attr.SetLastError);
+    EXPECT_FALSE(attr.ExactSpelling);
+    EXPECT_FALSE(attr.ThrowOnUnmappableChar);
+}
+
+TEST(InteropMetadataValueTests, Decl1980G2_BothCharSetDefaultsAreUnsetNotNamed) {
+    // A divergence the plan's G-2 list did NOT name, found by measuring the reference alongside
+    // the four it did. .NET declares `public CharSet CharSet;` on both attributes with no
+    // initializer, so the default is 0 -- and CharSet has NO enumerator with that value (None is
+    // 1). Reproducing it is deliberate: the header's stated purpose is to preserve the managed
+    // metadata values, and .NET's metadata really does carry an unset CharSet here.
+    EXPECT_EQ(static_cast<int>(CharSet::None), 1) << "None is 1, so 0 is genuinely unnamed";
+    EXPECT_EQ(static_cast<int>(StructLayoutAttribute(LayoutKind::Sequential).CharSet), 0);
+    EXPECT_EQ(static_cast<int>(DllImportAttribute("lib").CharSet), 0);
+}