Skip to content

Fix #3824: fold a hoisted argument null-guard into the chained ctor call#3829

Open
sailro wants to merge 1 commit into
icsharpcode:masterfrom
sailro:fix-ctor-initializer-nullguard
Open

Fix #3824: fold a hoisted argument null-guard into the chained ctor call#3829
sailro wants to merge 1 commit into
icsharpcode:masterfrom
sailro:fix-ctor-initializer-nullguard

Conversation

@sailro

@sailro sailro commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Fixes #3824.

When a constructor initializer argument contains argument-validation that throws
(e.g. arg ?? throw new ArgumentNullException(...)) and the argument is used by
more than one parameter, the compiler hoists the null-check in front of the
chained constructor call. MoveConstructorInitializer only inspected the first
body statement, so the hoisted guard defeated it and the chained call was left as
an illegal in-body this..ctor(...) / base..ctor(...) statement (a parse error).

Fix

When locating the chained call, skip leading argument null-guards
(if (param == null) throw ...;) and fold each one back into the first argument
that uses the parameter, as param ?? throw .... If a guard cannot be folded (its
parameter is not used by any argument), the constructor is left unchanged.

Before:

public PatternToken(string value)
{
    if (value == null)
    {
        throw new ArgumentNullException("value");
    }
    this..ctor(AsciiCategory.None, value.Length, value.Length, "value"); // parse error
    Value = value;
}

After:

public PatternToken(string value)
    : this(AsciiCategory.None, (value ?? throw new ArgumentNullException("value")).Length, value.Length, "value")
{
    Value = value;
}

Test

Pretty/ConstructorInitializers gains a #if CS70 case where the null-check is
hoisted because the parameter is used by more than one argument; it round-trips to
the folded initializer form.

Real-world occurrence: IbanNet PatternToken(string) (net462).

…ined ctor call

When an initializer argument contains an argument-validation null-check (e.g.
`arg ?? throw new ArgumentNullException(...)`) and the argument is used more than
once, the compiler hoists the null-check in front of the chained constructor
call. MoveConstructorInitializer only inspected the first body statement, so the
guard defeated it and the chained call was left as an illegal in-body
`this..ctor(...)` / `base..ctor(...)`.

Skip leading argument null-guards (if (param == null) throw ...;) when locating
the chained call, and fold each guard back into the first argument that uses the
parameter as `param ?? throw ...`. If a guard cannot be folded, the constructor
is left unchanged.

Assisted-by: Copilot:claude-opus-4.8:GitHub Copilot CLI

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chained constructor call emitted as illegal in-body this..ctor(...) when a null-check is hoisted before it

2 participants