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.

Sign up for GitHub

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

Pool instances of SymbolDisplayVisitor #70363

Merged
merged 3 commits into from
Nov 10, 2023
Merged

Pool instances of SymbolDisplayVisitor #70363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5fe957c
Enable nullable reference types in SymbolDisplayVisitor
sharwell Oct 12, 2023
ff56d76
Pool instances of SymbolDisplayVisitor
sharwell Oct 12, 2023
739bef5
Rename new properties to PascalCase
sharwell Oct 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplay.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,9 @@ private static ArrayBuilder<SymbolDisplayPart> PopulateDisplayParts(
}
else
{
var visitor = new SymbolDisplayVisitor(builder, format, semanticModelOpt, positionOpt);
var visitor = SymbolDisplayVisitor.GetInstance(builder, format, semanticModelOpt, positionOpt);
symbol.Accept(visitor);
visitor.Free();
}

return builder;
Expand Down
165 changes: 81 additions & 84 deletions src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Members.cs
View file
Open in desktop

Large diffs are not rendered by default.

131 changes: 65 additions & 66 deletions src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.Types.cs
View file
Open in desktop

Large diffs are not rendered by default.

148 changes: 91 additions & 57 deletions src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor.cs
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 3 additions & 5 deletions src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor_Constants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,14 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System.Diagnostics;
using System.Reflection;

