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

Support 'scoped' modifier for parameters and locals #61389

Merged
merged 33 commits into from
Jun 3, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
0055014
Support 'scoped' modifier for parameters and locals
cston May 18, 2022
405ce6a
Misc.
cston May 18, 2022
c67d771
Missing ConvertToKeyword()
cston May 20, 2022
35672d2
Bind scope for lambda parameters
cston May 20, 2022
88de445
PR feedback
cston May 21, 2022
322a61c
Update parsing
cston May 23, 2022
0d48f5a
Report error if scoped value is not ref struct
cston May 23, 2022
413b85b
Fix tests
cston May 25, 2022
6406e1c
More tests
cston May 25, 2022
f24747d
Add IsRefScoped and IsValueScoped to public API
cston May 25, 2022
290be82
Fix build
cston May 26, 2022
48a7a21
Parse 'scoped' as modifier regardless of -langversion
cston May 26, 2022
2346121
Fix tests
cston May 26, 2022
ba42c34
Update DeclarationScope enum
cston May 26, 2022
47c6f90
Additional tests
cston May 26, 2022
dbcd9ff
Misc.
cston May 26, 2022
5a376f2
PR feedback
cston May 29, 2022
12e5e65
Update tests
cston May 29, 2022
dd2b6a7
More tests
cston May 30, 2022
4ff6d0f
PR feedback
cston Jun 1, 2022
8f2cc65
Treat method as not supported if parameter has unexpected LifetimeAnn…
cston Jun 1, 2022
88f0053
Update SymbolDisplay
cston Jun 1, 2022
7d31323
PR feedback
cston Jun 2, 2022
3432100
Disallow scoped in function pointer signatures
cston Jun 2, 2022
1888ac7
PR feedback
cston Jun 2, 2022
6028395
Filter out attribute from PEParameterSymbol.GetAttributes()
cston Jun 2, 2022
d90e587
Revert some public API changes
cston Jun 2, 2022
452d6a5
Formatting
cston Jun 2, 2022
7d82030
Fix tests
cston Jun 3, 2022
65fb9fd
Revert public API changes
cston Jun 3, 2022
436677a
Rename helper method
cston Jun 3, 2022
5a3815a
Fix ScanExplicitlyTypedLambda()
cston Jun 3, 2022
3c5d140
Filter out attribute unconditionally
cston Jun 3, 2022
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 @@ -767,9 +767,23 @@ public override void VisitParameter(IParameterSymbol symbol)

if (includeType)
{
if (symbol.IsRefScoped &&
format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeScoped))
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 1, 2022

Choose a reason for hiding this comment

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

format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeScoped)

ref is added conditionally. Perhaps we should also the option that controls that. #Closed

{
AddKeyword(SyntaxKind.ScopedKeyword);
AddSpace();
}

AddParameterRefKindIfRequired(symbol.RefKind);
AddCustomModifiersIfRequired(symbol.RefCustomModifiers, leadingSpace: false, trailingSpace: true);

if (symbol.IsValueScoped &&
format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeScoped))
{
AddKeyword(SyntaxKind.ScopedKeyword);
AddSpace();
}

if (symbol.IsParams && format.ParameterOptions.IncludesOption(SymbolDisplayParameterOptions.IncludeParamsRefOut))
{
AddKeyword(SyntaxKind.ParamsKeyword);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,13 @@ public override void VisitLocal(ILocalSymbol symbol)
if (symbol.IsRef &&
format.LocalOptions.IncludesOption(SymbolDisplayLocalOptions.IncludeRef))
{
if (symbol.IsRefScoped &&
format.LocalOptions.IncludesOption(SymbolDisplayLocalOptions.IncludeScoped))
{
AddKeyword(SyntaxKind.ScopedKeyword);
AddSpace();
}

AddKeyword(SyntaxKind.RefKeyword);
AddSpace();

Expand All @@ -200,6 +207,13 @@ public override void VisitLocal(ILocalSymbol symbol)
}
}

if (symbol.IsValueScoped &&
format.LocalOptions.IncludesOption(SymbolDisplayLocalOptions.IncludeScoped))
{
AddKeyword(SyntaxKind.ScopedKeyword);
AddSpace();
}

