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 23 commits
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 @@ -48,6 +48,7 @@ public override bool HasExplicitReturnType(out RefKind refKind, out TypeWithAnno
public override bool IsAsync { get { return false; } }
public override bool IsStatic => false;
public override RefKind RefKind(int index) { return Microsoft.CodeAnalysis.RefKind.None; }
public override DeclarationScope Scope(int index) => DeclarationScope.Unscoped;
public override MessageID MessageID { get { return MessageID.IDS_FeatureQueryExpression; } } // TODO: what is the correct ID here?
public override Location ParameterLocation(int index) { return _parameters[index].Locations[0]; }
public override TypeWithAnnotations ParameterTypeWithAnnotations(int index) { throw new ArgumentException(); } // implicitly typed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2814,6 +2814,7 @@ private BoundExpression BindOutVariableDeclarationArgument(
CheckFeatureAvailability(declarationExpression, MessageID.IDS_FeatureExpressionVariablesInQueriesAndInitializers, diagnostics);
}

// PROTOTYPE: Test with 'out scoped R' and 'out scoped var', with -langversion:10 and -langversion:11.
bool isConst = false;
AliasSymbol alias;
var declType = BindVariableTypeWithAnnotations(declarationExpression, diagnostics, typeSyntax, ref isConst, out isVar, out alias);
Expand Down Expand Up @@ -7105,7 +7106,7 @@ protected BoundExpression BindFieldAccess(
// If this is a ref field from another compilation, check for support for ref fields.
// No need to check for a reference to a field declared in this compilation since
// we check at the declaration site. (Check RefKind after checking compilation to
// avoid cycles for source symbols.
// avoid cycles for source symbols.)
if ((object)Compilation.SourceModule != fieldSymbol.OriginalDefinition.ContainingModule &&
fieldSymbol.RefKind != RefKind.None)
{
Expand Down
53 changes: 44 additions & 9 deletions src/Compilers/CSharp/Portable/Binder/Binder_Lambda.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ private UnboundLambda AnalyzeAnonymousFunction(

ImmutableArray<string> names = default;
ImmutableArray<RefKind> refKinds = default;
ImmutableArray<DeclarationScope> scopes = default;
ImmutableArray<bool> nullCheckedOpt = default;
ImmutableArray<TypeWithAnnotations> types = default;
RefKind returnRefKind = RefKind.None;
Expand Down Expand Up @@ -99,10 +100,10 @@ private UnboundLambda AnalyzeAnonymousFunction(
if (parameterSyntaxList != null)
{
var hasExplicitlyTypedParameterList = true;
var allValue = true;

var typesBuilder = ArrayBuilder<TypeWithAnnotations>.GetInstance();
var refKindsBuilder = ArrayBuilder<RefKind>.GetInstance();
var scopesBuilder = ArrayBuilder<DeclarationScope>.GetInstance();
var nullCheckedBuilder = ArrayBuilder<bool>.GetInstance();
var attributesBuilder = ArrayBuilder<SyntaxList<AttributeListSyntax>>.GetInstance();

Expand Down Expand Up @@ -138,31 +139,31 @@ private UnboundLambda AnalyzeAnonymousFunction(
var typeSyntax = p.Type;
TypeWithAnnotations type = default;
var refKind = RefKind.None;
var scope = DeclarationScope.Unscoped;

if (typeSyntax == null)
{
hasExplicitlyTypedParameterList = false;
}
else
{
bool scopedBeforeRef = false;
bool scopedAfterRef = false;
type = BindType(typeSyntax, diagnostics);
foreach (var modifier in p.Modifiers)
{
switch (modifier.Kind())
{
case SyntaxKind.RefKeyword:
refKind = RefKind.Ref;
allValue = false;
break;

case SyntaxKind.OutKeyword:
refKind = RefKind.Out;
allValue = false;
break;

case SyntaxKind.InKeyword:
refKind = RefKind.In;
allValue = false;
break;

case SyntaxKind.ParamsKeyword:
Expand All @@ -175,13 +176,34 @@ private UnboundLambda AnalyzeAnonymousFunction(
case SyntaxKind.ThisKeyword:
Error(diagnostics, ErrorCode.ERR_ThisInBadContext, modifier);
break;

case SyntaxKind.ScopedKeyword:
ModifierUtils.CheckScopedModifierAvailability(p, modifier, diagnostics);
if (refKind == RefKind.None)
{
scopedBeforeRef = true;
}
else
{
scopedAfterRef = true;
}
break;
}
}
if (scopedAfterRef)
{
scope = DeclarationScope.ValueScoped;
}
else if (scopedBeforeRef)
{
scope = (refKind == RefKind.None) ? DeclarationScope.ValueScoped : DeclarationScope.RefScoped;
}
}

namesBuilder.Add(p.Identifier.ValueText);
typesBuilder.Add(type);
refKindsBuilder.Add(refKind);
scopesBuilder.Add(scope);
nullCheckedBuilder.Add(isNullChecked(p));
attributesBuilder.Add(syntax.Kind() == SyntaxKind.ParenthesizedLambdaExpression ? p.AttributeLists : default);
}
Expand All @@ -193,11 +215,16 @@ private UnboundLambda AnalyzeAnonymousFunction(
types = typesBuilder.ToImmutable();
}

if (!allValue)
if (refKindsBuilder.Any(r => r != RefKind.None))
{
refKinds = refKindsBuilder.ToImmutable();
}

if (scopesBuilder.Any(s => s != DeclarationScope.Unscoped))
{
scopes = scopesBuilder.ToImmutable();
}

if (nullCheckedBuilder.Contains(true))
{
nullCheckedOpt = nullCheckedBuilder.ToImmutable();
Expand All @@ -209,6 +236,7 @@ private UnboundLambda AnalyzeAnonymousFunction(
}

typesBuilder.Free();
scopesBuilder.Free();
refKindsBuilder.Free();
nullCheckedBuilder.Free();
attributesBuilder.Free();
Expand All @@ -221,7 +249,7 @@ private UnboundLambda AnalyzeAnonymousFunction(

namesBuilder.Free();

return UnboundLambda.Create(syntax, this, diagnostics.AccumulatesDependencies, returnRefKind, returnType, parameterAttributes, refKinds, types, names, discardsOpt, nullCheckedOpt, isAsync, isStatic);
return UnboundLambda.Create(syntax, this, diagnostics.AccumulatesDependencies, returnRefKind, returnType, parameterAttributes, refKinds, scopes, types, names, discardsOpt, nullCheckedOpt, isAsync, isStatic);

static bool isNullChecked(ParameterSyntax parameter)
=> parameter.ExclamationExclamationToken.IsKind(SyntaxKind.ExclamationExclamationToken);
Expand Down Expand Up @@ -322,10 +350,17 @@ private UnboundLambda BindAnonymousFunction(AnonymousFunctionExpressionSyntax sy
for (int i = 0; i < lambda.ParameterCount; i++)
{
// UNDONE: Where do we report improper use of pointer types?
var type = lambda.Data.ParameterTypeWithAnnotations(i);
if (type.HasType && type.IsStatic)
var type = data.ParameterTypeWithAnnotations(i).Type;
if (type is { })
{
Error(diagnostics, ErrorFacts.GetStaticClassParameterCode(useWarning: false), syntax, type.Type);
if (type.IsStatic)
{
Error(diagnostics, ErrorFacts.GetStaticClassParameterCode(useWarning: false), syntax, type);
}
if (data.Scope(i) == DeclarationScope.ValueScoped && !type.IsValidScopedType())
{
diagnostics.Add(ErrorCode.ERR_ScopedRefAndRefStructOnly, data.ParameterLocation(i));
}
}
}
}
Expand Down
21 changes: 21 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,22 @@ private BoundStatement BindDeclarationStatementParts(LocalDeclarationStatementSy
var typeSyntax = node.Declaration.Type.SkipRef(out _);
bool isConst = node.IsConst;

foreach (var modifier in node.Modifiers)
{
// Check for support for 'scoped'. Duplicate modifiers are reported
// as errors in parsing rather than here.
if (modifier.Kind() == SyntaxKind.ScopedKeyword)
{
ModifierUtils.CheckScopedModifierAvailability(node, modifier, diagnostics);
}
}

if (node.Declaration.Type is RefTypeSyntax { ScopedKeyword: var scopedKeyword } &&
scopedKeyword.Kind() == SyntaxKind.ScopedKeyword)
{
ModifierUtils.CheckScopedModifierAvailability(typeSyntax, scopedKeyword, diagnostics);
}

bool isVar;
AliasSymbol alias;
TypeWithAnnotations declType = BindVariableTypeWithAnnotations(node.Declaration, diagnostics, typeSyntax, ref isConst, isVar: out isVar, alias: out alias);
Expand Down Expand Up @@ -1078,6 +1094,11 @@ protected BoundLocalDeclaration BindVariableDeclaration(
hasErrors = true;
}

if (localSymbol.Scope == DeclarationScope.ValueScoped && !declTypeOpt.Type.IsValidScopedType())
{
localDiagnostics.Add(ErrorCode.ERR_ScopedRefAndRefStructOnly, typeSyntax.Location);
}

localSymbol.SetTypeWithAnnotations(declTypeOpt);

if (initializerOpt != null)
Expand Down
8 changes: 5 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/LocalScopeBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,10 @@ internal void BuildLocals(Binder enclosingBinder, StatementSyntax statement, Arr
{
kind = LocalDeclarationKind.RegularVariable;
}
bool scopedModifier = decl.Modifiers.Any(SyntaxKind.ScopedKeyword);
foreach (var vdecl in decl.Declaration.Variables)
{
var localSymbol = MakeLocal(decl.Declaration, vdecl, kind, localDeclarationBinder);
var localSymbol = MakeLocal(decl.Declaration, vdecl, kind, localDeclarationBinder, scopedModifier);
locals.Add(localSymbol);

// also gather expression-declared variables from the bracketed argument lists and the initializers
Expand Down Expand Up @@ -313,7 +314,7 @@ internal void BuildLocalFunctions(StatementSyntax statement, ref ArrayBuilder<Lo
}
}

protected SourceLocalSymbol MakeLocal(VariableDeclarationSyntax declaration, VariableDeclaratorSyntax declarator, LocalDeclarationKind kind, Binder initializerBinderOpt = null)
protected SourceLocalSymbol MakeLocal(VariableDeclarationSyntax declaration, VariableDeclaratorSyntax declarator, LocalDeclarationKind kind, Binder initializerBinderOpt = null, bool scopedModifier = false)
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 2, 2022

Choose a reason for hiding this comment

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

, bool scopedModifier = false

Were all call sites examined incase an explicit value should be passed? #Closed

{
return SourceLocalSymbol.MakeLocal(
this.ContainingMemberOrLambda,
Expand All @@ -323,7 +324,8 @@ protected SourceLocalSymbol MakeLocal(VariableDeclarationSyntax declaration, Var
declarator.Identifier,
kind,
declarator.Initializer,
initializerBinderOpt);
initializerBinderOpt,
scopedModifier);
}

protected LocalFunctionSymbol MakeLocalFunction(LocalFunctionStatementSyntax declaration)
Expand Down
18 changes: 15 additions & 3 deletions src/Compilers/CSharp/Portable/BoundTree/UnboundLambda.cs
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ public static UnboundLambda Create(
TypeWithAnnotations returnType,
ImmutableArray<SyntaxList<AttributeListSyntax>> parameterAttributes,
ImmutableArray<RefKind> refKinds,
ImmutableArray<DeclarationScope> scopes,
ImmutableArray<TypeWithAnnotations> types,
ImmutableArray<string> names,
ImmutableArray<bool> discardsOpt,
Expand All @@ -392,7 +393,7 @@ public static UnboundLambda Create(
bool hasErrors = !types.IsDefault && types.Any(t => t.Type?.Kind == SymbolKind.ErrorType);

var functionType = FunctionTypeSymbol.CreateIfFeatureEnabled(syntax, binder, static (binder, expr) => ((UnboundLambda)expr).Data.InferDelegateType());
var data = new PlainUnboundLambdaState(binder, returnRefKind, returnType, parameterAttributes, names, discardsOpt, nullCheckedOpt, types, refKinds, isAsync, isStatic, includeCache: true);
var data = new PlainUnboundLambdaState(binder, returnRefKind, returnType, parameterAttributes, names, discardsOpt, nullCheckedOpt, types, refKinds, scopes, isAsync, isStatic, includeCache: true);
var lambda = new UnboundLambda(syntax, data, functionType, withDependencies, hasErrors: hasErrors);
data.SetUnboundLambda(lambda);
functionType?.SetExpression(lambda.WithNoCache());
Expand Down Expand Up @@ -449,6 +450,7 @@ public TypeWithAnnotations InferReturnType(ConversionsBase conversions, NamedTyp
=> BindForReturnTypeInference(delegateType).GetInferredReturnType(conversions, _nullableState, ref useSiteInfo, out inferredFromFunctionType);

public RefKind RefKind(int index) { return Data.RefKind(index); }
public DeclarationScope Scope(int index) { return Data.Scope(index); }
public void GenerateAnonymousFunctionConversionError(BindingDiagnosticBag diagnostics, TypeSymbol targetType) { Data.GenerateAnonymousFunctionConversionError(diagnostics, targetType); }
public bool GenerateSummaryErrors(BindingDiagnosticBag diagnostics) { return Data.GenerateSummaryErrors(diagnostics); }
public bool IsAsync { get { return Data.IsAsync; } }
Expand Down Expand Up @@ -530,6 +532,7 @@ internal UnboundLambdaState WithCaching(bool includeCache)
public abstract Location ParameterLocation(int index);
public abstract TypeWithAnnotations ParameterTypeWithAnnotations(int index);
public abstract RefKind RefKind(int index);
public abstract DeclarationScope Scope(int index);
protected abstract BoundBlock BindLambdaBody(LambdaSymbol lambdaSymbol, Binder lambdaBodyBinder, BindingDiagnosticBag diagnostics);

/// <summary>
Expand Down Expand Up @@ -734,6 +737,7 @@ private BoundLambda ReallyBind(NamedTypeSymbol delegateType, bool inExpressionTr
}

ParameterHelpers.EnsureNativeIntegerAttributeExists(compilation, lambdaParameters, diagnostics, modifyCompilation: false);
ParameterHelpers.EnsureLifetimeAnnotationAttributeExists(compilation, lambdaParameters, diagnostics, modifyCompilation: false);
ParameterHelpers.EnsureNullableAttributeExists(compilation, lambdaSymbol, lambdaParameters, diagnostics, modifyCompilation: false);
// Note: we don't need to warn on annotations used in #nullable disable context for lambdas, as this is handled in binding already

Expand Down Expand Up @@ -1326,6 +1330,7 @@ internal sealed class PlainUnboundLambdaState : UnboundLambdaState
private readonly ImmutableArray<bool> _parameterIsNullCheckedOpt;
private readonly ImmutableArray<TypeWithAnnotations> _parameterTypesWithAnnotations;
private readonly ImmutableArray<RefKind> _parameterRefKinds;
private readonly ImmutableArray<DeclarationScope> _parameterScopes;
private readonly bool _isAsync;
private readonly bool _isStatic;

Expand All @@ -1339,6 +1344,7 @@ internal PlainUnboundLambdaState(
ImmutableArray<bool> parameterIsNullCheckedOpt,
ImmutableArray<TypeWithAnnotations> parameterTypesWithAnnotations,
ImmutableArray<RefKind> parameterRefKinds,
ImmutableArray<DeclarationScope> parameterScopes,
bool isAsync,
bool isStatic,
bool includeCache)
Expand All @@ -1352,11 +1358,11 @@ internal PlainUnboundLambdaState(
_parameterIsNullCheckedOpt = parameterIsNullCheckedOpt;
_parameterTypesWithAnnotations = parameterTypesWithAnnotations;
_parameterRefKinds = parameterRefKinds;
_parameterScopes = parameterScopes;
_isAsync = isAsync;
_isStatic = isStatic;
}


public override bool HasSignature { get { return !_parameterNames.IsDefault; } }

public override bool HasExplicitReturnType(out RefKind refKind, out TypeWithAnnotations returnType)
Expand Down Expand Up @@ -1429,6 +1435,12 @@ public override RefKind RefKind(int index)
return _parameterRefKinds.IsDefault ? Microsoft.CodeAnalysis.RefKind.None : _parameterRefKinds[index];
}

public override DeclarationScope Scope(int index)
{
Debug.Assert(0 <= index && index < _parameterTypesWithAnnotations.Length);
return _parameterScopes.IsDefault ? DeclarationScope.Unscoped : _parameterScopes[index];
}

public override TypeWithAnnotations ParameterTypeWithAnnotations(int index)
{
Debug.Assert(this.HasExplicitlyTypedParameterList);
Expand All @@ -1438,7 +1450,7 @@ public override TypeWithAnnotations ParameterTypeWithAnnotations(int index)

protected override UnboundLambdaState WithCachingCore(bool includeCache)
{
return new PlainUnboundLambdaState(Binder, _returnRefKind, _returnType, _parameterAttributes, _parameterNames, _parameterIsDiscardOpt, _parameterIsNullCheckedOpt, _parameterTypesWithAnnotations, _parameterRefKinds, _isAsync, _isStatic, includeCache);
return new PlainUnboundLambdaState(Binder, _returnRefKind, _returnType, _parameterAttributes, _parameterNames, _parameterIsDiscardOpt, _parameterIsNullCheckedOpt, _parameterTypesWithAnnotations, _parameterRefKinds, _parameterScopes, _isAsync, _isStatic, includeCache);
}

protected override BoundExpression? GetLambdaExpressionBody(BoundBlock body)
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -6775,6 +6775,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_ListPatternRequiresLength" xml:space="preserve">
<value>List patterns may not be used for a value of type '{0}'. No suitable 'Length' or 'Count' property was found.</value>
</data>
<data name="ERR_ScopedRefAndRefStructOnly" xml:space="preserve">
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.

I am curious about the placement of this message. Any specific reason to put it at this location rather than, say, at the end? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

ERR_ScopedRefAndRefStructOnly follows ERR_ListPatternRequiresLength in enum ErrorCode.

<value>The 'scoped' modifier can be used for refs and ref struct values only.</value>
</data>

<data name="WRN_UseDefViolationPropertySupportedVersion" xml:space="preserve">
<value>Auto-implemented property '{0}' is read before being explicitly assigned, causing a preceding implicit assignment of 'default'.</value>
Expand Down
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/CodeGen/Optimizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2267,5 +2267,7 @@ public override RefKind RefKind
/// Checking escape scopes is not valid here.
/// </summary>
internal override uint RefEscapeScope => throw ExceptionUtilities.Unreachable;

internal override DeclarationScope Scope => DeclarationScope.Unscoped;
}
}
Loading