Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test plan for "readonly members" #32911

Closed
52 tasks done
RikkiGibson opened this issue Jan 29, 2019 · 1 comment
Closed
52 tasks done

Test plan for "readonly members" #32911

RikkiGibson opened this issue Jan 29, 2019 · 1 comment
Assignees
Milestone

Comments

@RikkiGibson
Copy link
Contributor

RikkiGibson commented Jan 29, 2019

Proposal: dotnet/csharplang#1710

This issue is a place to collect test ideas and track their validation. Use the compiler test plan as a tool for brainstorming.

  • write speclet
  • update compiler test plan
  • validate with LangVersion 7.3

Methods

  • struct instance methods
  • struct static methods (should error)
  • class/interface static/instance methods (should error)
  • ref/ref readonly returning methods
  • Inherited object members should be considered readonly
  • 'readonly partial' methods
  • 'readonly partial' where one of declaration or implementation is missing keyword (error)
  • readonly on new ToString()

Properties

  • struct readonly getters
  • struct readonly setters (not auto-implemented)
  • struct static properties with readonly accessors (should error)
  • class/interface properties with readonly accessors (should error)
  • expression bodied properties int P readonly => 42; (should error)
  • indexers (allow)
  • readonly getter in a readonly property? (should be disallowed)
  • readonly on int P { readonly get; set; } allowed
  • getter implicitly readonly for all auto-props
  • int P { readonly get; } not allowed -> readonly int P { get; }

Misc

Semantic

IDE

- [ ] Extracting a method from a readonly method should produce a readonly method #34647
- [ ] Suggestion to make a member readonly when it is called on a ref readonly receiver #34648
- [ ] WRN_ImplicitCopyInReadOnlyMember should have a suggestion to make the callee readonly if possible #34649
- [ ] WRN_ImplicitCopyInReadOnlyMember should have a suggestion to make an explicit copy of the receiver

Major sections of work (ideally PRs will not cover more than one of these)

  • Parsing of readonly method and property syntax (mostly just testing)
  • IsReadOnly API on MethodSymbol
    • In the case of properties, is prop.GetMethod.IsReadOnly acceptable design, or is it confusing to have prop.IsReadOnly potentially return a different value than prop.GetMethod.IsReadOnly?
  • Flow analysis to prevent mutation of this in readonly member implementation
    • maybe the analysis used for in params or readonly struct can just be reused?
  • Error on invalid usage of readonly
    • Any method or property without a this parameter (i.e. statics)
    • Any member inside a non-struct declaration (i.e. class, interface)
  • Emit tests to demonstrate that expected metadata (IsReadOnly, modopt, modreq) and optimizations (reduced value copying) are present in IL

- [ ] Should some synthesized methods be marked as readonly? (discussion) #36587

  • Should we disallow readonly members on readonly structs, since redundant? Or maybe we create an IDE fixer?
    • Resolution: Should be allowed, based on the principle that changing struct to readonly struct on a type where all members are marked readonly shouldn't result in a compile error.
@jcouv
Copy link
Member

jcouv commented Jun 19, 2019

@RikkiGibson
It looks like there is only one unchecked bullet at this point (Should some synthesized methods be marked as readonly?). Can we get a resolution on that and close this issue? Alternatively, move this remaining question to a dedicated issue and close OP.

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

No branches or pull requests

3 participants