if (format.LocalOptions.IncludesOption(SymbolDisplayLocalOptions.IncludeType))
{
symbol.Type.Accept(this.NotFirstVisitor);
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/DeclarationScope.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
// PROTOTYPE: Remove and use separate IsRefScoped and IsValueScoped
// properties on LocalSymbol and ParameterSymbol, to match the public API.
[Flags]
internal enum DeclarationScope : byte
Copy link
Contributor

@RikkiGibson RikkiGibson May 21, 2022

Choose a reason for hiding this comment

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

Have we considered other names for this? e.g. 'EscapeScope', 'ReferenceLifetime', ...

It might be confusing that this refers to the "lifetime of variables referenced by this variable", not to the lifetime of this variable itself. Unifying the terms we use in the LifetimeAnnotationAttribute and in this type might be good. #Resolved

Copy link
Member Author

@cston cston May 22, 2022

Choose a reason for hiding this comment

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

I think it would be good to align the terms used for this type and the corresponding attribute.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated the PROTOTYPE comment here for now.

{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ bool ILocalSymbol.IsFunctionValue

RefKind ILocalSymbol.RefKind => _underlying.RefKind;

bool ILocalSymbol.IsRefScoped => (_underlying.Scope & DeclarationScope.RefScoped) != 0;

bool ILocalSymbol.IsValueScoped => (_underlying.Scope & DeclarationScope.ValueScoped) != 0;

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,10 @@ IParameterSymbol IParameterSymbol.OriginalDefinition

RefKind IParameterSymbol.RefKind => _underlying.RefKind;

bool IParameterSymbol.IsRefScoped => (_underlying.Scope & DeclarationScope.RefScoped) != 0;

bool IParameterSymbol.IsValueScoped => (_underlying.Scope & DeclarationScope.ValueScoped) != 0;

bool IParameterSymbol.IsDiscard => _underlying.IsDiscard;

bool IParameterSymbol.IsParams => _underlying.IsParams;
Expand Down
31 changes: 27 additions & 4 deletions src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4501,11 +4501,25 @@ static void Main()
VerifyParameterSymbol(method.Parameters[1], "scoped in System.Int32 y", RefKind.In, DeclarationScope.RefScoped);
}

private static readonly SymbolDisplayFormat displayFormatWithScoped = SymbolDisplayFormat.TestFormat.
AddParameterOptions(SymbolDisplayParameterOptions.IncludeScoped).
AddLocalOptions(SymbolDisplayLocalOptions.IncludeRef | SymbolDisplayLocalOptions.IncludeScoped);

private static void VerifyParameterSymbol(ParameterSymbol parameter, string expectedDisplayString, RefKind expectedRefKind, DeclarationScope scope)
{
Assert.Equal(expectedRefKind, parameter.RefKind);
Assert.Equal(scope, parameter.Scope);
Assert.Equal(expectedDisplayString.Replace("scoped ", ""), parameter.ToTestDisplayString()); // PROTOTYPE: Remove string.Replace().
Assert.Equal(expectedDisplayString, parameter.ToDisplayString(displayFormatWithScoped));

VerifyParameterSymbol(parameter.GetPublicSymbol(), expectedDisplayString, expectedRefKind, scope);
}

private static void VerifyParameterSymbol(IParameterSymbol parameter, string expectedDisplayString, RefKind expectedRefKind, DeclarationScope scope)
{
Assert.Equal(expectedRefKind, parameter.RefKind);
Assert.Equal((scope & DeclarationScope.RefScoped) != 0, parameter.IsRefScoped);
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
Assert.Equal((scope & DeclarationScope.ValueScoped) != 0, parameter.IsValueScoped);
Assert.Equal(expectedDisplayString, parameter.ToDisplayString(displayFormatWithScoped));
}

[Fact]
Expand Down Expand Up @@ -4630,15 +4644,24 @@ public void LocalScope_04()
var decls = tree.GetRoot().DescendantNodes().OfType<VariableDeclaratorSyntax>().ToArray();
var locals = decls.Select(d => model.GetDeclaredSymbol(d).GetSymbol<LocalSymbol>()).ToArray();

VerifyLocalSymbol(locals[0], "scoped s1", RefKind.None, DeclarationScope.None);
VerifyLocalSymbol(locals[0], "System.Boolean scoped", RefKind.None, DeclarationScope.None);
}

private static void VerifyLocalSymbol(LocalSymbol local, string expectedDisplayString, RefKind expectedRefKind, DeclarationScope scope)
{
Assert.Equal(expectedRefKind, local.RefKind);
Assert.Equal(scope, local.Scope);
// PROTOTYPE: Enable after adding 'scoped' to SymbolDisplay.
//Assert.Equal(expectedDisplayString.Replace("scoped ", ""), local.ToTestDisplayString());
Assert.Equal(expectedDisplayString, local.ToDisplayString(displayFormatWithScoped));

VerifyLocalSymbol(local.GetPublicSymbol(), expectedDisplayString, expectedRefKind, scope);
}

