Remove SyntaxTrivia from the compiler #11702

DustinCampbell · 2025-04-03T17:30:16Z

Currently, the syntax tree produced by the Razor compiler looks like it might support trivia. It even has a SyntaxTrivia class, methods on SyntaxNode and SyntaxToken to handle leading and trailing trivia, and methods on SyntaxVisitor, SyntaxRewriter and SyntaxWalker to visit trivia. There're even separate properties that look like they might provide different values depending on trivia, such as Width and FullWIdth.

None of that is real.

The Razor parser doesn't actually produce trivia. The leading and trailing trivia for a SyntaxToken is always empty. The FullWIdth and Width properties always return the same value, as do FullSpan and Span. Since there's so much infrastructure to support trivia, it seems like the Razor compiler might have generated it at some point, though I suspect it was just left over from an old copy-paste from Roslyn and never cleaned up.'

Honestly, it would be a major endeavor if the Razor compiler were updated to support trivia, and I personally don't believe the value is worth the effort. So, this change removes all of the compiler infrastructure in support of syntax trivia. The goal is to make steps toward eventually converting SyntaxToken to a struct. Today, SyntaxToken is defined as a class, which consistently shows up as a major source of allocations in perf traces.

I was pretty meticulous with my commit history and recommend reviewing commit-by-commit. Most of the changes are deletions. 😄

This method is essentially a no-op. It's purpose is to walk the leading and trailing trivia of a SyntaxToken, but the Razor parser doesn't actually produce tokens with trivia! On my machine, this method was called 10,478 times when creating a brand new Blazor Web App in VS, but it never amounted to anything except returning its input. Now, that's all it does.

With SyntaxRewriter.VisitToken(...)'s implementation removed, these methods are never called.

When I create a new Blazor Web app in VS, these methods called 20,186 times each (without opening any files!). In each call, nothing happens because there isn't actually any leading or trailing trivia. So, this change removes both methods. In addition, VisitToken(...) has been removed, since it's sole purpose is to call these methods.

These are no longer called now that SyntaxWalker.VisitLeadingTrivia and SyntaxWalker.VisitTrailingTrivia have been removed.

This is no longer caller now that the VisitToken(...) implementation has been removed.

These look impressive, but they're ultimately uncalled. Also, the comments seem to indicate that SyntaxToken is a struct, which it isn't.

These methods (and the overrides on SyntaxToken and SyntaxTrivia) are uncalled and can be removed.

These types are now only used by each other.

This is a simple change to delete two InternalSyntax.SyntaxToken constructors that aren't ever called.

Like the "red" SyntaxRewriter, InternalSyntax.SyntaxRewriter.VisitToken is essentially a no-op. It's purpose is to walk the leading and trailing trivia of an InternalSyntax.SyntaxToken, but the Razor parser doesn't actually produce tokens with trivia!

These methods are unused. In addition, remove the overrides on SyntaxToken along with the ToxkenWithLeadingTrivia and TokenWithTrailingTrivia methods that are used to implement them.

This properties are no longer accessed by anything.

These properties are never accessed.

Because the Razor parser never produces any trivia, GetLeadingTriviaWidth and GetTrailing TriviaWidth always return 0. So, they can be deleted along with their overrides. In addition, any calculations that used these methods can be performed without them.

Since trivia isn't produced by the Razor parser, the extra logic to write it out can be removed.

Since trivia doesn't ever exist, there's no need to compare it in InternalSyntax.SyntaxToken's IsEquivalentTo implementation.

No code calls these methods, so they can be removed. In addition, remove the overrides from InternalSyntax.SyntaxToken.

This change removes the _leadingTrivia and _trailingTrivia fields from InternalSyntax.SyntaxToken and removes one of the two constructors.

The SyntaxTrivia types have been detangled enough that they can be removed entirely.

Since there wasn't ever any trivia, FullWidth and Width were effectively the same value. This change consolidates them into Width.

Since there wasn't ever any trivia, FullSpan and Span were effectively the same TextSpan value. This change consolidates them into Span.

davidwengier

✅ Tooling

You get all of the fun jobs :)

DustinCampbell · 2025-04-04T20:54:59Z

