Params parameters are implicitly scoped when their type is a ref struct.

[AlekseyTs]

UnscopedRefAttribute can be used to override that.

https://github.com/dotnet/csharplang/blob/main/proposals/params-collections.md#ref-safety

[AlekseyTs]

@RikkiGibson, @333fred, @dotnet/roslyn-compiler Please review. It looks like the CI failures are due to some test infrastructure issues.

[333fred]

Done review pass.

[333fred]

I think we need to modify these mismatch tests, or add more versions. The interesting cases for mismatches are cases where a missed scoped on an overridden method could cause a safety issue. For example, in the below code example, M1 and M2 have a compile error due to mismatched scoped, while M3 does not. All the examples above are of M3, as far as I can tell, so we should have variants that look like M1 and M2 that use params parameters.

class Base
{
    public virtual void M1(scoped Span<string> s, ref Span<string> s2) { }
    public virtual Span<string> M2(scoped Span<string> s) => throw new NotImplementedException();
    public virtual void M3(scoped Span<string> s) { }
}

class Derived : Base
{
    // error CS8987: The 'scoped' modifier of parameter 's' doesn't match overridden or implemented member.
    public override void M1(Span<string> s, ref Span<string> s2) => s2 = s;
    // error CS8987: The 'scoped' modifier of parameter 's' doesn't match overridden or implemented member.
    public override Span<string> M1(Span<string> s) => s;
    // Fine because there's no chance of `s` getting exposed.
    public override void M3(Span<string> s) { }
}
``` #Resolved

[AlekseyTs]

I think we need to modify these mismatch tests

Thanks. I thought I was missing something because I was not getting any mismatch errors even for non-params scenarios.

[AlekseyTs]

I updated the tests

[333fred]

This doesn't seem correct. Since params is scoped by default, we should have errors on the overrides for being unscoped. This implies that the following code will compile, which is a ref safety hole:

abstract class C1
{
    public abstract Span<long> Test1(params Span<long> a);
}
class C2 : C1
{
    public override Span<long> Test1(Span<long> a) => a;
}
``` #Resolved

[AlekseyTs]

This doesn't seem correct.

That might come as a surprise, but the params modifier, or lack there of, is getting inherited during overriding regardless of the modifiers on the overriding method. The parameters are implicitly params and, therefore, they are implicitly scoped. The asserts above actually demonstrate that the scope is identical to the overridden method for Test1 and Test2.

[333fred]

Alright. I think this is something to bring up with LDM and consider enforcing scoped or params on overiddes for this scenario.

[AlekseyTs]

@333fred For the second review

[333fred]

Alright. I think this is something to bring up with LDM and consider enforcing scoped or params on overiddes for this scenario.

[AlekseyTs]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).