private static void VerifyLocalSymbol(ILocalSymbol local, string expectedDisplayString, RefKind expectedRefKind, DeclarationScope scope)
{
Assert.Equal(expectedRefKind, local.RefKind);
Assert.Equal((scope & DeclarationScope.RefScoped) != 0, local.IsRefScoped);
Assert.Equal((scope & DeclarationScope.ValueScoped) != 0, local.IsValueScoped);
Assert.Equal(expectedDisplayString, local.ToDisplayString(displayFormatWithScoped));
}

[Fact]
Expand Down
158 changes: 158 additions & 0 deletions src/Compilers/CSharp/Test/Symbol/SymbolDisplay/SymbolDisplayTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8034,5 +8034,163 @@ ref struct S<T>
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.FieldName);
}

[Fact]
public void ScopedParameter()
{
var source =
@"ref struct R { }
class Program
{
static void F(scoped R r1, in scoped R r2, scoped ref R r3) { }
}";

var comp = CreateCompilation(source);
comp.VerifyDiagnostics();
var method = comp.GetMember<MethodSymbol>("Program.F");

var formatWithoutScoped = SymbolDisplayFormat.TestFormat;
var formatWithScoped = formatWithoutScoped.AddParameterOptions(SymbolDisplayParameterOptions.IncludeScoped);

Verify(method.ToDisplayParts(formatWithoutScoped),
"void Program.F(R r1, in R r2, ref R r3)",
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.ClassName,
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.MethodName,
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.StructName,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.ParameterName,
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.StructName,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.ParameterName,
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.StructName,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.ParameterName,
SymbolDisplayPartKind.Punctuation);

Verify(method.ToDisplayParts(formatWithScoped),
"void Program.F(scoped R r1, in scoped R r2, scoped ref R r3)",
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.ClassName,
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.MethodName,
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.StructName,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.ParameterName,
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.StructName,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.ParameterName,
SymbolDisplayPartKind.Punctuation,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.StructName,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.ParameterName,
SymbolDisplayPartKind.Punctuation);
}

