fix(runtime): three attribute shapes match .NET (#1980, group G-4)

Rule-14 sweep. The point of this group is that the port was wrong in BOTH
directions, which is why one rule could not fix it:

  * CompilerFeatureRequiredAttribute published a full IsOptional setter where
    .NET's is { get; init; } -- too permissive;
  * ObsoletedOSPlatformAttribute and RequiresPreviewFeaturesAttribute took a
    `url` constructor parameter .NET does not declare and fed it into a
    read-only accessor where .NET's is { get; set; } -- too inventive on the
    constructor and too restrictive on the property.

`init` is two facts, not one: the value CAN be supplied at construction and
CANNOT be assigned afterwards. C++ has no init, and the analogue of that pair
is a constructor parameter with no setter -- so removing the setter alone
would have been a narrowing rather than a translation. The two-argument
constructor is what makes the removal faithful, and one test asserts both
halves together.

Landed under SA-8 (mutability) and SA-5 (Url), with SA-2's five conditions.

Five mutations, all caught, two at compile time. M4 and M5 are run once per
type deliberately -- two separate declarations, and fixing one while leaving
the other is the easy half-repair. M4 was invalid as first written and was
reformulated rather than counted. The absence pin uses a dependent parameter
(the #2299 gcc trap).

Fixture set 41/215 -> 42/219; site 4 is the trait query, which breaks a
consumer silently. First-party migration was two sites, both tests, both
found by the compiler. Downstream measured: 0 sites in cna, 0 in
mobile-eggbert.

#1980 stays open for G-3 (vtable) and G-5.

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

Author: robertvokac

CLAUDE.md

@@ -0,0 +1,115 @@
+<!-- SPDX-License-Identifier: MIT -->
+<!-- Copyright (c) Robert Vokac and contributors -->
+
+# Migration — three attribute shapes match .NET (ticket #1980, group G-4)
+
+*2026-08-19.* `CompilerFeatureRequiredAttribute` loses its `IsOptional` setter and gains a
+two-argument constructor; `ObsoletedOSPlatformAttribute` and `RequiresPreviewFeaturesAttribute`
+lose their `url` constructor parameter and gain a `Url` setter.
+
+**This is a public source break**, landed under **SA-8** (the mutability half) and **SA-5** (the
+`Url` half), with SA-2's five conditions discharged. No layout or vtable change.
+
+---
+
+## 1. The port was wrong in **both** directions
+
+That is the point of this group, and it is why the two halves could not be fixed by one rule.
+
+| | Port had | .NET has | Port was |
+|---|---|---|---|
+| `CompilerFeatureRequiredAttribute::IsOptional` | full setter | `{ get; init; }` | **too permissive** |
+| `ObsoletedOSPlatformAttribute::Url` | `url` ctor param, read-only accessor | no such param; `{ get; set; }` | **too restrictive** on the property, **inventive** on the constructor |
+| `RequiresPreviewFeaturesAttribute::Url` | same | same | same |
+
+## 2. `init` is two facts, not one
+
+.NET's `public bool IsOptional { get; init; }` means the value **can** be supplied at
+construction —
+
+```csharp
+new CompilerFeatureRequiredAttribute("RefStructs") { IsOptional = true }   // legal
+```
+
+— and **cannot** be assigned afterwards. C++ has no `init`, and the exact analogue of that pair is
+a **constructor parameter with no setter**.
+
+So removing `setIsOptionalProperty` *alone* would have been a narrowing, not a translation: the
+value would have become unsettable. The two-argument constructor is what makes the removal
+faithful. A test asserts both halves together, and the absence pin uses a **dependent** parameter,
+because gcc evaluates a non-dependent `requires` eagerly and hard-errors instead of yielding
+`false` — the #2299 trap.
+
+## 3. `Url` was inverted
+
+.NET declares exactly:
+
+```csharp
+public ObsoletedOSPlatformAttribute(string platformName)                    // PlatformAttributes.cs
+public ObsoletedOSPlatformAttribute(string platformName, string? message)
+public string? Message { get; }
+public string? Url     { get; set; }
+
+public RequiresPreviewFeaturesAttribute()                                   // …Attribute.cs:28
+public RequiresPreviewFeaturesAttribute(string? message)                    // …Attribute.cs:34
+public string? Url { get; set; }                                            // …Attribute.cs:47
+```
+
+There is **no** `url` constructor parameter in either type, and `Url` is **settable**. This port
+had a third (respectively second) parameter .NET does not declare, feeding an accessor that .NET
+makes writable.
+
+## 4. To migrate
+
+```cpp
+// before
+CompilerFeatureRequiredAttribute f("RefStructs");
+f.setIsOptionalProperty(true);
+ObsoletedOSPlatformAttribute o("ios", "Use X", "https://example.com");
+RequiresPreviewFeaturesAttribute p("Preview", "https://aka.ms/preview");
+
+// after
+CompilerFeatureRequiredAttribute f("RefStructs", true);
+ObsoletedOSPlatformAttribute o("ios", "Use X");
+o.setUrlProperty("https://example.com");
+RequiresPreviewFeaturesAttribute p("Preview");
+p.setUrlProperty("https://aka.ms/preview");
+```
+
+**First-party migration was two sites**, both tests, and the compiler found both. Nothing in any
+`modules/*/src` uses these types.
+
+## 5. Evidence
+
+Five mutations, **all caught**, two at compile time:
+
+| Mutation | Caught by |
+|---|---|
+| M1 — `setIsOptionalProperty` reinstated | `Decl1980G4_IsOptionalIsNotMutableAfterConstruction` (compile time) |
+| M2 — the two-argument constructor ignores its second argument | `StoresFeatureNameAndOptionalFlag` |
+| M3 — the `url` constructor parameter restored | `Fix1980G4_UrlIsASettablePropertyNotAConstructorArgument` (compile time) |
+| M4 — `ObsoletedOSPlatformAttribute::setUrlProperty` silently does nothing | that type's own case |
+| M5 — `RequiresPreviewFeaturesAttribute::setUrlProperty` silently does nothing | that type's own case |
+
+M4 and M5 are run **once per type** on purpose: they are two separate declarations, and fixing
+one while leaving the other is the easy half-repair. M4 was invalid as first written — the anchor
+spanned two doc-comments that differ between the types — and was reformulated rather than counted.
+
+Negative consumer fixture: `test/consumer/runtime_g4_attribute_shape_negative.cpp`, four sites,
+all rejected. Fixture set grows to **42 fixtures / 219 sites**. Site 4 is the trait query — the
+shape that breaks a consumer **silently** rather than at the call site.
+
+Gate: **17,506 run, 17,506 passed, 0 failed, 0 skipped** across 38 executables — `+3` on 17,503
+(`SharpRuntimeTests_Runtime` 186 → 189; two pins migrated in place, three cases added). No other
+executable moved. Module graph unchanged at 41/93.
+
+## 6. Downstream, measured
+
+All three types appear in **zero** places in `cna` and **zero** in `mobile-eggbert`. Neither
+repository was modified, and no downstream ticket is needed.
+
+## 7. Scope
+
+This closes **G-4** of #1980. G-1 and G-2 landed earlier the same day. Remaining: **G-3**
+(reparenting and sealing — a vtable *and* layout change SA-3 excludes) and **G-5** / SR-AUD-167
+(retyping `MarshalAs` fields, adding `ComInterfaceType`/`ClassInterfaceType`).

docs/Migration-RuntimeAttributeShapesG4.md

@@ -31,14 +31,29 @@ class CompilerFeatureRequiredAttribute final : public System::Attribute {
     explicit CompilerFeatureRequiredAttribute(std::string featureName)
         : featureName_(std::move(featureName)) {}
 
+    /**
+     * Initializes the attribute with its feature name and optionality.
+     *
+     * #1980 group G-4 / SR-AUD-160. .NET declares `public bool IsOptional { get; init; }`
+     * (`CompilerFeatureRequiredAttribute.cs`), and `init` means the value can be supplied **at
+     * construction and never afterwards** -- `new CompilerFeatureRequiredAttribute("X") {
+     * IsOptional = true }` is legal, assigning to it later is not. C++ has no `init`, and the
+     * exact analogue of that pair of facts is a constructor parameter with no setter. This
+     * overload is what makes removing `setIsOptionalProperty` a *translation* rather than a
+     * narrowing: the value is still settable, just no longer mutable.
+     */
+    CompilerFeatureRequiredAttribute(std::string featureName, bool isOptional)
+        : featureName_(std::move(featureName)), isOptional_(isOptional) {}
+
     /** Gets the required compiler feature's name. */
     [[nodiscard]] const std::string& getFeatureNameProperty() const noexcept { return featureName_; }
 
     /** Gets whether a compiler may ignore an unknown feature name. */
     [[nodiscard]] bool getIsOptionalProperty() const noexcept { return isOptional_; }
 
-    /** Sets whether a compiler may ignore an unknown feature name. */
-    void setIsOptionalProperty(bool value) noexcept { isOptional_ = value; }
+    // #1980 G-4 / SR-AUD-160: setIsOptionalProperty is GONE. .NET's IsOptional is `{ get; init; }`
+    // -- settable at construction, immutable afterwards -- so a full setter published a mutability
+    // .NET does not have. Supply the value through the two-argument constructor above.
 };
 
 } // namespace System::Runtime::CompilerServices

modules/runtime/include/System/Runtime/CompilerServices/CompilerFeatureRequiredAttribute.hpp

@@ -100,21 +100,34 @@ namespace System::Runtime::Versioning {
         /**
          * @param platformName Platform on which the API is obsolete.
          * @param message      Optional deprecation message.
-         * @param url          Optional URL with migration guidance.
+         *
+         * #1980 group G-4 / SR-AUD-164. **There is no `url` parameter, deliberately.** .NET
+         * declares exactly two constructors -- `(platformName)` and `(platformName, message)`
+         * (`PlatformAttributes.cs`) -- and exposes the URL as a **settable property**,
+         * `public string? Url { get; set; }`. This port had it the other way round: a third
+         * constructor parameter .NET does not have, feeding a read-only accessor. Both halves
+         * were wrong, and in opposite directions.
          */
         explicit ObsoletedOSPlatformAttribute(const std::string& platformName,
-                                               const std::string& message = {},
-                                               const std::string& url = {})
-            : platformName_(platformName), message_(message), url_(url) {}
+                                               const std::string& message = {})
+            : platformName_(platformName), message_(message) {}
 
         /** @return The platform identifier. */
         [[nodiscard]] const std::string& getPlatformNameProperty() const { return platformName_; }
 
         /** @return The deprecation message, or empty if not provided. */
         [[nodiscard]] const std::string& getMessageProperty()      const { return message_; }
 
-        /** @return The migration URL, or empty if not provided. */
+        /** @return The migration URL, or empty if not set. */
         [[nodiscard]] const std::string& getUrlProperty()          const { return url_; }
+
+        /**
+         * @brief Sets the URL that provides more information about the obsolescence.
+         *
+         * #1980 G-4 / SR-AUD-164: .NET's `Url` is `{ get; set; }` -- a fully settable property,
+         * not a constructor parameter.
+         */
+        void setUrlProperty(const std::string& value) { url_ = value; }
     };
 
     /** Marks an API as requiring preview features that may change in future releases. */