namespace Microsoft.CodeAnalysis.CSharp
{
internal partial class SymbolDisplayVisitor
{
private void AddConstantValue(ITypeSymbol type, object constantValue, bool preferNumericValueOrExpandedFlagsForEnum = false)
private void AddConstantValue(ITypeSymbol type, object? constantValue, bool preferNumericValueOrExpandedFlagsForEnum = false)
{
if (constantValue != null)
{
Expand All @@ -24,7 +22,7 @@ private void AddConstantValue(ITypeSymbol type, object constantValue, bool prefe
else
{
AddKeyword(SyntaxKind.DefaultKeyword);
if (!format.MiscellaneousOptions.IncludesOption(SymbolDisplayMiscellaneousOptions.AllowDefaultLiteral))
if (!Format.MiscellaneousOptions.IncludesOption(SymbolDisplayMiscellaneousOptions.AllowDefaultLiteral))
{
AddPunctuation(SyntaxKind.OpenParenToken);
type.Accept(this.NotFirstVisitor);
Expand Down Expand Up @@ -60,7 +58,7 @@ protected override void AddLiteralValue(SpecialType type, object value)
break;
}

this.builder.Add(CreatePart(kind, null, valueString));
this.Builder.Add(CreatePart(kind, null, valueString));
}

protected override void AddBitwiseOr()
Expand Down
81 changes: 43 additions & 38 deletions src/Compilers/CSharp/Portable/SymbolDisplay/SymbolDisplayVisitor_Minimal.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,11 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;
Expand All @@ -25,17 +22,19 @@ private bool TryAddAlias(
var alias = GetAliasSymbol(symbol);
if (alias != null)
{
Debug.Assert(IsMinimizing);

// We must verify that the alias actually binds back to the thing it's aliasing.
// It's possible there's another symbol with the same name as the alias that binds
// first
var aliasName = alias.Name;

var boundSymbols = semanticModelOpt.LookupNamespacesAndTypes(positionOpt, name: aliasName);
var boundSymbols = SemanticModelOpt.LookupNamespacesAndTypes(PositionOpt, name: aliasName);

if (boundSymbols.Length == 1)
{
var boundAlias = boundSymbols[0] as IAliasSymbol;
if ((object)boundAlias != null && alias.Target.Equals(symbol))
if ((object?)boundAlias != null && alias.Target.Equals(symbol))
{
builder.Add(CreatePart(SymbolDisplayPartKind.AliasName, alias, aliasName));
return true;
Expand All @@ -48,14 +47,18 @@ private bool TryAddAlias(

protected override bool ShouldRestrictMinimallyQualifyLookupToNamespacesAndTypes()
{
var token = semanticModelOpt.SyntaxTree.GetRoot().FindToken(positionOpt);
Debug.Assert(IsMinimizing);

var token = SemanticModelOpt.SyntaxTree.GetRoot().FindToken(PositionOpt);
var startNode = token.Parent;

return SyntaxFacts.IsInNamespaceOrTypeContext(startNode as ExpressionSyntax) || token.IsKind(SyntaxKind.NewKeyword) || this.inNamespaceOrType;
return SyntaxFacts.IsInNamespaceOrTypeContext(startNode as ExpressionSyntax) || token.IsKind(SyntaxKind.NewKeyword) || this.InNamespaceOrType;
}

private void MinimallyQualify(INamespaceSymbol symbol)
{
Debug.Assert(IsMinimizing);

// only the global namespace does not have a containing namespace
Debug.Assert(symbol.ContainingNamespace != null || symbol.IsGlobalNamespace);

Expand All @@ -72,8 +75,8 @@ private void MinimallyQualify(INamespaceSymbol symbol)
// then that's all we need to add. Otherwise, we will add the minimally qualified
// version of our parent, and then add ourselves to that.
var symbols = ShouldRestrictMinimallyQualifyLookupToNamespacesAndTypes()
? semanticModelOpt.LookupNamespacesAndTypes(positionOpt, name: symbol.Name)
: semanticModelOpt.LookupSymbols(positionOpt, name: symbol.Name);
? SemanticModelOpt.LookupNamespacesAndTypes(PositionOpt, name: symbol.Name)
: SemanticModelOpt.LookupSymbols(PositionOpt, name: symbol.Name);
var firstSymbol = symbols.OfType<ISymbol>().FirstOrDefault();
if (symbols.Length != 1 ||
firstSymbol == null ||
Expand All @@ -83,16 +86,16 @@ private void MinimallyQualify(INamespaceSymbol symbol)
// we have one), a dot, and then our name.
var containingNamespace = symbol.ContainingNamespace == null
? null
: semanticModelOpt.Compilation.GetCompilationNamespace(symbol.ContainingNamespace);
: SemanticModelOpt.Compilation.GetCompilationNamespace(symbol.ContainingNamespace);
if (containingNamespace != null)
{
if (containingNamespace.IsGlobalNamespace)
{
Debug.Assert(format.GlobalNamespaceStyle == SymbolDisplayGlobalNamespaceStyle.Included ||
format.GlobalNamespaceStyle == SymbolDisplayGlobalNamespaceStyle.Omitted ||
format.GlobalNamespaceStyle == SymbolDisplayGlobalNamespaceStyle.OmittedAsContaining);
Debug.Assert(Format.GlobalNamespaceStyle == SymbolDisplayGlobalNamespaceStyle.Included ||
Format.GlobalNamespaceStyle == SymbolDisplayGlobalNamespaceStyle.Omitted ||
Format.GlobalNamespaceStyle == SymbolDisplayGlobalNamespaceStyle.OmittedAsContaining);

if (format.GlobalNamespaceStyle == SymbolDisplayGlobalNamespaceStyle.Included)
if (Format.GlobalNamespaceStyle == SymbolDisplayGlobalNamespaceStyle.Included)
{
AddGlobalNamespace(containingNamespace);
AddPunctuation(SyntaxKind.ColonColonToken);
Expand All @@ -107,7 +110,7 @@ private void MinimallyQualify(INamespaceSymbol symbol)
}

// If we bound properly, then we'll just add our name.
builder.Add(CreatePart(SymbolDisplayPartKind.NamespaceName, symbol, symbol.Name));
Builder.Add(CreatePart(SymbolDisplayPartKind.NamespaceName, symbol, symbol.Name));
}

private void MinimallyQualify(INamedTypeSymbol symbol)
Expand All @@ -124,6 +127,8 @@ private void MinimallyQualify(INamedTypeSymbol symbol)

if (!(symbol.IsAnonymousType || symbol.IsTupleType))
{
Debug.Assert(IsMinimizing);

if (!NameBoundSuccessfullyToSameSymbol(symbol))
{
// Just the name alone didn't bind properly. Add our minimally qualified parent (if
Expand All @@ -137,7 +142,7 @@ private void MinimallyQualify(INamedTypeSymbol symbol)
{
var containingNamespace = symbol.ContainingNamespace == null
? null
: semanticModelOpt.Compilation.GetCompilationNamespace(symbol.ContainingNamespace);
: SemanticModelOpt.Compilation.GetCompilationNamespace(symbol.ContainingNamespace);
if (containingNamespace != null)
{
if (containingNamespace.IsGlobalNamespace)
Expand Down Expand Up @@ -174,35 +179,35 @@ private IDictionary<INamespaceOrTypeSymbol, IAliasSymbol> CreateAliasMap()
// model, walk up the corresponding ancestors in the parent model.
SemanticModel semanticModel;
int position;
if (semanticModelOpt.IsSpeculativeSemanticModel)
if (SemanticModelOpt.IsSpeculativeSemanticModel)
{
semanticModel = semanticModelOpt.ParentModel;
position = semanticModelOpt.OriginalPositionForSpeculation;
semanticModel = SemanticModelOpt.ParentModel;
position = SemanticModelOpt.OriginalPositionForSpeculation;
}
else
{
semanticModel = semanticModelOpt;
position = positionOpt;
semanticModel = SemanticModelOpt;
position = PositionOpt;
}

var token = semanticModel.SyntaxTree.GetRoot().FindToken(position);
var startNode = token.Parent;
var startNode = token.Parent!;

// NOTE(cyrusn): If we're currently in a block of usings, then we want to collect the
// aliases that are higher up than this block. Using aliases declared in a block of
// usings are not usable from within that same block.
var usingDirective = GetAncestorOrThis<UsingDirectiveSyntax>(startNode);
if (usingDirective != null)
{
startNode = usingDirective.Parent.Parent;
startNode = usingDirective.Parent!.Parent!;
}

var usingAliases = GetAncestorsOrThis<BaseNamespaceDeclarationSyntax>(startNode)
.SelectMany(n => n.Usings)
.Concat(GetAncestorsOrThis<CompilationUnitSyntax>(startNode).SelectMany(c => c.Usings))
.Where(u => u.Alias != null)
.Select(u => semanticModel.GetDeclaredSymbol(u) as IAliasSymbol)
.Where(u => u != null);
.WhereNotNull();

var builder = ImmutableDictionary.CreateBuilder<INamespaceOrTypeSymbol, IAliasSymbol>();
foreach (var alias in usingAliases)
Expand All @@ -216,43 +221,43 @@ private IDictionary<INamespaceOrTypeSymbol, IAliasSymbol> CreateAliasMap()
return builder.ToImmutable();
}

private ITypeSymbol GetRangeVariableType(IRangeVariableSymbol symbol)
private ITypeSymbol? GetRangeVariableType(IRangeVariableSymbol symbol)
{
ITypeSymbol type = null;
ITypeSymbol? type = null;

if (this.IsMinimizing && !symbol.Locations.IsEmpty)
{
var location = symbol.Locations.First();
if (location.IsInSource && location.SourceTree == semanticModelOpt.SyntaxTree)
if (location.IsInSource && location.SourceTree == SemanticModelOpt.SyntaxTree)
{
var token = location.SourceTree.GetRoot().FindToken(positionOpt);
var token = location.SourceTree.GetRoot().FindToken(PositionOpt);
var queryBody = GetQueryBody(token);
if (queryBody != null)
{
// To heuristically determine the type of the range variable in a query
// clause, we speculatively bind the name of the variable in the select
// or group clause of the query body.
var identifierName = SyntaxFactory.IdentifierName(symbol.Name);
type = semanticModelOpt.GetSpeculativeTypeInfo(
type = SemanticModelOpt.GetSpeculativeTypeInfo(
queryBody.SelectOrGroup.Span.End - 1, identifierName, SpeculativeBindingOption.BindAsExpression).Type;
}

var identifier = token.Parent as IdentifierNameSyntax;
if (identifier != null)
{
type = semanticModelOpt.GetTypeInfo(identifier).Type;
type = SemanticModelOpt.GetTypeInfo(identifier).Type;
}
}
}

return type;
}

private static QueryBodySyntax GetQueryBody(SyntaxToken token) =>
private static QueryBodySyntax? GetQueryBody(SyntaxToken token) =>
token.Parent switch
{
FromClauseSyntax fromClause when fromClause.Identifier == token =>
fromClause.Parent as QueryBodySyntax ?? ((QueryExpressionSyntax)fromClause.Parent).Body,
fromClause.Parent as QueryBodySyntax ?? ((QueryExpressionSyntax)fromClause.Parent!).Body,
LetClauseSyntax letClause when letClause.Identifier == token =>
letClause.Parent as QueryBodySyntax,
JoinClauseSyntax joinClause when joinClause.Identifier == token =>
Expand All @@ -265,10 +270,10 @@ private static QueryBodySyntax GetQueryBody(SyntaxToken token) =>
private string RemoveAttributeSuffixIfNecessary(INamedTypeSymbol symbol, string symbolName)
{
if (this.IsMinimizing &&
format.MiscellaneousOptions.IncludesOption(SymbolDisplayMiscellaneousOptions.RemoveAttributeSuffix) &&
semanticModelOpt.Compilation.IsAttributeType(symbol))
Format.MiscellaneousOptions.IncludesOption(SymbolDisplayMiscellaneousOptions.RemoveAttributeSuffix) &&
SemanticModelOpt.Compilation.IsAttributeType(symbol))
{
string nameWithoutAttributeSuffix;
string? nameWithoutAttributeSuffix;
if (symbolName.TryGetWithoutAttributeSuffix(out nameWithoutAttributeSuffix))
{
var token = SyntaxFactory.ParseToken(nameWithoutAttributeSuffix);
Expand All @@ -282,7 +287,7 @@ private string RemoveAttributeSuffixIfNecessary(INamedTypeSymbol symbol, string
return symbolName;
}

private static T GetAncestorOrThis<T>(SyntaxNode node) where T : SyntaxNode
private static T? GetAncestorOrThis<T>(SyntaxNode node) where T : SyntaxNode
{
return GetAncestorsOrThis<T>(node).FirstOrDefault();
}
Expand All @@ -309,9 +314,9 @@ private IDictionary<INamespaceOrTypeSymbol, IAliasSymbol> AliasMap
}
}

private IAliasSymbol GetAliasSymbol(INamespaceOrTypeSymbol symbol)
private IAliasSymbol? GetAliasSymbol(INamespaceOrTypeSymbol symbol)
{
IAliasSymbol result;
IAliasSymbol? result;
return AliasMap.TryGetValue(symbol, out result) ? result : null;
}
}
Expand Down
Loading