Test plan for "ref readonly" and related features (7.2)

[jcouv]

Write a test plan for "ref readonly", "readonly structs", "ref and ref readonly extensions" and "ref ternary"

LDM

• [x] remove in syntax from initial release? (merged)
• [x] ref readonly locals? (implemented in 7.2, Andy)
• [x] allowing ref readonly at call site (for those who dislike auto-ref)? (yes, but we're switching to in as keyword)
• [ ] throw expression in ref ternary
• [x] ref re-assignment (discussed in Oct 2nd LDM, C# 7.2 candidate)
• [ ] Overloading on in/byval should cause an error (#22132)
• [x] Should referencing [IsReadOnly] methods without an [IsConst] modreq be allowed? (#19792) (yes)

Spec

• [x] Merge the "decisions made" spec into the existing docs
• [ ] Note that in parameters are not allowed in iterator or async methods
• [ ] Note that you can take unmanaged address of readonly variable (https://github.com/dotnet/roslyn/pull/22400)
• [ ] Note the typing rule for stackalloc (var x = stackalloc ... is pointer type for compat reasons, but condition ? stackalloc ... : stackalloc ... is Span)

Misc

• [x] Add feature docs for all these features (latest rules on Span, ref re-assignment)
• [ ] Update Compiler test plan (stackalloc, in/ref readonly, ref struct)
• [ ] Review BCL work (are the new types everywhere? for example, Span in mono?)
• [ ] do IDE test pass
  • [x] verify IDE completion and formatting on ref readonly, in, return ref ..., ref ternary,
  • [x] CodeStyle for in versus ref readonly
  • [ ] Analyzer/code fixer to recommend readonly struct
  • [ ] If we choose to allow ref readonly at call site, CodeStyle for call site (for those who don't like auto-ref)
  • [x] Block EE (problem with emitting attribute types?)
  • [ ] Test Interactive
• [ ] Test on various runtimes (desktop, core, native, mono). May need a spec of expected runtime behavior.
• [ ] Triage remaining issues for the feature
• [x] dogfood in Roslyn repo (https://github.com/dotnet/roslyn/pull/26010)
• [ ] document PE Verify gaps (either with existing tracking issue, or creating a new one)

Ref readonly parameters

• [x] Spec exists

• [x] test passing too many modifiers ("ref readonly in" or "in ref readonly", etc)

• [x] verify API declaration and usage from compilation, image, metadata-only image and ref assembly

• [x] ref readonly property from metadata with different attributes on getter and property

• [x] Where is it allowed or blocked?

  • [x] combine with out or params (expect error)
  • [x] ref readonly is disallowed: in local declarations ref readonly var x = y;, in front of expressions var x = ref readonly y;, return ref readonly, foreach (readonly ref i in ...)
  • [x] ref readonly optional parameter
  • [x] using in and ref readonly in lambda
  • [x] using in in delegate
  • [x] using in in local function
  • [x] using in in async and iterator methods (disallowed)
  • [x] explicit in or ref readonly at call site (disallowed)
  • [x] the order of ref and readonly does matter (what is the error? should we offer a fixer?)
  • [x] Using in in pattern-based lowering:
    • [x] Deconstruct method with in parameters is not applicable for deconstruction
    • [x] Implementing an task-like with in in builder type
    • [x] enumerable/enumerator GetEnumerator(in int count = 10) (see #19742)

• [x] OHI

  • [x] methods differing in ref/out/in (we only consider types)
  • [x] invoke M(1); with void M(ref readonly int i) { } and void M(int i) { } in situation of ambiguity (because of inheritance or extension methods)

• [x] verify in is invariant (because of CLR limitation, just like out)

• [ ] Verify that IL for copy versus no-copy

  • [ ] Passing in an RValue should work (making a temp copy, verify IL)
  • [ ] Default values (temp copy)
  • [ ] LValue need no copy
  • [ ] test spilling for M(someInArg, async M2(), someOtherInArg)
    • someInArg could be local, constant, RefReadonlyM(refReadonlyField), someArray[index], field

• [ ] No writes allowed:

  • [ ] assignment, compound assignment, passing as ref
  • [ ] invocation (in S s, then s.Mutate();) (also allowed, operating on a temp copy, verify IL. But no copy if S is readonly struct)
  • [ ] calling into a method passing in through is ok
  • [ ] returning ref readonly parameter as ref (disallowed)
  • [ ] returning ref parameter as ref readonly (allowed)

• [ ] in parameters cannot be captured (lambda/async/iterator)

• [x] in allowed in indexer and operator parameters

• [x] test VB interop (calling blocked because modreq on overridable members and delegate/interface methods, but allowed on non-overridable members)

• [x] taking pointer to ref readonly parameter (in unsafe code) is disallowed

• [x] passing nameof expression as in argument (expect copy?)

Ref readonly returns

• [x] Spec exists

• [x] in syntax disallowed in method declaration

• [x] ref readonly return in async method (no syntax for it)

• [x] ref readonly return on operator is disallowed (no syntax for it)

  • [x] operator from metadata should be handled (load as operator, but use-site error?)

• [x] ref readonly on indexer (allowed)

• [x] readonly is disallowed in return statement

• [x] signature needs exact match in OHI

  • [x] differentiates between in and ref

• [x] ref-readonly-returning lambda?

• [x] calling with a discard (no syntax for it?)

• [x] metadata:

  • [x] IsReadOnlyAttribute gets embbeded if not found, disallowed in source
  • [x] what if the attribute exists, but has wrong ctor shape?
  • [x] If InAttribute modreq present but IsReadOnlyAttribute is missing, then cannot load metadata
  • [x] If IsReadOnlyAttribute is present, but InAttribute modreq is missing, then can load and this absence of modreq will be carried over when overriding.
  • [x] modreq always emitted (test VB interop)
  • [x] verify API declaration and usage from compilation, image, metadata-only image and ref assembly

Readonly struct

• [x] Spec
• [ ] readonly on class declaration and other illegal members
  • [ ] readonly is floating, but ref must be next to struct in readonly ref struct
  • [ ] partial must be before ref or struct (but what about the readonly?)
  • [ ] no in struct
• [ ] readonly on half a partial struct (allowed, just like other modifiers)
• [ ] How is it surfaced in symbol (semantic model)?
• [ ] verify API declaration and usage from compilation, image, metadata-only image and ref assembly
• [ ] readonly attribute is missing, readonly attribute specified in source, obsolete attribute is missing
• [ ] Obsolete attribute given by user wins. There should be a warning.
• [ ] can call void M(in S s) with M(this), but not void M(ref S s).
• [ ] can call void M(S s) with M(this), but that will make a copy.
• [ ] this cannot be captured by lambda or other
• [ ] taking a pointer to this is disallowed
• [ ] all fields must be readonly
• [ ] property setters and events are disallowed

Ref ternary

• [ ] Blocked in expression tree
• [ ] (b ? ref x : ref y).M() where M is ref extension method, regular extension method (error), regular method (error)
  • [ ] does it make copies? what if x or y or both are readonly structs?
• [ ] for M(b ? ref x : ref y) where void M(in ...), I expect no temporary.
• [ ] for b ? ref M() : ref M2() where ref readonly C M() (and same for M2), expect the ternary is readonly
• [ ] for b ? ref this : ref this where this refers to a readonly struct, is the ternary readonly?
• [ ] what if the branches differ in readonlyness in the examples above?
• [ ] what if b is known to be constant (compiler knows which branch will be executed)?
• [ ] Mixing ref and non-ref is disallowed. b ? ref x : y
• [ ] Nesting. b1 ? ref (b2 ? ref x : ref y) : ref z
• [ ] No ref coalescing operator ref x ?? ref y
• [ ] Pattern matching on a ref ternary, or on a ref readonly ternary (this may matter later with tuple/deconstruction pattern?)
• [ ] b ? ref x : ref default (disallowed)
• [ ] Ref ternaries in tuple literal
• [x] Ref ternary with throw expression (https://github.com/dotnet/csharplang/issues/919)
• [ ] Ref ternary as in argument (M(in condition ? ref x : ref y);)

Ref readonly extension methods

• [x] ref readonly S Extension(ref readonly this S s) { return ref s; }
• [x] void RRExtension<T>(ref readonly this T t) { ... } (expect error)
• [x] 42.RRExtension() (ok, but makes temporary)
• [x] readonlyField.RRExtension() (expect no temporary)
• [x] refReadonlyParameter.RRExtension() (expect no temporary)
• [x] M().RRExtension() (expect no temporary)
• [ ] this.RRExtension()
  • [ ] on readonly struct (expect no temporary)
  • [ ] on a plain struct (expect makes temporary)
• [ ] (ref readonly ternary).RRExtension() (expect no temporary)
• [ ] Why do we allow on rvalues? (I assume for consistency with methods)
• [x] verify API declaration and usage from compilation, image, metadata-only image and ref assembly

Ref-like types and safety (Span)

• [x] Spec exists
• [ ] See Neal's test plan: https://github.com/dotnet/roslyn/issues/20127
• [x] ref readonly string M(ref readonly string s = "hello") { return ref s; }. Same with value type. (gives a unsafe-to-escape diagnostic)
• [ ] test foreach with API pattern (as opposed to interface) on ref-like iterator
• [x] dynamic and stackalloc
  • [x] dynamic d = stackalloc int[10];
  • [x] Span<dynamic> d = stackalloc dynamic[10];

Misc

• [ ] (b ? ref x : ref x).foo()
• [ ] x.y.foo() // ref may be inferred
• [ ] Interaction with dynamic? nullable?
• [x] attributes IsReadOnly and IsByRefLike are disallowed in source

See also

• Language Feature Status
• The feature branch
• [Work items] Readonly ref parameters/returns
• Proposal (ref readonly, readonly struct, ref extensions)
• Ref-like types and safety,
• Decisions made

[jcouv]

FYI @VSadov @OmarTawfik I'll start gathering ideas for test plan.

[OmarTawfik]

After talking with @cston, it seems that we need to add tests for both EnC and EE for:

1. Generating IsReadOnly attribute if needed.
2. Evaluating ref-readonly-ness from source if necessary.
3. Overload resolution on such parameters.
4. Using attributes from referenced assemblies vs generating a new one.

I was going to do that in #18715, but this is getting too big of a PR, so I'll keep it for later.

cc @VSadov @jcouv

[tmat]

Is this doc up-to-date, so I can review it to see what's needed to make the debugging experience good?

[tmat]

Turns out it isn't. Specifically https://github.com/dotnet/csharplang/blob/master/proposals/readonly-ref.md#metadata-representaion.

[OmarTawfik]

@tmat for metadata representation specifically, I've the work to enable that in #18715. Basically, we added a new attribute to the framework (IsReadOnlyAttribute) and we will use that to annotate parameters and return types.

[tmat]

@OmarTawfik So we are not using custom modifiers anywhere? Last time I heard we used a mix. How do we mark local variables that are readonly ref? A specification clarifying these questions would be useful.

[OmarTawfik]

@tmat the plan (in a future PR) is to use modreqs on parameters and return types to break older compilers and other languages from using it (until they support the feature). @VSadov can you please comment on the local variables? is there anything I'm missing?

[tmat]

@OmarTawfik Could you update the spec please?

[OmarTawfik]

@tmat. Yes. I'll send a PR out later.

[tmat]

@OmarTawfik Thanks!

[OmarTawfik]

@jcouv about this part:

ref readonly string M(ref readonly string s = "hello") { return ref s; }. Same with value type. (gives a unsafe-to-escape diagnostic)

The current design doesn't prohibit such scenario. Can you explain?