You get all of the fun jobs :)

The fun part of this:

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Syntax/GreenNode.cs

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Syntax/SyntaxListOfT.cs

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Syntax/GreenNode.cs

chsienki

Love it.

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Syntax/GreenNode.cs

...iler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Syntax/InternalSyntax/SyntaxToken.cs

333fred

Sorry, but I need some time to think about this. I am very concerned that we'll just need to revert this change when the blazor team asks for @# at some point in the next release or two, and I'm not convinced that this is worth the potential cost.

The ContainsStructuredTrivia, ContainsDirectives, and ContainsSkippedText flags are not used by the Razor compiler and can be removed. In addition, remove the "#nullable disable" from NodeFlags.

These methods are unused.

These methods are unused. SyntaxNode.GetFirstToken() and SyntaxToken.GetLastToken() delegate to SyntaxNavigator.

Remove SyntaxList<T>.FullSpan, since it returns the same value as SyntaxList<T>.Span. In addition, tweak the XML doc comment for Span to not refer to leading and trailing trivia. (Aside: Interestly, the result of SyntaxList<T>.FullSpan would have always been incorrect if the Razor complier produced trivia.)

GreenNode.ToString() has an inconsistent implementation from ToFullString(). Instead of returning the the result of building a string from its subtree, it returns what appears to be a debugging value. It turns out that only one unit test depends on this ToString() result and is easy to update. So, this change makes ToString() return the same result as ToFullString(). The old ToString() implementation is used for debugger display.

Replaces callers of ToFullString() with calls to ToString() and removes ToFullString() implementations.

Concerns addressed through conversation

DustinCampbell · 2025-04-09T21:57:11Z

Concerns addressed through conversation

Thanks @333fred!

DustinCampbell added 22 commits April 3, 2025 06:38

SyntaxToken: Remove WithLeadingTrivia and WithTrailingTrivia

6237c2b

With SyntaxRewriter.VisitToken(...)'s implementation removed, these methods are never called.

SyntaxNode: Remove HasLeadingTrivia and HasTrailingTrivia

39a09d0

These are no longer called now that SyntaxWalker.VisitLeadingTrivia and SyntaxWalker.VisitTrailingTrivia have been removed.

SyntaxRewriter: Remove VisitList(SyntaxTriviaList)

00f4ae5

This is no longer caller now that the VisitToken(...) implementation has been removed.

SyntaxTriviaList: Remove Enumerator.InitializeFrom helpers

3060e2d

These look impressive, but they're ultimately uncalled. Also, the comments seem to indicate that SyntaxToken is a struct, which it isn't.

SyntaxNode: Remove GetLeadingTrivia and GetTrailingTrivia

ac8fdb9

These methods (and the overrides on SyntaxToken and SyntaxTrivia) are uncalled and can be removed.

Remove SyntaxTriviaList and SyntaxTriviaListBuilder

ab2325f

These types are now only used by each other.

Internal.SyntaxToken: Remove uncalled constructors

4371ff3

This is a simple change to delete two InternalSyntax.SyntaxToken constructors that aren't ever called.

GreenNode: Remove WithLeadingTrivia and WithTrailingTrivia

8bc91a1

These methods are unused. In addition, remove the overrides on SyntaxToken along with the ToxkenWithLeadingTrivia and TokenWithTrailingTrivia methods that are used to implement them.

Internal.SyntaxToken: Remove LeadingTrivia and TrailingTrivia

758cc7d

This properties are no longer accessed by anything.

GeenNode: Remove HasLeadingTrivia and HasTrailingTrivia

34f2a80

These properties are never accessed.

GreenNode: Remove logic to write leading/trailing trivia

bdcc24f

Since trivia isn't produced by the Razor parser, the extra logic to write it out can be removed.

Internal.SyntaxToken: Don't compare trivia in IsEquivalentTo

099c81f

Since trivia doesn't ever exist, there's no need to compare it in InternalSyntax.SyntaxToken's IsEquivalentTo implementation.

GreenNode: Remove GetLeadingTrivia() and GetTrailingTrivia()

339803e

