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

IDE work items for c# 9.0 patterns. #42368

Closed
20 of 29 tasks
CyrusNajmabadi opened this issue Mar 12, 2020 · 16 comments
Closed
20 of 29 tasks

IDE work items for c# 9.0 patterns. #42368

CyrusNajmabadi opened this issue Mar 12, 2020 · 16 comments

Comments

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Mar 12, 2020

Language/Compiler work: dotnet/csharplang#2850 dotnet/csharplang#3361

These are the things we will need to support and/or validate:

@jinujoseph
Copy link
Contributor

cc @mikadumont for info

@alrz
Copy link
Contributor

alrz commented Mar 16, 2020

we would take a refactoring to convert to/from if-chains to and/or patterns
would take a refactoring to convert long chains of comparisons to patterns.

Some other ideas for refactorings:

  • Convert subsequent switch case labels to or patterns
    case p: case q: -> case p or q:
  • Convert subsequent switch case labels to relational+and patterns
    case 'a': .. case 'z': -> case >= 'a' and <= 'z':

These might be useful to simplify current workarounds:

  • Convert switch expressions to is+or patterns if applicable i.e. all arm values are boolean constants
  • Use or-patterns in switch expression arms if applicable i.e. there's some identical arm values

Some of these might overlap though, and we should decide which one should be an analyzer versus a refactoring.

case string _ =>. Add analzyer that removes unnecessary _ .

I think this should be in the simplifier. There are other cases that need to be covered there.

Update existing pattern analyzers to try to use and, or, not patterns when appropriate

This should include if-to-switch and switch-statement-to-expression in particular, those are specifically limited due to lack of or-patterns.

Worth to consider:

  • fade out default: when the switch is already exhaustive.
  • fix overlapping/redundant relational patterns if the compiler doesn't complain e.g. =>2 and =<4 or =>3 and =<5

@CyrusNajmabadi
Copy link
Member Author

great stuff. Thanks @alrz !

@gafter
Copy link
Member

gafter commented Apr 14, 2020

New diagnostics for pattern-matching in C# 9.0

CS8780 ERR_DesignatorBeneathPatternCombinator

You aren't permitted to declare a pattern variable under an or or not
pattern combinator, so we produce this error when you attempt to do so.

  if (o is int or long l)        // error: A variable may not be declared within a 'not' or 'or' pattern.
  if (o is Bar { F: not int i }) // error: A variable may not be declared within a 'not' or 'or' pattern.

CS8781 ERR_UnsupportedTypeForRelationalPattern

The new relational patterns work only for the built-in types that
have comparison operator. If you attempt to use them for other
types it is an error

  string s = ...;
  if (s < "aardvark") // error: Relational patterns may not be used for a value of type 'string'.

CS8782 ERR_RelationalPatternWithNaN

The new relational patterns do not permit comparing with NaN.

  double d = ...;
  switch (d)
  {
      case < double.NaN: // error: Relational patterns may not be used for a floating-point NaN.

CS8793 WRN_GivenExpressionAlwaysMatchesPattern

When a new pattern form always matches in an is-pattern expression because the input is a constant, we warn.

  if (1 is < 10) // warning: The given expression always matches the provided pattern.

CS8794 WRN_IsPatternAlways

When a new pattern form always matches in an is-pattern (whether or not the input is a constant), we warn:

  int x = ...;
  if (x is <= int.MaxValue) // warning: The input always matches the provided pattern.

@tmat
Copy link
Member

tmat commented Apr 18, 2020

Active statement tracking in switch expression arms works well.
Found a couple of related issues:
[Debugger] Disambiguate sequence points with equal text span: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1106749.
[Compilers] Switch expression stepping not working correctly: #43468

@alrz
Copy link
Contributor

alrz commented Apr 24, 2020

[Sam][LowPriority] case string _ =>. Add analzyer that removes unnecessary _ .

@sharwell I can take this if you're not working on it.

@CyrusNajmabadi

A langversion-dependant reducer works here without a need for an analyzer, right?
(much like CSharpDefaultExpressionReducer)

@sharwell
Copy link
Member

@alrz Sure go ahead! Also consider the following case, which is not new for C# 9 but would benefit from the same simplification:

if (o is string _)

@CyrusNajmabadi
Copy link
Member Author

Closing out. Any remaining work/enhancements can come through directed issues.

@julealgon
Copy link

@gafter

You aren't permitted to declare a pattern variable under an or or not
pattern combinator

Can you point me to more documentation on this limitation?

I just stumbled upon it on a scenario where it didn't make a lot of sense to me:

if (x is { Code: not 1 code })
{
    return new Error(code);
}

The value 1 above is a "success" indicator in my domain, and I want to match "when the code is not 1" then generate an error with the actual code in it.

Can you elaborate why this is an invalid scenario? The restriction you mentioned doesn't seem to make sense when using constant values on the match IMHO.

Seems like the only way out of this is degrading the code into using a when like:

if (x is { Code: var code } when code != 1)
{
    return new Error(code);
}

FAKE EDIT:

Found a really weird (undocumented?) way of extracting the value, by repeating the same property twice:

if (x is { Code: not 1, Code: var code })
{
    return new Error(code);
}

@CyrusNajmabadi
Copy link
Member Author

Can you elaborate why this is an invalid scenario?

Because nothing in the language allows it. Neither hte 'not' nor 'constant' patterns allow you to name the result. We'd need a proposal allowing that for that to work :)

Found a really weird (undocumented?) way of extracting the value, by repeating the same property twice:

Now that is weird. @333fred @jcouv is that supposed to be allowed? Seems really really strange and unintentional.

@333fred
Copy link
Member

333fred commented Nov 6, 2020

Huh. I agree it's odd, but I don't see anything in the spec that would disallow this: https://github.com/dotnet/csharplang/blob/master/proposals/csharp-8.0/patterns.md#property-pattern.

@alrz
Copy link
Contributor

alrz commented Nov 6, 2020

@julealgon Try:

if (x is { Code: not 1 and var code })
{
    return new Error(code);
}

@julealgon
Copy link

Now that is weird. ... Seems really really strange and unintentional.

I agree @CyrusNajmabadi . I honestly thought it would give me some sort of error at runtime the first time I tried it, or match the values incorrectly.

@julealgon Try:

if (x is { Code: not 1 and var code })
{
   return new Error(code);
}

It works @alrz ! Interesting... is that how it is supposed to be done at the end of the day? I don't think I've ever seen an example like that.

@CyrusNajmabadi
Copy link
Member Author

is that how it is supposed to be done at the end of the day?

Yes. This is definitely a supported and used pattern today. In the future, we might consider a way of making this more terse. But absent that, the above is def a realistic way to go.

@Thaina
Copy link

Thaina commented Mar 17, 2021

@gafter

CS8780 ERR_DesignatorBeneathPatternCombinator

You aren't permitted to declare a pattern variable under an or or not
pattern combinator, so we produce this error when you attempt to do so.

  if (o is int or long l)        // error: A variable may not be declared within a 'not' or 'or' pattern.
  if (o is Bar { F: not int i }) // error: A variable may not be declared within a 'not' or 'or' pattern.

I wish this would be allowed

object obj = 1;
if(obj is not int i or <= 0)
    return;

// Do something with i when i is positive integer

@333fred
Copy link
Member

333fred commented Mar 17, 2021

@Thaina I expect that would fall out of dotnet/csharplang#4018.

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

9 participants