Check modifiers on record positional members

[jcouv]

Fixes #45008
Corresponding spec update: dotnet/csharplang#3672

[AlekseyTs]

public sealed override bool IsImplicitlyDeclared [](start = 8, length = 48)

It looks like this change is unnecessary, given the comment in SynthesizedRecordConstructor.cs. #Closed

[AlekseyTs]

Done with review pass (iteration 2) #Closed

[jcouv]

📝 This is primarily to avoid cascading diagnostics (illustrated below), but also seems reasonable (why do extra analysis work on synthesized methods which we fully control?).

        [Fact, WorkItem(45008, "[https://github.com/dotnet/roslyn/issues/45008](https://github.com/dotnet/roslyn/issues/45008)")]
        public void PositionalMemberModifiers_RefOrOut()
        {
            var src = @"
record R(ref int P1, out int P2);
";

            var comp = CreateCompilation(src);
            comp.VerifyDiagnostics(
                // (2,9): error CS0177: The out parameter 'P2' must be assigned to before control leaves the current method
                // record R(ref int P1, out int P2);
                Diagnostic(ErrorCode.ERR_ParamUnassigned, "(ref int P1, out int P2)").WithArguments("P2").WithLocation(2, 9),
                // (2,10): error CS0631: ref and out are not valid in this context
                // record R(ref int P1, out int P2);
                Diagnostic(ErrorCode.ERR_IllegalRefParam, "ref").WithLocation(2, 10),
                // (2,22): error CS0631: ref and out are not valid in this context
                // record R(ref int P1, out int P2);
                Diagnostic(ErrorCode.ERR_IllegalRefParam, "out").WithLocation(2, 22)
                );
        }

[AlekseyTs]

why do extra analysis work on synthesized methods which we fully control?

We are not fully in control, there is user code in this method. Please revert this change. If you want to suppress specific diagnostics, suppress that specific diagnostics instead. It will be more robust to avoid making suppressions like that across the board. I personally, don't even think the diagnostics is cascading or should be suppressed, it reflects exactly what is happening, the parameter could be assigned in any of the initializers, or in base arguments.

---

In reply to: 454571864 [](ancestors = 454571864)

[AlekseyTs]

We might even come up with a scenario where the analysis produces useful diagnostics about code in initializers, or in base arguments.

---

In reply to: 455167934 [](ancestors = 455167934,454571864)

[jcouv]

Thanks. I understand now. We definitely want to analyze the base call.

[AlekseyTs]

Done with review pass (iteration 5) #Closed

[AlekseyTs]

LGTM (iteration 6), assuming CI is passing.