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 "lambda discard parameters" #38820

Closed
30 of 34 tasks
jcouv opened this issue Sep 24, 2019 · 2 comments
Closed
30 of 34 tasks

Test plan for "lambda discard parameters" #38820

jcouv opened this issue Sep 24, 2019 · 2 comments
Assignees
Labels
Area-Compilers Test Test failures in roslyn-CI Test-Gap Describes a specific feature or scenario that does not have test coverage
Milestone

Comments

@jcouv
Copy link
Member

jcouv commented Sep 24, 2019

Championed issue

  • Specification checked in to csharplang (PR Create speclet for lambda discard parameters csharplang#2901)
  • (_, _) => 0 with LangVersion 8 (error) (see DiscardParameters_CSharp8)
  • (_, _) => 0 with LangVersion Preview (ok) (see DiscardParameters)
  • also works with explicit types (int _, string _) => 0 (see DiscardParameters_WithTypes)
  • also works in delegate syntax delegate(int _, string _) { return 0; } (see DiscardParameters_InDelegates)
  • discard parameters in delegates do not allow attributes (see DiscardParameters_InDelegates_WithAttribute)
  • discard parameters are not in scope and cannot be read from or written to (see DiscardParameters_NotInScope)
  • that a single _ is not a discard parameter (ie. is in scope and can be read from) (see DiscardParameters_NotADiscardWhenSingleUnderscore)
  • Verify semantic model on discard parameters (see DiscardParameters_NotInScope)
    • parameter symbol has IsDiscard set to true
    • parameter symbol has _ as name
    • parameter symbol has expected type and nullability
  • variable named _ allowed inside a lambda with discard parameters (see DiscardParameters)
  • discard parameters with ref, in and out parameters (see open question below) (see DiscardParameters_RefAndOut)
  • multiple _ parameters on local function (still error) (see DiscardParameters_OnLocalFunction)
  • unicode underscore \u005f doesn't cound as discard (same as in other contexts) (see DiscardParameters_UnicodeUnderscore)
  • @_ is not a discard (same as in other contexts) (see DiscardParameters_EscapedUnderscore)
  • test lambda in expression tree (disallow) (see DiscardParameters_ExpressionTreeNotAllowed)
  • test no regression on shadowing of underscore (see Implement lambda discard parameters #38786 (comment))

Questions raised on team review today:

  • confirm desired debugging and EE behavior. Are the values visible? F# doesn't show discards while debugging, but we should try if we can. (debugger-discards branch, issue Display discard values when debugging #48408)
  • LDM: confirm whether we want the discards to be in scope or block visibility in some way or disallow locals named _ within a scope where discard parameters exist. Some scenarios:
    • int _ = 1; Func<int, int, int> x = (_, _) => _++; /* this currently binding to the local declared outside the lambda? */
    • Func<int, int, int> x = (_, _) => { int _ = 1; return _; } /* this is currently allowed */
  • test nameof(_) (see DiscardParameters_NotInScope_Nameof)
  • verify GetAttributes() if we have a new symbol (latest implementation removed the new parameter symbol)
  • allow expression trees unless good reason not to (see DiscardParameters)
  • test IDE rename
  • test GetDeclaredSymbol (see DiscardParameters_NotInScope)
  • test FindAllRefs
  • LDM: should we extend the feature to local functions? (see lambda-to-local-function refactoring)

Productivity

  • converting from lambda with discards to local function (currently produces code with invalid parameters)
  • no IDE diagnostic reporting that discard parameters are unused
  • QuickInfo shows lambda parameter discards as (discard) int _
  • IDE doesn't use a special classification for discards (same as in other contexts)
  • test rename (see RenameLambdaDiscard)

Open issues for LDM:

  • Should we produce a distinct error for out or ref discard?
@jcouv jcouv added this to the Compiler.Next milestone Sep 24, 2019
@jcouv jcouv self-assigned this Sep 24, 2019
@gafter gafter added the Test Test failures in roslyn-CI label Sep 26, 2019
@jcouv jcouv modified the milestones: Compiler.Next, Compiler.Net5 Dec 11, 2019
@gafter gafter added the Test-Gap Describes a specific feature or scenario that does not have test coverage label Jan 2, 2020
@chsienki
Copy link
Contributor

Looks good, couple of notes:

  • IDE rename works, but incorrectly reports conflicts (true of single shadowed discard params too)
  • As noted lambda -> local function works but introduces invalid code.
  • EE during debugging will display correctly and allow editing but fail in immediate / locals / watch with ambiguity.
  • Hover in IDE doesn't show value during debugging, but does in watch / locals?
  • Discards seem to be allowed in expression trees / missing test?

@jcouv
Copy link
Member Author

jcouv commented Oct 7, 2020

Filed a follow-up issue for discards in debugger: #48408
I'll close this test plan now

@jcouv jcouv closed this as completed Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Test Test failures in roslyn-CI Test-Gap Describes a specific feature or scenario that does not have test coverage
Projects
Status: Done Umbrellas
Development

No branches or pull requests

5 participants