[Fact]
public void ScopedLocal()
{
var source =
@"ref struct R { }
class Program
{
static void Main()
{
scoped R r1;
ref readonly scoped R r2 = ref r1;
scoped ref R r3 = ref r1;
}
}";

var comp = CreateCompilation(source);
comp.VerifyDiagnostics();
var tree = comp.SyntaxTrees[0];
var model = comp.GetSemanticModel(tree);
var decls = tree.GetRoot().DescendantNodes().OfType<VariableDeclaratorSyntax>().ToArray();
var locals = decls.Select(d => model.GetDeclaredSymbol(d)).ToArray();

var formatWithoutScoped = SymbolDisplayFormat.TestFormat.AddLocalOptions(SymbolDisplayLocalOptions.IncludeRef);
var formatWithScoped = formatWithoutScoped.AddLocalOptions(SymbolDisplayLocalOptions.IncludeScoped);

Verify(locals[0].ToDisplayParts(formatWithoutScoped),
"R r1",
SymbolDisplayPartKind.StructName,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.LocalName);

Verify(locals[0].ToDisplayParts(formatWithScoped),
"scoped R r1",
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.StructName,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.LocalName);

Verify(locals[1].ToDisplayParts(formatWithoutScoped),
"ref readonly R r2",
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.StructName,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.LocalName);

Verify(locals[1].ToDisplayParts(formatWithScoped),
"ref readonly scoped R r2",
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.StructName,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.LocalName);

Verify(locals[2].ToDisplayParts(formatWithoutScoped),
"ref R r3",
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.StructName,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.LocalName);

Verify(locals[2].ToDisplayParts(formatWithScoped),
"scoped ref R r3",
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.Keyword,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.StructName,
SymbolDisplayPartKind.Space,
SymbolDisplayPartKind.LocalName);
}
}
}
7 changes: 6 additions & 1 deletion src/Compilers/Core/Portable/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Microsoft.CodeAnalysis.IImportScope.Aliases.get -> System.Collections.Immutable.
Microsoft.CodeAnalysis.IImportScope.ExternAliases.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.IAliasSymbol!>
Microsoft.CodeAnalysis.IImportScope.Imports.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.ImportedNamespaceOrType>
Microsoft.CodeAnalysis.IImportScope.XmlNamespaces.get -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.ImportedXmlNamespace>
Microsoft.CodeAnalysis.ILocalSymbol.IsRefScoped.get -> bool
Microsoft.CodeAnalysis.ILocalSymbol.IsValueScoped.get -> bool
Microsoft.CodeAnalysis.ImportedNamespaceOrType
Microsoft.CodeAnalysis.ImportedNamespaceOrType.DeclaringSyntaxReference.get -> Microsoft.CodeAnalysis.SyntaxReference?
Microsoft.CodeAnalysis.ImportedNamespaceOrType.ImportedNamespaceOrType() -> void
Expand Down Expand Up @@ -41,8 +43,12 @@ Microsoft.CodeAnalysis.IOperation.OperationList.Reversed.Reversed() -> void
Microsoft.CodeAnalysis.IOperation.OperationList.Reversed.Count.get -> int
Microsoft.CodeAnalysis.IOperation.OperationList.Reversed.GetEnumerator() -> Microsoft.CodeAnalysis.IOperation.OperationList.Reversed.Enumerator
Microsoft.CodeAnalysis.IOperation.OperationList.Reversed.ToImmutableArray() -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.IOperation!>
Microsoft.CodeAnalysis.IParameterSymbol.IsRefScoped.get -> bool
Microsoft.CodeAnalysis.IParameterSymbol.IsValueScoped.get -> bool
Microsoft.CodeAnalysis.SemanticModel.GetImportScopes(int position, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.IImportScope!>
Microsoft.CodeAnalysis.ISymbol.Accept<TArgument, TResult>(Microsoft.CodeAnalysis.SymbolVisitor<TArgument, TResult>! visitor, TArgument argument) -> TResult
Microsoft.CodeAnalysis.SymbolDisplayLocalOptions.IncludeScoped = 8 -> Microsoft.CodeAnalysis.SymbolDisplayLocalOptions
Microsoft.CodeAnalysis.SymbolDisplayParameterOptions.IncludeScoped = 64 -> Microsoft.CodeAnalysis.SymbolDisplayParameterOptions
Microsoft.CodeAnalysis.SymbolVisitor<TArgument, TResult>
Microsoft.CodeAnalysis.SymbolVisitor<TArgument, TResult>.SymbolVisitor() -> void
override sealed Microsoft.CodeAnalysis.Diagnostic.Equals(object? obj) -> bool
Expand Down Expand Up @@ -82,5 +88,4 @@ virtual Microsoft.CodeAnalysis.SymbolVisitor<TArgument, TResult>.VisitPointerTyp
virtual Microsoft.CodeAnalysis.SymbolVisitor<TArgument, TResult>.VisitProperty(Microsoft.CodeAnalysis.IPropertySymbol! symbol, TArgument argument) -> TResult
virtual Microsoft.CodeAnalysis.SymbolVisitor<TArgument, TResult>.VisitRangeVariable(Microsoft.CodeAnalysis.IRangeVariableSymbol! symbol, TArgument argument) -> TResult
virtual Microsoft.CodeAnalysis.SymbolVisitor<TArgument, TResult>.VisitTypeParameter(Microsoft.CodeAnalysis.ITypeParameterSymbol! symbol, TArgument argument) -> TResult

Microsoft.CodeAnalysis.Operations.BinaryOperatorKind.UnsignedRightShift = 25 -> Microsoft.CodeAnalysis.Operations.BinaryOperatorKind
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,10 @@ public enum SymbolDisplayLocalOptions
/// Includes the <c>ref</c> keyword for ref-locals.
/// </summary>
IncludeRef = 1 << 2,

/// <summary>
/// Includes the <c>scoped</c> keyword.
/// </summary>
IncludeScoped = 1 << 3,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,10 @@ public enum SymbolDisplayParameterOptions
/// Includes square brackets around optional parameters.
/// </summary>
IncludeOptionalBrackets = 1 << 5,

/// <summary>
/// Includes the <c>scoped</c> keyword.
/// </summary>
IncludeScoped = 1 << 6,
}
}
10 changes: 10 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,16 @@ public interface ILocalSymbol : ISymbol
/// </summary>
RefKind RefKind { get; }

/// <summary>
/// Returns true if the ref local is scoped to the current method.
/// </summary>
bool IsRefScoped { get; }

/// <summary>
/// Returns true if the local value is scoped to the current method.
/// </summary>
bool IsValueScoped { get; }

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

/// <summary>
/// Returns true if the ref parameter is scoped to the current method.
/// </summary>
bool IsRefScoped { get; }

/// <summary>
/// Returns true if the parameter value is scoped to the current method.
/// </summary>
bool IsValueScoped { get; }

/// <summary>
/// Gets the ordinal position of the parameter. The first parameter has ordinal zero.
/// The 'this' parameter ('Me' in Visual Basic) has ordinal -1.
Expand Down
Loading