Skip to content

Pin production primary-constructor storage shape - #6048

Merged
richlander merged 2 commits into
mainfrom
feat/5255-primary-constructor-fallback
Sep 6, 2026
Merged

richlander merged 2 commits into
mainfrom
feat/5255-primary-constructor-fallback

Conversation

@richlander

@richlander richlander commented Sep 6, 2026 •

Copy link
Copy Markdown
Owner

Pins a reliable production source-shape foundation for CSharp memory-safety
spelling without adding redundant reconstruction machinery.

Fixes #6046; advances the narrow source-shape prerequisite in #5255 for #5257.
Overall tracker: #5226.

Card revision: 027256b4ffd5e6f20b4193e0ae06a21a182bc2f8.

Change

Conclusion: REVIEW — production already emits explicit fields and ordinary
constructors. This change records the Decompiler-owned contract and adds
compiler-produced regression evidence; it does not change product output.

The normative owner is docs/design/memory-safety-modes.md, Primary-constructor
storage shape
. CSharp independently owns declaration modifiers. There is no
new raise, fallback API, host-local policy, or compile-back-planner rewrite.

Demo

The new updated-rules fixture begins with a primary constructor and an explicitly
safe, offset-bearing field:

[StructLayout(LayoutKind.Explicit)]
public class ExplicitSafePrimaryStorage(int value)
{
    [FieldOffset(0)]
    public safe readonly int Value = value;
}

The production CLI at the card revision, using type with
-S "Decompiled Source" --bare, produces:

using System.Runtime.InteropServices;

namespace ILInspector.Decompiler.Fixtures.NewUnsafe;

[StructLayout(LayoutKind.Explicit)]
public class ExplicitSafePrimaryStorage
{
    [FieldOffset(0)]
    public readonly int Value;

    public ExplicitSafePrimaryStorage(int value) => Value = value;
}

The field and ordinary constructor remain separate declaration sites.
This is source-shape evidence, not completed v2 spelling: the missing
safe is the pre-existing #5257 work. No claim is made that this output already
compiles under updated semantics.

The ordinary-capture neighbor remains unchanged:

namespace LadderRung5;

public class PrimaryCounter
{
    private int seed;
    private int step;

    public PrimaryCounter(int seed, int step)
    {
        this.seed = seed;
        this.step = step;
    }

    public int Next() => seed + step;

    public int SeedPlus(int extra) => seed + extra;
}

The second new fixture covers an updated-rules explicit-layout struct whose
pointer-free field has an explicit unsafe caller contract. Its declaration and
parameter store remain explicit too.

Evidence

  • Baseline normal graph: dotnet build dotnet-inspect.slnx -c Release -m:2 -p:UseSharedCompilation=false -v:q succeeded.
  • Focused Release run: 19 passed, zero failed or skipped:
dotnet run --project src/ILInspector.Decompiler.Tests -c Release --no-build -- \
  -class ILInspector.Decompiler.Tests.PrimaryConstructorStorageTests \
  -class ILInspector.Decompiler.Tests.ConstructorCallDiagnosticsPassTests \
  -class ILInspector.Decompiler.Tests.LadderRung5GateTests
  • PrimaryConstructorStorageTests observes the first product projection:
    explicit fields, offsets, readonly state, ordinary constructor parameters,
    and parameter-dependent assignments. It parses but does not repair source.
  • Existing constructor-call gates preserve the visible decline for parameter
    stores before nontrivial base constructors; no ordering behavior changes.
  • npx markdownlint-cli docs/design/memory-safety-modes.md passed.
  • Structural A/B and changed-method census are not applicable: the exact
    authored diff contains one owner-document section, inspected fixture source,
    and product-outcome tests, with no raise/printer/product changes.

Compatibility and remaining work

The existing three-stage #5226 adoption stays: Metadata facts, shared CSharp
adoption, then CLI and browser/Wasm outcome gates. CLI whole-type decompilation
calls MemberBodyProducer.Project directly in ApiCommand; browser
SourceExports.QueryTypeSourceCore reaches the same composer through
AssemblyContextSourceQuery. No new adoption stage, dependency, platform
exception, or rendering architecture is introduced.

Required safe/unsafe spelling, extended-layout declaration legality, and
whole-output v2 compilation remain unverified by this slice and stay with
#5257/#5255. Generated-name authentication remains #5595. The harness's primary
constructor synthesis is not presented as production support.

Establish the Decompiler-owned source-shape prerequisite for CSharp safety spelling using the existing explicit-field and ordinary-constructor projection.

Refs #6046, #5255, #5257

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@richlander

Copy link
Copy Markdown
Owner Author