No code calls these methods, so they can be removed. In addition, remove the overrides from InternalSyntax.SyntaxToken.

Internal.SyntaxToken: Remove leading and trailing trivia fields

d17f252

This change removes the _leadingTrivia and _trailingTrivia fields from InternalSyntax.SyntaxToken and removes one of the two constructors.

Remove SyntaxTrivia and InternalSyntax.SyntaxTrivia

5432996

The SyntaxTrivia types have been detangled enough that they can be removed entirely.

GreenNode: Consolidate FullWidth and Width

1977b4a

Since there wasn't ever any trivia, FullWidth and Width were effectively the same value. This change consolidates them into Width.

SyntaxNode: Consolidate FullWidth and Width

74259d7

Since there wasn't ever any trivia, FullWidth and Width were effectively the same value. This change consolidates them into Width.

SyntaxNode: Consolidate FullSpan and Span

2186f51

Since there wasn't ever any trivia, FullSpan and Span were effectively the same TextSpan value. This change consolidates them into Span.

DustinCampbell requested review from a team as code owners April 3, 2025 17:30

davidwengier approved these changes Apr 3, 2025

View reviewed changes

jjonescz approved these changes Apr 7, 2025

View reviewed changes

chsienki approved these changes Apr 7, 2025

View reviewed changes

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Syntax/GreenNode.cs Outdated Show resolved Hide resolved

...iler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Syntax/InternalSyntax/SyntaxToken.cs Outdated Show resolved Hide resolved

333fred previously requested changes Apr 7, 2025

View reviewed changes

Remove outdated comment and simplify GreenNode.WriteTo

194bae9

DustinCampbell added 7 commits April 7, 2025 10:55

Remove unused NodeFlags

111ae93

The ContainsStructuredTrivia, ContainsDirectives, and ContainsSkippedText flags are not used by the Razor compiler and can be removed. In addition, remove the "#nullable disable" from NodeFlags.

GreenNode: Remove GetValue and GetValueText

48b244a

These methods are unused.

GreenNode: Remove GetFirstToken/Terminal and GetLastToken/Terminal

559723d

These methods are unused. SyntaxNode.GetFirstToken() and SyntaxToken.GetLastToken() delegate to SyntaxNavigator.

Remove GreenNode.ToFullString() and SyntaxNode.ToFullString()

947d638

Replaces callers of ToFullString() with calls to ToString() and removes ToFullString() implementations.

Merge branch 'main' into remove-dead-syntax

684cae0

DustinCampbell merged commit b4c3326 into dotnet:main Apr 9, 2025
11 checks passed

DustinCampbell deleted the remove-dead-syntax branch April 9, 2025 23:33

dotnet-policy-service bot added this to the Next milestone Apr 9, 2025

This was referenced Apr 11, 2025

[Automated] PRs inserted in VS build feature.debugger.main-10510.153 #11729

Closed

[Automated] PRs inserted in VS build main-10510.94 #11730

Closed

[Automated] PRs inserted in VS build feature.telemetry.d18OpenTelemetry-10516.102 #11747

Closed

RikkiGibson modified the milestones: Next, 18.0 P1 Aug 20, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Remove SyntaxTrivia from the compiler #11702

Remove SyntaxTrivia from the compiler #11702

Uh oh!

DustinCampbell commented Apr 3, 2025 •

edited

Loading

Uh oh!

davidwengier left a comment

Uh oh!

DustinCampbell commented Apr 4, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chsienki left a comment

Uh oh!

Uh oh!

Uh oh!

333fred left a comment

Uh oh!

DustinCampbell commented Apr 9, 2025

Uh oh!

Uh oh!

Uh oh!

Remove SyntaxTrivia from the compiler #11702

Remove SyntaxTrivia from the compiler #11702

Uh oh!

Conversation

DustinCampbell commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidwengier left a comment

Choose a reason for hiding this comment

Uh oh!

DustinCampbell commented Apr 4, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chsienki left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

333fred left a comment

Choose a reason for hiding this comment

Uh oh!

DustinCampbell commented Apr 9, 2025

Uh oh!

Uh oh!

Uh oh!

DustinCampbell commented Apr 3, 2025 •

edited

Loading