@@ -127,16 +140,27 @@ namespace System::Runtime::Versioning {
 
         /**
          * @param message Explanation of why the API is preview.
-         * @param url     Optional URL with more information.
+         *
+         * #1980 group G-4 / SR-AUD-164. **No `url` parameter**, matching .NET's
+         * `public RequiresPreviewFeaturesAttribute(string? message)`
+         * (`RequiresPreviewFeaturesAttribute.cs:34`); the URL is a settable property there.
          */
-        explicit RequiresPreviewFeaturesAttribute(const std::string& message, const std::string& url = {})
-            : message_(message), url_(url) {}
+        explicit RequiresPreviewFeaturesAttribute(const std::string& message)
+            : message_(message) {}
 
         /** @return The informational message, or empty if not provided. */
         [[nodiscard]] const std::string& getMessageProperty() const { return message_; }
 
-        /** @return The informational URL, or empty if not provided. */
+        /** @return The informational URL, or empty if not set. */
         [[nodiscard]] const std::string& getUrlProperty()     const { return url_; }
+
+        /**
+         * @brief Sets the URL that provides more information.
+         *
+         * #1980 G-4 / SR-AUD-164: .NET's `Url` is `{ get; set; }`
+         * (`RequiresPreviewFeaturesAttribute.cs:47`).
+         */
+        void setUrlProperty(const std::string& value) { url_ = value; }
     };
 
 } // namespace System::Runtime::Versioning

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

