Support parsing for Unsigned Right Shift operator by AlekseyTs · Pull Request #60501 · dotnet/roslyn

AlekseyTs · 2022-03-31T02:28:37Z

https://github.com/dotnet/csharplang/blob/main/proposals/unsigned-right-shift-operator.md

Test plan #60433

https://github.com/dotnet/csharplang/blob/main/proposals/unsigned-right-shift-operator.md

Youssef1313 · 2022-03-31T06:01:00Z

src/Compilers/CSharp/Portable/Parser/LanguageParser.cs

+                    NoTriviaBetween(opToken2, tk)) // no trailing trivia and no leading trivia
+                {
+                    opToken2 = this.EatToken();
+                    opToken = SyntaxFactory.Token(opToken.GetLeadingTrivia(), SyntaxKind.GreaterThanGreaterThanGreaterThanToken, opToken2.GetTrailingTrivia());


Should MergeAdjacent be used? #Resolved

Should MergeAdjacent be used?

I am not inventing new ways of doing things, simply following what we do for SyntaxKind.GreaterThanGreaterThanToken.

CyrusNajmabadi · 2022-03-31T16:42:59Z

src/Compilers/CSharp/Portable/Parser/DocumentationCommentParser.cs

+                                operatorToken.ValueText + operatorToken2.ValueText + operatorToken3.ValueText,
+                                operatorToken3.GetTrailingTrivia());
+
+                            operatorToken = SyntaxFactory.MissingToken(SyntaxKind.PlusToken);


not entirely clear waht's going on here. Is x >>>= 1 not allowed?

oh. this is doc comment parsing. n/m/

fwiw, there feels like there's a lot of overlap between this and the same logic below for >>=. consider extracting a local function to help out here.

333fred

Couple of questions.

333fred · 2022-03-31T17:58:47Z

src/Compilers/CSharp/Portable/Parser/DocumentationCommentParser.cs

+                                operatorToken.Text + operatorToken2.Text + operatorToken3.Text,
+                                operatorToken.ValueText + operatorToken2.ValueText + operatorToken3.ValueText,
+                                operatorToken3.GetTrailingTrivia());
+


Why use a PlusToken, and not a GreaterThanToken? #Resolved

Why use a PlusToken, and not a GreaterThanToken?

Simply following the existing behavior on line 1092

333fred · 2022-03-31T18:13:10Z

src/Compilers/CSharp/Portable/Parser/LanguageParser.cs

@@ -10639,25 +10650,42 @@ private ExpressionSyntax ParseExpressionContinued(ExpressionSyntax leftOperand,
                var newPrecedence = GetPrecedence(opKind);

                // check for >> or >>=


Consider updating this comment. #Resolved

333fred · 2022-03-31T18:20:20Z

src/Compilers/CSharp/Test/Syntax/Parsing/ExpressionParsingTests.cs

@@ -3720,12 +3722,106 @@ void M()
        var j = e is a < i >>> 2;


Consider removing the comment above this line. #Closed

333fred · 2022-03-31T18:48:57Z

src/Compilers/CSharp/Test/Syntax/Parsing/TypeArgumentListParsingTests.cs

+{
+    void M()
+    {
+        var added = ImmutableDictionary<T<S>>>


Would it be useful to have a test with a tuple type next to the >>> as well? #Resolved

Would it be useful to have a test with a tuple type next to the >>> as well?

I am not aware of any specific reason why this could be useful, other than the fact that any test potentially has some value. Also, I think there is no test like that for >>, so I assume there isn't anything special there.

AlekseyTs · 2022-03-31T23:08:31Z

@jcouv, @dotnet/roslyn-compiler For the second review.

AlekseyTs · 2022-04-01T14:17:08Z

@jcouv, @dotnet/roslyn-compiler For the second review.

jcouv · 2022-04-01T23:06:43Z

src/Compilers/CSharp/Test/Syntax/Parsing/ExpressionParsingTests.cs

+                    }
+                }
+                EOF();
+            }


nit: I was trying to brainstorm cases to tests. Scenarios like is A<B<C<T>>>, as A<B<C<T>>>, (A<B<C<T>>>, Type) a = ... are probably already covered (plus those relate to parsing types, not parsing expressions).
So maybe a legit expression scenario: a < b < c < t >>> n, or an incremental scenario transitioning from type to expression or vice-versa?

jcouv

LGTM Thanks (iteration 2)

Youssef1313 · 2022-04-01T23:30:45Z

src/Compilers/CSharp/Test/Syntax/Parsing/ExpressionParsingTests.cs

+        {
+            foreach (var options in new[] { TestOptions.RegularPreview, TestOptions.Regular10, TestOptions.RegularNext })
+            {
+                UsingExpression("x >> >= y", options,


Consider a test similar to:

_ = x switch { A<B<C<D>>>=> 0, }

AlekseyTs · 2022-04-04T17:27:52Z

I'll follow up on test suggestions separately.

Support parsing for Unsigned Right Shift operator

d6ed8cb

https://github.com/dotnet/csharplang/blob/main/proposals/unsigned-right-shift-operator.md

AlekseyTs added Area-Compilers Feature - UnsignedRightShift labels Mar 31, 2022

AlekseyTs requested review from a team, 333fred and jcouv March 31, 2022 02:28

AlekseyTs requested a review from a team as a code owner March 31, 2022 02:28

Youssef1313 reviewed Mar 31, 2022

View reviewed changes

CyrusNajmabadi reviewed Mar 31, 2022

View reviewed changes

333fred reviewed Mar 31, 2022

View reviewed changes

333fred approved these changes Mar 31, 2022

View reviewed changes

PR feedback

decfbdb

jcouv reviewed Apr 1, 2022

View reviewed changes

jcouv approved these changes Apr 1, 2022

View reviewed changes

jcouv self-assigned this Apr 1, 2022

Youssef1313 reviewed Apr 1, 2022

View reviewed changes

Merge 'dotnet/features/UnsignedRightShift' into UnsignedRightShift_01

617c4c6

AlekseyTs enabled auto-merge (squash) April 4, 2022 17:26

AlekseyTs merged commit 6ab2107 into dotnet:features/UnsignedRightShift Apr 4, 2022

		@@ -10639,25 +10650,42 @@ private ExpressionSyntax ParseExpressionContinued(ExpressionSyntax leftOperand,
		var newPrecedence = GetPrecedence(opKind);

		// check for >> or >>=

Conversation

AlekseyTs commented Mar 31, 2022 • edited by jcouv Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Youssef1313 Mar 31, 2022 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Mar 31, 2022

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi Mar 31, 2022

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi Mar 31, 2022

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi Mar 31, 2022

Choose a reason for hiding this comment

Uh oh!

333fred left a comment

Choose a reason for hiding this comment

Uh oh!

333fred Mar 31, 2022 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Mar 31, 2022

Choose a reason for hiding this comment

Uh oh!

333fred Mar 31, 2022 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

333fred Mar 31, 2022 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

333fred Mar 31, 2022 • edited by AlekseyTs Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs Mar 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlekseyTs commented Mar 31, 2022

Uh oh!

AlekseyTs commented Apr 1, 2022

Uh oh!

jcouv Apr 1, 2022

Choose a reason for hiding this comment

Uh oh!

jcouv left a comment

Choose a reason for hiding this comment

Uh oh!

Youssef1313 Apr 1, 2022

Choose a reason for hiding this comment

Uh oh!

AlekseyTs commented Apr 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AlekseyTs commented Mar 31, 2022 •

edited by jcouv

Loading

Youssef1313 Mar 31, 2022 •

edited by AlekseyTs

Loading

333fred Mar 31, 2022 •

edited by AlekseyTs

Loading

333fred Mar 31, 2022 •

edited by AlekseyTs

Loading

333fred Mar 31, 2022 •

edited by AlekseyTs

Loading

333fred Mar 31, 2022 •

edited by AlekseyTs

Loading

AlekseyTs Mar 31, 2022 •

edited

Loading