-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Share certain raw string algorithms between lexer and parser. #80733
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
base: main
Are you sure you want to change the base?
Conversation
- Create RawStringIndentationHelper with shared logic for both lexer and parser - Move CheckForSpaceDifference, CharToString, and StartsWith to helper class - Update Lexer_RawStringLiteral.cs to use helper methods - Update LanguageParser_InterpolatedString.cs to use helper methods - Tests pass: 165 RawStringLiteralLexingTests + 37 RawStringLiteralCompilingTests Co-authored-by: CyrusNajmabadi <[email protected]>
Add UTF-8 BOM to match encoding requirements of other C# files in the project Co-authored-by: CyrusNajmabadi <[email protected]>
|
||
if (errorCode != 0) | ||
{ | ||
// We have a line where the indentation of that line isn't a prefix of indentation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is now shared with lexer.
// If we're not on a blank line then this is bad. That's a content line that doesn't start | ||
// with the indentation whitespace. If we are on a blank line then it's ok if the whitespace | ||
// we do have is a prefix of the indentation whitespace. | ||
var isBlankLine = SyntaxFacts.IsNewLine(TextWindow.PeekChar()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all this logic is now shared with the parser instead of each having to reeimplement it.
} | ||
} | ||
|
||
private static bool CheckForSpaceDifference( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all this logic is shared too.
var originalToken = this.EatToken(); | ||
|
||
var originalText = originalToken.ValueText; // this is actually the source text | ||
var originalTextSpan = originalText.AsSpan(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i ended up moving off of ROS because the generics trick we use here can't work with them because we're not ona runtime that allows-ref-structs in generics.
} | ||
|
||
CodeAnalysis.Syntax.InternalSyntax.SyntaxList<InterpolatedStringContentSyntax> getContent(ReadOnlySpan<char> originalTextSpan) | ||
CodeAnalysis.Syntax.InternalSyntax.SyntaxList<InterpolatedStringContentSyntax> getContent(TString originalTextSpan) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
general pattern here is that if we took a ROS we now take a TString. And operations on that ROS now become operations on the 'helper' struct that will operate on the actual tuple that backs things here.
} | ||
} | ||
|
||
private static bool CheckForSpaceDifference( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is now shared with lexer.
return currentIndex; | ||
} | ||
|
||
private static int ConsumeRemainingContentThroughNewLine(StringBuilder content, ReadOnlySpan<char> text, int currentIndex) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this moved to be a local function.
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
View with whitespace off.
@dotnet/roslyn-compiler ptal. |
Fixes #80731