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.

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

Fix excessive compilation times due to speculative parsing after an incomplete string #72565

Merged
merged 9 commits into from
Mar 20, 2024
52 changes: 49 additions & 3 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,7 @@ private AttributeSyntax ParseAttribute()
SyntaxKind.CloseParenToken,
static @this => @this.IsPossibleAttributeArgument(),
static @this => @this.ParseAttributeArgument(),
immediatelyAbort,
skipBadAttributeArgumentTokens,
allowTrailingSeparator: false,
requireOneElement: false,
Expand All @@ -1058,6 +1059,20 @@ static PostSkipAction skipBadAttributeArgumentTokens(
static (p, closeKind) => p.CurrentToken.Kind == closeKind,
expectedKind, closeKind);
}

static bool immediatelyAbort(AttributeArgumentSyntax argument)
{
// We can be very thrown off by incomplete strings in an attribute argument (especially as the lexer
// will restart on the next line with the contents of the string then being interpreted as more
// arguments). Bail out in this case to prevent going off the rails.
if (argument.expression is LiteralExpressionSyntax { Kind: SyntaxKind.StringLiteralExpression, Token: var literalToken } &&
literalToken.GetDiagnostics().Contains(d => d.Code == (int)ErrorCode.ERR_NewlineInConst))
{
return true;
}

return false;
}
}

private bool IsPossibleAttributeArgument()
Expand Down Expand Up @@ -13348,6 +13363,7 @@ private delegate PostSkipAction SkipBadTokens<TNode>(
/// All the callbacks should passed as static lambdas or static methods to prevent unnecessary delegate
/// allocations.
/// </remarks>

private SeparatedSyntaxList<TNode> ParseCommaSeparatedSyntaxList<TNode>(
ref SyntaxToken openToken,
SyntaxKind closeTokenKind,
Expand All @@ -13357,6 +13373,31 @@ private SeparatedSyntaxList<TNode> ParseCommaSeparatedSyntaxList<TNode>(
bool allowTrailingSeparator,
bool requireOneElement,
bool allowSemicolonAsSeparator) where TNode : GreenNode
{
return ParseCommaSeparatedSyntaxList(
ref openToken,
closeTokenKind,
isPossibleElement,
parseElement,
immediatelyAbort: null,
skipBadTokens,
allowTrailingSeparator,
requireOneElement,
allowSemicolonAsSeparator);
}

#nullable enable

private SeparatedSyntaxList<TNode> ParseCommaSeparatedSyntaxList<TNode>(
ref SyntaxToken openToken,
SyntaxKind closeTokenKind,
Func<LanguageParser, bool> isPossibleElement,
Func<LanguageParser, TNode> parseElement,
Func<TNode, bool>? immediatelyAbort,
SkipBadTokens<TNode> skipBadTokens,
bool allowTrailingSeparator,
bool requireOneElement,
bool allowSemicolonAsSeparator) where TNode : GreenNode
{
// If we ever want this function to parse out separated lists with a different separator, we can
// parameterize this method on this value.
Expand All @@ -13369,15 +13410,17 @@ private SeparatedSyntaxList<TNode> ParseCommaSeparatedSyntaxList<TNode>(
if (requireOneElement || shouldParseSeparatorOrElement())
{
// first argument
nodes.Add(parseElement(this));
var node = parseElement(this);
nodes.Add(node);

// now that we've gotten one element, we don't require any more.
requireOneElement = false;

// Ensure that if parsing separators/elements doesn't move us forward, that we always bail out from
// parsing this list.
int lastTokenPosition = -1;
while (IsMakingProgress(ref lastTokenPosition))

while (immediatelyAbort?.Invoke(node) != true && IsMakingProgress(ref lastTokenPosition))
{
if (this.CurrentToken.Kind == closeTokenKind)
break;
Expand All @@ -13402,7 +13445,8 @@ private SeparatedSyntaxList<TNode> ParseCommaSeparatedSyntaxList<TNode>(
}
}

nodes.Add(parseElement(this));
node = parseElement(this);
nodes.Add(node);
continue;
}

Expand Down Expand Up @@ -13441,6 +13485,8 @@ bool shouldParseSeparatorOrElement()
}
}

#nullable disable

private DisposableResetPoint GetDisposableResetPoint(bool resetOnDispose)
=> new DisposableResetPoint(this, resetOnDispose, GetResetPoint());

Expand Down
Loading