Round 1 attempt superseded before reviewer dispatch: the owner text incorrectly said both hosts use AssemblyContextSourceQuery. CLI whole-type decompilation calls MemberBodyProducer.Project directly (ApiCommand), while browser SourceExports reaches the same composer through that query. Correcting this documentation-only path description; no product behavior changes and no review round is spent.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@richlander

Copy link
Copy Markdown
Owner Author

Round 1 candidate: 027256b4ffd5e6f20b4193e0ae06a21a182bc2f8, effective base 94de72f5503dec46fde511dc98b07edc88d1aa49. Focused Release evidence remains 19 passed; the corrected head changes only the host-path description. Required CI has not yet produced an aggregate verdict. Fixed-head review is prepared and will dispatch after green ci-required, using GPT-6 Astra and Claude Opus 5 plus GPT-5.6 Sol for continuity with this session. Bounded status observation ends at 2026-09-06T02:30:25Z if unresolved. No merge authorization or auto-merge is in place.

@richlander

Copy link
Copy Markdown
Owner Author

Round 1 starts at exact head 027256b4ffd5e6f20b4193e0ae06a21a182bc2f8, effective base 94de72f5503dec46fde511dc98b07edc88d1aa49. Current-head ci-required is green. The fetched main tip 18cb8b53185ef80104619cc2ffb7293b91a3be14 has a clean non-mutating conflict probe; the candidate is not being integrated or changed. Dispatching the identical canonical candidate brief to GPT-6 Astra, Claude Opus 5, and GPT-5.6 Sol in separate fixed-head worktrees, as previously recorded. No merge authorization is in place.

@richlander

Copy link
Copy Markdown
Owner Author

Round 1 reconciliation

The production storage-shape foundation is review-clean without adding
reconstruction machinery or claiming completed v2 declaration spelling.

Reviewed head: 027256b4ffd5e6f20b4193e0ae06a21a182bc2f8.
Effective base: 94de72f5503dec46fde511dc98b07edc88d1aa49.

Reviewer Verdict Qualifying findings
GPT-6 Astra CLEAN None
Claude Opus 5 CLEAN None
GPT-5.6 Sol CLEAN None

All three reviewed the complete authored diff in isolated fixed-head worktrees.
No review-driven commit or product change was needed.

Non-blocking observation: derived safe versus recovered source

Opus correctly noted that the original safe keyword leaves no distinct
recoverable marker. That does not make required safe spelling unavailable.
The accepted CSharp owner contract, Required safe spelling, explicitly
says: "safe is derived spelling, not a recovered source keyword or a new
binary fact."

The class fixture contributes recognized updated module rules, explicit
layout, and a non-propagating field contract. Together with the retained
instance-field declaration, those facts allow #5257 to derive the required
safe choice. The test does not claim to recover the original keyword.
Therefore the PR's description of missing safe as unimplemented #5257
spelling remains accurate. This is a public clarification of the existing
contract, not a new requirement, source-provenance claim, or head change.

Carry-forward

Analyzed main movement from 94de72f5503dec46fde511dc98b07edc88d1aa49
through d119594d9553804591761f30f7f0de5a2749c905: no interaction.
No landed commit touches this PR's three authored files.

The nearest dependency changes are #5767's explicit-interface accessor
rendering, member-source selection, and method/accessor modifier facts;
they do not change capture-field or ordinary-constructor storage projection.
Whole-type query composition is unchanged. #6074 path-gates the fast
Decompiler lane using the existing Decompiler project-reference closure,
which includes this slice's fixture and test changes. Other landed work
concerns separate browser, package, delegation, discovery, and test-host
surfaces.

The local non-mutating merge probe is clean at that analyzed tip.
The reviewed head remains unchanged; no integration, new CI run, or repeat
review is warranted by this base movement.

Completion state

Current-head ci-required succeeded, observed again at
2026-09-06T04:49:40Z. The live PR snapshot reports MERGEABLE and
mergeStateStatus: CLEAN. The focused Release evidence remains 19 passing
cases; Opus independently reproduced it.

All required reviews are clean, and review-clean is being applied for the
reviewed head above. No merge authorization has been given; no merge or
auto-merge action has been taken.

@richlander richlander added the review-clean PR has met the adversarial review standard per AGENTS.md. label Sep 6, 2026
@richlander
richlander merged commit 715c7bd into main Sep 6, 2026
17 checks passed
@richlander
richlander deleted the feat/5255-primary-constructor-fallback branch September 6, 2026 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-clean PR has met the adversarial review standard per AGENTS.md.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pin the production primary-constructor storage fallback

1 participant