@@ -122,16 +122,39 @@ TEST(StateMachineAttributeTests, AsyncAndIteratorAttributes_InheritStateMachineT
               System::Type::From<char>().getNameProperty());
 }
 
+namespace {
+    /// Dependent on purpose: gcc evaluates a NON-dependent `requires` eagerly and hard-errors on
+    /// the missing member instead of yielding false (the #2299 trap).
+    template <typename T>
+    concept HasSetIsOptional = requires(T a) { a.setIsOptionalProperty(true); };
+}
+
 TEST(CompilerFeatureRequiredAttributeTests, StoresFeatureNameAndOptionalFlag) {
+    // MIGRATED by #1980 group G-4 / SR-AUD-160. The old body called setIsOptionalProperty, which
+    // published a mutability .NET does not have: its IsOptional is `{ get; init; }` -- settable at
+    // construction, immutable afterwards. The value is now supplied through the constructor.
     CompilerFeatureRequiredAttribute attr("custom-feature");
     EXPECT_EQ(attr.getFeatureNameProperty(), "custom-feature");
     EXPECT_FALSE(attr.getIsOptionalProperty());
-    attr.setIsOptionalProperty(true);
-    EXPECT_TRUE(attr.getIsOptionalProperty());
+
+    CompilerFeatureRequiredAttribute optional("custom-feature", true);
+    EXPECT_EQ(optional.getFeatureNameProperty(), "custom-feature");
+    EXPECT_TRUE(optional.getIsOptionalProperty());
+
     EXPECT_EQ(CompilerFeatureRequiredAttribute::RefStructs, "RefStructs");
     EXPECT_EQ(CompilerFeatureRequiredAttribute::RequiredMembers, "RequiredMembers");
 }
 
