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

• remove in syntax from initial release? (merged)
• ref readonly locals? (implemented in 7.2, Andy)
• 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
• ref re-assignment (discussed in Oct 2nd LDM, C# 7.2 candidate)
• Overloading on in/byval should cause an error (Overloading on in/byval should cause an error #22132)
• Should referencing [IsReadOnly] methods without an [IsConst] modreq be allowed? (Should referencing [IsReadOnly] signatures without an [InAttribute] modreq be allowed? #19792) (yes)

Spec

• 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 (Allow taking unmanaged/pinned addresses of readonly lvalues. #22400)
• Note the typing rule for stackalloc (var x = stackalloc ... is pointer type for compat reasons, but condition ? stackalloc ... : stackalloc ... is Span)

Misc

• 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
  • verify IDE completion and formatting on ref readonly, in, return ref ..., ref ternary,
  • 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)
  • 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
• dogfood in Roslyn repo (Use 7.2 language features in Roslyn #26010)
• document PE Verify gaps (either with existing tracking issue, or creating a new one)

Ref readonly parameters

• Spec exists

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

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

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

• Where is it allowed or blocked?

  • combine with out or params (expect error)
  • 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 ...)
  • ref readonly optional parameter
  • using in and ref readonly in lambda
  • using in in delegate
  • using in in local function
  • using in in async and iterator methods (disallowed)
  • explicit in or ref readonly at call site (disallowed)
  • the order of ref and readonly does matter (what is the error? should we offer a fixer?)
  • Using in in pattern-based lowering:
    • Deconstruct method with in parameters is not applicable for deconstruction
    • Implementing an task-like with in in builder type
    • enumerable/enumerator GetEnumerator(in int count = 10) (see Overload resolution for GetEnumerator fails with optional parameters #19742)

• OHI

  • methods differing in ref/out/in (we only consider types)
  • 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)

• 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)

• in allowed in indexer and operator parameters

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

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

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

Ref readonly returns

• Spec exists

• in syntax disallowed in method declaration

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

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

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

• ref readonly on indexer (allowed)

• readonly is disallowed in return statement

• signature needs exact match in OHI

  • differentiates between in and ref

• ref-readonly-returning lambda?

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

• metadata:

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

Readonly struct

• 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
• Ref ternary with throw expression (Throw expression disallowed in ref ternary csharplang#919)
• Ref ternary as in argument (M(in condition ? ref x : ref y);)

Ref readonly extension methods

• ref readonly S Extension(ref readonly this S s) { return ref s; }
• void RRExtension<T>(ref readonly this T t) { ... } (expect error)
• 42.RRExtension() (ok, but makes temporary)
• readonlyField.RRExtension() (expect no temporary)
• refReadonlyParameter.RRExtension() (expect no temporary)
• 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)
• verify API declaration and usage from compilation, image, metadata-only image and ref assembly

Ref-like types and safety (Span)

• Spec exists
• See Neal's test plan: Test Plan for Span<T>, aka interior pointer, aka stackonly struct #20127
• 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
• dynamic and stackalloc
  • dynamic d = stackalloc int[10];
  • Span<dynamic> d = stackalloc dynamic[10];

Misc

• (b ? ref x : ref x).foo()
• x.y.foo() // ref may be inferred
• Interaction with dynamic? nullable?
• 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?