Skip to content

[P2] Use the captured direct source value for patch-mode nested updater calls #29

Description

@AGiorgetti

Priority and scope

P2: A direct nullable getter is evaluated twice when patch mode selects a declared nested updater; side effects or a changing getter can break the null guard.

Reviewed default develop at 612e5c03bba3e811741385cd7440ffb5af43b8df. main at b9697f2f91aa737360479a434f7d3cc01ccaa8fd has the identical source tree.

Evidence and cause

Patch planning captures a direct nullable member, but the updater argument's direct-member branch ignores sourcePathCaptureName. It uses only throwCaptureName or the original getter. throwCaptureName is disabled in IgnoreNullSourceMembers mode. The configured-path branch does use its capture.

Minimal reproduction (proposed, not executed)

using Mammoth.LiteMapper;
[LiteMapper(IgnoreNullSourceMembers = true)]
public static partial class Mapper
{
    public static partial void Apply(Source source, Target target);
    public static partial void ApplyChild(Child source, ChildDto target);
}
public sealed class Source
{
    public int Reads;
    private readonly Child child = new Child { Value = 7 };
    public Child? Child { get { Reads++; return child; } }
}
public sealed class Child { public int Value { get; set; } }
public sealed class Target { public ChildDto Child { get; } = new ChildDto(); }
public sealed class ChildDto { public int Value { get; set; } }
// After Mapper.Apply(source, target), source.Reads must be 1.

Actual behavior predicted from source

The generated code is equivalent to if (source.Child is { } captured) { ApplyChild(source.Child, target.Child); }. The non-null getter runs twice. If it returns a child first and null second, the selected updater receives null despite the guard.

Expected behavior

Patch null checks and updater invocation must consume the same captured member value exactly once, preserving the existing destination when null.

Fix direction

Use sourcePathCaptureName in the direct-member updater argument just as for the configured-path case; preserve the existing Throw capture and per-member ordering.

Regression acceptance

  • Assert one getter read for present and null direct members
  • Use a getter returning different values on successive reads to prove the capture is actually consumed
  • Cover declared and explicitly selected handwritten updaters, void and returning forms
  • Preserve configured-path behavior and earlier-assignment retention on later failure
  • Assert no nullable argument warning is introduced

Duplicate review

Closed #13/#16 fixed the direct nullable updater under NullableMismatch.Throw. Closed #6/#9 added general patch captures. This is the still-unfixed combination of direct members, patch mode and nested updater dispatch. All 18 existing issue/PR records and 72 issue-conversation comments were inspected; none currently tracks this specific unresolved case.

Verification limits

This is a static source finding. The reproduction was not compiled or run, and no repository code, release workflow, or benchmark was executed in this review. The existing exact-head CI run passed build/test and Roslyn-host jobs; release jobs were skipped. It does not validate this proposed regression. No measured timing or allocation improvement is claimed.

Prepared with OpenAI Codex.


Executable validation : 2026-10-02

Confirmed at runtime. The proposed patch nested-updater fixture reads the present source getter twice and maps value 7. A stronger fixture returning the child on the first read and null on the second throws NullReferenceException after two reads, demonstrating that the captured value is not consumed. A null-first control reads once and preserves existing target value 42. These generated compilations have no nullable warnings. Handwritten/returning-updater variants were not separately executed.

Tested unchanged develop commit 612e5c03bba3e811741385cd7440ffb5af43b8df on Linux x64, SDK 10.0.401 / runtime 10.0.12, Roslyn 4.8.0. Validation used MSTest built-in contract assertions and the repository’s in-memory compilation/invocation approach. No production source changes or fixes were made. This focused evidence does not complete every proposed regression-acceptance case.

Prepared with OpenAI Codex.

Overall cloud-validation scope

The dated evidence above supersedes the earlier static-only verification statement for the listed cases; unexecuted regression-acceptance variants remain proposed. The unchanged full solution built in Release with zero warnings/errors. The existing suites recorded 653 passes and 2 failures across 655 tests, with no skips. Both failures, TrimmedConsumerPublishesAndRunsWithoutLiteMapperWarnings and NativeAotConsumerPublishesAndRunsWithoutLiteMapperWarnings, were cloud infrastructure blocked: ILLink's out-of-process ComputeManagedAssemblies task host failed with MSB4216 and Unix-domain pipe SocketException (13), permission denied. Trimming/AOT behavior was not established, and these tests are not intrinsically Windows-only. The repository's separate win-x64 final-package release lane still requires a Windows execution environment and Windows/MSVC toolchain.

The 20 focused generator tests comprised 14 intentional contract assertion failures reproducing defects and 6 passing controls/measurement cases; they are not an all-green acceptance suite. No production fixes or releases were made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    exec:agentAgentStack execution mode: agentpriority:p2AgentStack priority P2ready:agentAgentStack readiness: ready for agenttype:bugAgentStack work item type: bug

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions