Improve error recovery around 'scoped' modifier parsing #81636

CyrusNajmabadi · 2025-12-10T13:26:23Z

We now allow a misplacced 'scoped' keyword to be treated as a normal modifier, and move teh reporting of issues to the binding phase. Today we get entirely thrown off and enter very bad error recovery that often gets thrown entirely off, creating tons of cascading errors.

Note: the net effect of this is much fewer errors, and much better error recovery. The added length in this PR is the addition of new tests, or updating existing tests to report binding errors to demonstrate that errors are still produced semantically where they would have previously been syntactic.

CyrusNajmabadi · 2025-12-10T14:35:23Z

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

+                return true;

-            using var beforeScopedResetPoint = this.GetDisposableResetPoint(resetOnDispose: false);
+            using var beforeScopedResetPoint = this.GetDisposableResetPoint(resetOnDispose: true);


no need to for dual reset points. and we can always trivially clean up. we just let the caller then eat the token if this function says it is a keyword.

CyrusNajmabadi · 2025-12-10T14:36:27Z

src/Compilers/CSharp/Test/Emit3/RefReadonlyParameterTests.cs

-            Diagnostic(ErrorCode.ERR_IdentifierExpected, "readonly").WithLocation(3, 37),
-            // (3,37): error CS1003: Syntax error, ',' expected
-            //     public static void M(ref scoped readonly int p) => throw null;
-            Diagnostic(ErrorCode.ERR_SyntaxError, "readonly").WithArguments(",").WithLocation(3, 37),


lots of the PR will just be syntax errors going away, and better errors about misplaced 'scopes' modifiers.

CyrusNajmabadi · 2025-12-10T14:36:45Z

src/Compilers/CSharp/Test/Emit3/RefReadonlyParameterTests.cs

-            Diagnostic(ErrorCode.ERR_RefReadOnlyWrongOrdering, "readonly").WithLocation(3, 26),
-            // (3,35): error CS0246: The type or namespace name 'scoped' could not be found (are you missing a using directive or an assembly reference?)
-            //     public static void M(readonly scoped ref int p) => throw null;
-            Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "scoped").WithArguments("scoped").WithLocation(3, 35),


or removing errors where we considered it a type name, but now realie it should be treated as a modifier.

CyrusNajmabadi · 2025-12-10T14:37:59Z

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

+                    Diagnostic(ErrorCode.ERR_ScopedAfterInOutRefReadonly, "scoped").WithLocation(1, 29),
+                    // (1,47): error CS8936: Feature 'ref fields' is not available in C# 10.0. Please use language version 11.0 or greater.
+                    // void F(ref scoped int b, in scoped int c, out scoped int d) { }
+                    Diagnostic(ErrorCode.ERR_ScopedAfterInOutRefReadonly, "scoped").WithLocation(1, 47)]);


the difference between these two is just the errors about a feature not being available. is tehre a helper to filter those out appropriately to make the test cleaner?

CyrusNajmabadi · 2025-12-10T14:38:26Z

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

-                // (1,54): error CS1003: Syntax error, ',' expected
-                // void F(ref scoped int b, in scoped int c, out scoped int d) { }
-                Diagnostic(ErrorCode.ERR_SyntaxError, "int").WithArguments(",").WithLocation(1, 54)
-                );


in some tests all syntax diagnostics went away. as such, i added semantic diagnostics to show we still report problems with the scoped modifier.

CyrusNajmabadi · 2025-12-10T14:38:47Z

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

-                    M(SyntaxKind.CommaToken);
-                    N(SyntaxKind.Parameter);
-                    {
+                        N(SyntaxKind.ScopedKeyword);


these tests demonstrate better understanding, with 'scoped' properly being treated as a modifier.

CyrusNajmabadi · 2025-12-10T14:39:30Z

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

+                    Diagnostic(ErrorCode.ERR_SemicolonExpected, "").WithLocation(1, 26)]);

-            N(SyntaxKind.ParenthesizedExpression);
+            N(SyntaxKind.ParenthesizedLambdaExpression);


we didn't even realize this was a lambda before...

CyrusNajmabadi · 2025-12-10T14:44:04Z

@dotnet/roslyn-compiler this is ready for review.

CyrusNajmabadi · 2025-12-10T14:45:00Z

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


-        private bool IsPossibleScopedKeyword(bool isFunctionPointerParameter)
-        {
-            using var _ = this.GetDisposableResetPoint(resetOnDispose: true);


for those keeping count, this made for a triple nested reset point. We now just have one reset point.

CyrusNajmabadi · 2025-12-10T14:45:53Z

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

+            // trivial case.  scoped ref/out/in/this  is definitely the scoped keyword. Note: the only actual legal
+            // cases are `scoped ref`, `scoped out`, and `scoped in`.  But we detect and allow `scoped this`, `scoped
+            // params` and `scoped readonly` as well.  These will be reported as errors later in binding.
+            if (IsParameterModifierExcludingScoped(this.CurrentToken))


we intentionally allow a broader set than before. so you can say things like scoped this. This is caught later in the binder.

CyrusNajmabadi · 2025-12-10T17:43:42Z

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

+                return false;

-            // In C# 14 we decided that within a lambda 'scoped' would *always* be a keyword.
+            // In C# 14 we decided that within a lambda 'scoped' would *always* be a modifier, not a type.


refining this comment. It wasn't 100% accurate. It's not always a keyword. But it is a modifier if it previous would have been allowed as a type.

CyrusNajmabadi · 2025-12-10T18:04:18Z

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

            bool isFunctionPointerParameter,
            bool isLambdaParameter)
        {
            if (this.CurrentToken.ContextualKind != SyntaxKind.ScopedKeyword)


this got much easier as we just need to return a bool saying if this should be considered a modifier. We don't actually have to do all the setting-up/rewinding if we change our minds. This means we just need a single reset point in here, and the caller can just eat depending on what we return.

src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs

CyrusNajmabadi · 2025-12-11T12:02:07Z

@dotnet/roslyn-compiler ptal. I would likke to take this before #81604 as the latter ends up changing error recovery at the top level, and we have a bunch of tests that test pathological 'scoped' cases at the top level. These result in bad cascading errors today, and bad (but different) cascading errors after that change (making the diff messy). By cleaning things up here and making scoped-parsing very resilient and clean, it improves the end user UX and helps out other changes to improve bad parsing in other scenarios.

Improve error recovery around 'scoped' modifier parsing

aa4a9a9

CyrusNajmabadi requested a review from a team as a code owner December 10, 2025 13:26

github-actions bot added the Area-Compilers label Dec 10, 2025

Delete

6e7a144

CyrusNajmabadi marked this pull request as draft December 10, 2025 13:27

CyrusNajmabadi added 9 commits December 10, 2025 14:32

Update test

70b8e1a

Preserve modifier/identifier logic from before

3e9d93b

Update tests

0be99eb

Update tsts

26b6685

Update tests

332babb

Update tests

686ad93

Update tests

b3e35fb

delete

179a870

delete

6e0779a

CyrusNajmabadi commented Dec 10, 2025

View reviewed changes

Update tests

742fdb7

CyrusNajmabadi marked this pull request as ready for review December 10, 2025 14:43

CyrusNajmabadi commented Dec 10, 2025

View reviewed changes

Simplify tests

4aa09cf

CyrusNajmabadi commented Dec 10, 2025

View reviewed changes

CyrusNajmabadi added 2 commits December 10, 2025 18:54

Add test

2a7b3a1

better error for 'scoped scoped'

85d0a2d

Docs and simplifying

3d8d9fc

CyrusNajmabadi commented Dec 10, 2025

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs Outdated Show resolved Hide resolved

CyrusNajmabadi added 3 commits December 10, 2025 19:05

Revert

14db92b

Merge remote-tracking branch 'upstream/main' into scopedParsing

29b92fb

Update tests

fb0a724

This was referenced Dec 11, 2025

Improve error recovery around modifiers in a declaration-expression. #81655

Draft

Improve error recovery in top level statements #81604

Draft

Improve error recovery around 'scoped' modifier parsing #81636

Are you sure you want to change the base?

Improve error recovery around 'scoped' modifier parsing #81636

Conversation

CyrusNajmabadi commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi commented Dec 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CyrusNajmabadi commented Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

CyrusNajmabadi commented Dec 10, 2025 •

edited

Loading