+TEST(CompilerFeatureRequiredAttributeTests, Decl1980G4_IsOptionalIsNotMutableAfterConstruction) {
+    // `init` is settable-at-construction AND immutable-afterwards. Removing the setter without
+    // adding the constructor would have been a narrowing; keeping the setter would have published
+    // mutability .NET lacks. This asserts both halves at once.
+    static_assert(std::is_constructible_v<CompilerFeatureRequiredAttribute, std::string, bool>,
+                  "#1980 G-4: the value must still be settable at construction");
+    static_assert(!HasSetIsOptional<CompilerFeatureRequiredAttribute>,
+                  "#1980 G-4: .NET's IsOptional is init-only, so there is no setter");
+}
+
 TEST(CompilerMetadataMarkerAttributeTests, MarkersInstantiateAndDeriveFromAttribute) {
     ExtensionAttribute extension;
     RequiredMemberAttribute required;
@@ -640,11 +663,29 @@ TEST(UnsupportedOSPlatformGuardAttributeTests, Constructor_StoresPlatform) {
     EXPECT_EQ(attr.getPlatformNameProperty(), "windows");
 }
 
-TEST(ObsoletedOSPlatformAttributeTests, Constructor_AllFields) {
-    ObsoletedOSPlatformAttribute attr("ios", "Use X instead", "https://example.com");
+TEST(ObsoletedOSPlatformAttributeTests, Fix1980G4_UrlIsASettablePropertyNotAConstructorArgument) {
+    // MIGRATED by #1980 group G-4 / SR-AUD-164, and the port had BOTH halves wrong, in OPPOSITE
+    // directions: it took a third constructor parameter .NET does not have, and fed it into a
+    // read-only accessor where .NET's is `public string? Url { get; set; }`. .NET declares
+    // exactly `(platformName)` and `(platformName, message)` (PlatformAttributes.cs).
+    ObsoletedOSPlatformAttribute attr("ios", "Use X instead");
     EXPECT_EQ(attr.getPlatformNameProperty(), "ios");
     EXPECT_EQ(attr.getMessageProperty(), "Use X instead");
+    EXPECT_EQ(attr.getUrlProperty(), "") << "unset until assigned, as .NET's null is";
+
+    attr.setUrlProperty("https://example.com");
     EXPECT_EQ(attr.getUrlProperty(), "https://example.com");
+
+    static_assert(!std::is_constructible_v<ObsoletedOSPlatformAttribute,
+                                            std::string, std::string, std::string>,
+                  "#1980 G-4: .NET has no three-argument constructor");
+}
+
+TEST(ObsoletedOSPlatformAttributeTests, Fix1980G4_TheOneArgumentConstructorIsDotNets) {
+    ObsoletedOSPlatformAttribute attr("android");
+    EXPECT_EQ(attr.getPlatformNameProperty(), "android");
+    EXPECT_EQ(attr.getMessageProperty(), "");
+    EXPECT_EQ(attr.getUrlProperty(), "");
 }
 
 TEST(RequiresPreviewFeaturesAttributeTests, DefaultConstructor_EmptyMessage) {
@@ -657,6 +698,20 @@ TEST(RequiresPreviewFeaturesAttributeTests, Constructor_WithMessage) {
     EXPECT_EQ(attr.getMessageProperty(), "Preview feature");
 }
 
+TEST(RequiresPreviewFeaturesAttributeTests, Fix1980G4_UrlIsASettablePropertyNotAConstructorArgument) {
+    // The same inversion as ObsoletedOSPlatformAttribute, in the second of the two types G-4
+    // names. .NET: `public RequiresPreviewFeaturesAttribute(string? message)` and
+    // `public string? Url { get; set; }` (RequiresPreviewFeaturesAttribute.cs:34,47).
+    RequiresPreviewFeaturesAttribute attr("Preview feature");
+    EXPECT_EQ(attr.getUrlProperty(), "");
+    attr.setUrlProperty("https://aka.ms/preview");
+    EXPECT_EQ(attr.getUrlProperty(), "https://aka.ms/preview");
+
+    static_assert(!std::is_constructible_v<RequiresPreviewFeaturesAttribute,
+                                            std::string, std::string>,
+                  "#1980 G-4: .NET's constructor takes the message alone");
+}
+
 // ===========================================================================
 // CompilerGeneratedAttribute
 // ===========================================================================