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

Add ScopedKind to IParameterSymbol and ILocalSymbol #64190

Merged
merged 4 commits into from
Sep 23, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -640,19 +640,6 @@ void visitFunctionPointerSignature(IMethodSymbol symbol)

foreach (var param in symbol.Parameters)
{
// https://github.com/dotnet/roslyn/issues/61647: Use public API.
#if DEBUG
if ((param as Symbols.PublicModel.ParameterSymbol)?.GetSymbol<ParameterSymbol>() is { } p)
{
Debug.Assert((p.EffectiveScope, ParameterHelpers.IsRefScopedByDefault(p)) switch
{
(DeclarationScope.Unscoped, false) => true,
(DeclarationScope.RefScoped, true) => true,
_ => false,
});
}
#endif

AddParameterRefKind(param.RefKind);

AddCustomModifiersIfNeeded(param.RefCustomModifiers);
Expand Down Expand Up @@ -828,8 +815,7 @@ public override void VisitParameter(IParameterSymbol symbol)
AddParameterRefKindIfNeeded(symbol);
AddCustomModifiersIfNeeded(symbol.RefCustomModifiers, leadingSpace: false, trailingSpace: true);

// https://github.com/dotnet/roslyn/issues/61647: Use public API.
if ((symbol as Symbols.PublicModel.ParameterSymbol)?.GetSymbol<ParameterSymbol>().EffectiveScope == DeclarationScope.ValueScoped &&
if (symbol.ScopedKind == ScopedKind.ScopedValue &&
format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.IncludeScoped))
{
AddKeyword(SyntaxKind.ScopedKeyword);
Expand Down Expand Up @@ -1129,9 +1115,8 @@ private void AddParameterRefKindIfNeeded(IParameterSymbol symbol)
{
if (format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeParamsRefOut))
{
// https://github.com/dotnet/roslyn/issues/61647: Use public API.
if ((symbol as Symbols.PublicModel.ParameterSymbol)?.GetSymbol<ParameterSymbol>() is { } parameter &&
parameter.EffectiveScope == DeclarationScope.RefScoped &&
if (symbol.ScopedKind == ScopedKind.ScopedRef &&
(symbol as Symbols.PublicModel.ParameterSymbol)?.GetSymbol<ParameterSymbol>() is { } parameter &&
!ParameterHelpers.IsRefScopedByDefault(parameter) &&
Copy link
Contributor

@AlekseyTs AlekseyTs Sep 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we missing IsRefScopedByDefault as a public API? Is there an alternative way to detect this in a language independent way? #Closed

Copy link
Contributor

@AlekseyTs AlekseyTs Sep 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not, perhaps we should use this condition only for C# symbols and not dismiss other symbols?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsRefScopedByDefault() relies on ModuleSymbol.UsesUpdatedEscapeRules, which is not tracked in VB, nor is scoped. Changed to use for C# symbols only.

!parameter.IsThis &&
format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.IncludeScoped))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,7 @@ public override void VisitLocal(ILocalSymbol symbol)
if (symbol.IsRef &&
format.LocalOptions.IncludesOption(SymbolDisplayLocalOptions.IncludeRef))
{
// https://github.com/dotnet/roslyn/issues/61647: Use public API.
if ((symbol as Symbols.PublicModel.LocalSymbol)?.GetSymbol<LocalSymbol>().Scope == DeclarationScope.RefScoped &&
if (symbol.ScopedKind == ScopedKind.ScopedRef &&
format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.IncludeScoped))
{
AddKeyword(SyntaxKind.ScopedKeyword);
Expand All @@ -208,8 +207,7 @@ public override void VisitLocal(ILocalSymbol symbol)
}
}

// https://github.com/dotnet/roslyn/issues/61647: Use public API.
if ((symbol as Symbols.PublicModel.LocalSymbol)?.GetSymbol<LocalSymbol>().Scope == DeclarationScope.ValueScoped &&
if (symbol.ScopedKind == ScopedKind.ScopedValue &&
format.CompilerInternalOptions.IncludesOption(SymbolDisplayCompilerInternalOptions.IncludeScoped))
{
AddKeyword(SyntaxKind.ScopedKeyword);
Expand Down
20 changes: 16 additions & 4 deletions src/Compilers/CSharp/Portable/Symbols/DeclarationScope.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,28 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
// https://github.com/dotnet/roslyn/issues/61647: Internally, scope is represented with this enum,
// but the public API uses a pair of IsRefScoped and IsValueScoped bools (see ILocalSymbol,
// IParameterSymbol, and ScopedRefAttribute). We should have a common representation.
// And we should use common terms for the attribute and enum names.
internal enum DeclarationScope : byte
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DeclarationScope

Replace with ScopedKind?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like it would be a reasonable change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
Unscoped = 0,
RefScoped = 1,
ValueScoped = 2,
}

internal static class DeclarationScopeExtensions
{
internal static ScopedKind AsScopedKind(this DeclarationScope scope)
{
return scope switch
{
DeclarationScope.Unscoped => ScopedKind.None,
DeclarationScope.RefScoped => ScopedKind.ScopedRef,
DeclarationScope.ValueScoped => ScopedKind.ScopedValue,
_ => throw ExceptionUtilities.UnexpectedValue(scope),
};
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ bool ILocalSymbol.IsFunctionValue

RefKind ILocalSymbol.RefKind => _underlying.RefKind;

ScopedKind ILocalSymbol.ScopedKind => _underlying.Scope.AsScopedKind();

bool ILocalSymbol.HasConstantValue => _underlying.HasConstantValue;

object ILocalSymbol.ConstantValue => _underlying.ConstantValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ IParameterSymbol IParameterSymbol.OriginalDefinition

RefKind IParameterSymbol.RefKind => _underlying.RefKind;

ScopedKind IParameterSymbol.ScopedKind => _underlying.EffectiveScope.AsScopedKind();

bool IParameterSymbol.IsDiscard => _underlying.IsDiscard;

bool IParameterSymbol.IsParams => _underlying.IsParams;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,6 @@ static void Main()
}
}";
var comp = CreateCompilation(source1, references: new[] { ref0 });
// https://github.com/dotnet/roslyn/issues/61647: If the [ScopedRef] scoped value is an int
// rather than a pair of bools, the compiler should reject attribute values that it does not recognize.
comp.VerifyDiagnostics();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9873,9 +9873,7 @@ private static void VerifyParameterSymbol(ParameterSymbol parameter, string expe
private static void VerifyParameterSymbol(IParameterSymbol parameter, string expectedDisplayString, RefKind expectedRefKind, DeclarationScope expectedScope)
{
Assert.Equal(expectedRefKind, parameter.RefKind);
// https://github.com/dotnet/roslyn/issues/61647: Use public API.
//Assert.Equal(expectedScope == DeclarationScope.RefScoped, parameter.IsRefScoped);
//Assert.Equal(expectedScope == DeclarationScope.ValueScoped, parameter.IsValueScoped);
Assert.Equal(expectedScope.AsScopedKind(), parameter.ScopedKind);
Assert.Equal(expectedDisplayString, parameter.ToDisplayString(displayFormatWithScoped));
}

Expand Down Expand Up @@ -10369,9 +10367,7 @@ private static void VerifyLocalSymbol(LocalSymbol local, string expectedDisplayS
private static void VerifyLocalSymbol(ILocalSymbol local, string expectedDisplayString, RefKind expectedRefKind, DeclarationScope expectedScope)
{
Assert.Equal(expectedRefKind, local.RefKind);
// https://github.com/dotnet/roslyn/issues/61647: Use public API.
//Assert.Equal(expectedScope == DeclarationScope.RefScoped, local.IsRefScoped);
//Assert.Equal(expectedScope == DeclarationScope.ValueScoped, local.IsValueScoped);
Assert.Equal(expectedScope.AsScopedKind(), local.ScopedKind);
Assert.Equal(expectedDisplayString, local.ToDisplayString(displayFormatWithScoped));
}

Expand Down
6 changes: 6 additions & 0 deletions src/Compilers/Core/Portable/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ Microsoft.CodeAnalysis.Diagnostics.SyntaxTreeAnalysisContext.IsGeneratedCode.get
Microsoft.CodeAnalysis.GeneratorDriver.ReplaceGenerators(System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.ISourceGenerator!> generators) -> Microsoft.CodeAnalysis.GeneratorDriver!
Microsoft.CodeAnalysis.ILocalSymbol.IsForEach.get -> bool
Microsoft.CodeAnalysis.ILocalSymbol.IsUsing.get -> bool
Microsoft.CodeAnalysis.ILocalSymbol.ScopedKind.get -> Microsoft.CodeAnalysis.ScopedKind
Microsoft.CodeAnalysis.IParameterSymbol.ScopedKind.get -> Microsoft.CodeAnalysis.ScopedKind
Microsoft.CodeAnalysis.ScopedKind
Microsoft.CodeAnalysis.ScopedKind.None = 0 -> Microsoft.CodeAnalysis.ScopedKind
Microsoft.CodeAnalysis.ScopedKind.ScopedRef = 1 -> Microsoft.CodeAnalysis.ScopedKind
Microsoft.CodeAnalysis.ScopedKind.ScopedValue = 2 -> Microsoft.CodeAnalysis.ScopedKind
override sealed Microsoft.CodeAnalysis.CompilationOptions.GetHashCode() -> int
static Microsoft.CodeAnalysis.ModuleMetadata.CreateFromMetadata(System.IntPtr metadata, int size, System.Action! onDispose) -> Microsoft.CodeAnalysis.ModuleMetadata!
Microsoft.CodeAnalysis.INamedTypeSymbol.IsFileLocal.get -> bool
5 changes: 5 additions & 0 deletions src/Compilers/Core/Portable/Symbols/ILocalSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ public interface ILocalSymbol : ISymbol
/// </summary>
RefKind RefKind { get; }

/// <summary>
/// Returns the scoped kind of the local.
/// </summary>
ScopedKind ScopedKind { get; }

/// <summary>
/// Returns false if the local variable wasn't declared as "const", or constant value was omitted or erroneous.
/// True otherwise.
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/Core/Portable/Symbols/IParameterSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ public interface IParameterSymbol : ISymbol
/// </summary>
RefKind RefKind { get; }

/// <summary>
/// Returns the scoped kind of the local.
Copy link
Contributor

@AlekseyTs AlekseyTs Sep 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

local

parameter? #Closed

/// </summary>
ScopedKind ScopedKind { get; }

/// <summary>
/// Returns true if the parameter was declared as a parameter array.
/// </summary>
Expand Down
27 changes: 27 additions & 0 deletions src/Compilers/Core/Portable/Symbols/ScopedKind.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// 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.

namespace Microsoft.CodeAnalysis
{
/// <summary>
/// Enumeration for kinds of scoped modifiers.
/// </summary>
public enum ScopedKind : byte
{
/// <summary>
/// Not scoped.
/// </summary>
None = 0,

/// <summary>
/// A ref scoped to the current method or block.
Copy link
Contributor

@AlekseyTs AlekseyTs Sep 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

current

enclosing? #Closed

/// </summary>
ScopedRef = 1,

/// <summary>
/// A value scoped to the current method or block.
/// </summary>
ScopedValue = 2,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
End Get
End Property

Private ReadOnly Property IParameterSymbol_ScopedKind As ScopedKind Implements IParameterSymbol.ScopedKind
Get
Return ScopedKind.None
End Get
End Property

Private ReadOnly Property IParameterSymbol_Type As ITypeSymbol Implements IParameterSymbol.Type
Get
Return Me.Type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
End Get
End Property

Private ReadOnly Property ILocalSymbol_ScopedKind As ScopedKind Implements ILocalSymbol.ScopedKind
Get
Return ScopedKind.None
End Get
End Property

Public MustOverride ReadOnly Property IsFunctionValue As Boolean Implements ILocalSymbol.IsFunctionValue

Friend ReadOnly Property IsCompilerGenerated As Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ public override TResult Accept<TArgument, TResult>(SymbolVisitor<TArgument, TRes

public ImmutableArray<CustomModifier> CustomModifiers => ImmutableArray.Create<CustomModifier>();

public ScopedKind ScopedKind => ScopedKind.None;

public bool IsDiscard => false;
}
}