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.

Sign up for GitHub

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

Jump to bottom

Implement relational patterns for decimal. #41912

Merged
merged 2 commits into from
Feb 28, 2020
Merged

Implement relational patterns for decimal. #41912

merged 2 commits into from
Feb 28, 2020

Conversation

gafter
Copy link
Member

@gafter gafter commented Feb 24, 2020 •
edited by jcouv
Loading

  • Implement relational patterns for decimal.
  • Implements and tests null purity checking for the type pattern object (e.g. in is not object).
  • Significantly improves the debug dumper for the decision dag builder.
  • Fixed a bug in EnumeratedValueSet and enhanced the fuzz tests to catch it.
  • Move to common random-set generation code.
  • Rename the typeclass interfaces xTC to IxTC.
  • Improve the ToString for char.
  • Add double complement tests.
  • Changes recommended in previous PR code review.

@RikkiGibson @agocke Can you review this please? It addresses one of the outstanding pattern-matching issues from #41502

Relates to #40727 (test plan)

@gafter
Implement relational patterns for decimal.
ba0123a 
* Implement relational patterns for decimal.
* Implements and tests null purity checking for the type pattern `object` (e.g. in `is not object`).
* Significantly improves the debug dumper for the decision dag builder.
* Fixed a bug in EnumeratedValueSet and enhanced the fuzz tests to catch it.
* Move to common random-set generation code.
* Rename the typeclass interfaces xTC to IxTC.
* Improve the `ToString` for `char`.
* Add double complement tests.
* Changes recommended in previous PR code review.
@gafter gafter added this to the Compiler.Net5 milestone Feb 24, 2020
@gafter gafter requested a review from a team as a code owner February 24, 2020 22:47
@gafter gafter self-assigned this Feb 24, 2020
gafter
gafter commented Feb 24, 2020
View reviewed changes
Compilers.sln Show resolved Hide resolved
@cston
Copy link
Member

cston commented Feb 24, 2020 •
edited by gafter
Loading 

            _factory.Syntax = test.Syntax;

Are callers expecting LowerTest() to change _factory.Syntax? If this is necessary, consider restoring original value before returning. #ByDesign

Refers to: src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs:253 in ba0123a. [](commit_id = ba0123a, deletion_comment = False)

RikkiGibson
RikkiGibson approved these changes Feb 25, 2020
View reviewed changes
Copy link
Contributor

@RikkiGibson RikkiGibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@gafter
Copy link
Member Author

gafter commented Feb 25, 2020 

            _factory.Syntax = test.Syntax;

Generally callers do not care much about Syntax, as we are past binding. It is for developer convenience only. If you care about it you need to set it immediately before using the factory to construct a node. I only care about it because I'm debugging this code.

In reply to: 590598886 [](ancestors = 590598886)

Refers to: src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Patterns.cs:253 in ba0123a. [](commit_id = ba0123a, deletion_comment = False)

@gafter gafter merged commit 400d27b into dotnet:features/patterns3 Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agocke agocke agocke left review comments

@RikkiGibson RikkiGibson RikkiGibson approved these changes

@cston cston cston approved these changes

Assignees

@gafter gafter

Labels
Area-Compilers New Language Feature - Pattern Matching Pattern Matching
Projects
None yet
Milestone
Compiler.Net5
Development

Successfully merging this pull request may close these issues.
4 participants
@gafter @cston @agocke @RikkiGibson