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

Address PROTOTYPE comments in features/ref-fields #62088

Merged
merged 13 commits into from
Jun 27, 2022
65 changes: 65 additions & 0 deletions docs/compilers/CSharp/Compiler Breaking Changes - DotNet 7.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,71 @@ Note: The break will also apply to C# 10 and earlier when .NET 7 ships, but is
currently scoped down to users of LangVer=preview.
Tracked by https://github.com/dotnet/roslyn/issues/60640

## Cannot return an out parameter by reference

***Introduced in .NET SDK 7.0.100, Visual Studio 2022 version 17.3.***

With language version C# 11 or later, or with .NET 7.0 or later, an `out` parameter cannot be returned by reference.

```csharp
static ref T ReturnOutParamByRef<T>(out T t)
{
t = default;
return ref t; // error CS8166: Cannot return a parameter by reference 't' because it is not a ref parameter
}
```

A possible workaround is to change the method signature to pass the parameter by `ref` instead.

```csharp
static ref T ReturnRefParamByRef<T>(ref T t)
{
t = default;
return ref t; // ok
}
```

## Method ref struct return escape analysis depends on ref escape of ref arguments
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved

***Introduced in .NET SDK 7.0.100, Visual Studio 2022 version 17.3.***

With language version C# 11 or later, or with .NET 7.0 or later, the return value of a method invocation that returns a `ref struct` is only _safe-to-escape_ if all the `ref` and `in` arguments to the method invocation are _ref-safe-to-escape_. _The `in` arguments may include implicit default parameter values._

```csharp
ref struct R { }

static R MayCaptureArg(ref int i) => new R();

static R MayCaptureDefaultArg(in int i = 0) => new R();

static R Create()
{
int i = 0;
// error CS8347: Cannot use a result of 'MayCaptureArg(ref int)' because it may expose
// variables referenced by parameter 'i' outside of their declaration scope
return MayCaptureArg(ref i);
}

static R CreateDefault()
{
// error CS8347: Cannot use a result of 'MayCaptureDefaultArg(in int)' because it may expose
// variables referenced by parameter 'i' outside of their declaration scope
return MayCaptureDefaultArg();
}
```

A possible workaround, if the `ref` or `in` argument is not captured in the `ref struct` return value, is to declare the parameter as `scoped ref` or `scoped in`.

```csharp
static R CannotCaptureArg(scoped ref int i) => new R();

static R Create()
{
int i = 0;
return CannotCaptureArg(ref i); // ok
}
```

## Unsigned right shift operator

***Introduced in .NET SDK 6.0.400, Visual Studio 2022 version 17.3.***
Expand Down
19 changes: 9 additions & 10 deletions src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ private static bool CheckLocalRefEscape(SyntaxNode node, BoundLocal local, uint
return false;
}

Error(diagnostics, ErrorCode.ERR_EscapeLocal, node, localSymbol);
Error(diagnostics, ErrorCode.ERR_EscapeVariable, node, localSymbol);
return false;
}

Expand Down Expand Up @@ -813,8 +813,7 @@ private bool CheckParameterValEscape(SyntaxNode node, BoundParameter parameter,
var parameterSymbol = parameter.ParameterSymbol;
if (GetParameterValEscape(parameterSymbol) > escapeTo)
{
// PROTOTYPE: Add specific error.
Error(diagnostics, ErrorCode.ERR_EscapeLocal, node, parameterSymbol);
Error(diagnostics, ErrorCode.ERR_EscapeVariable, node, parameterSymbol);
return false;
}
return true;
Expand Down Expand Up @@ -3232,23 +3231,23 @@ internal bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint escapeF
case BoundKind.DeconstructValuePlaceholder:
if (((BoundDeconstructValuePlaceholder)expr).ValEscape > escapeTo)
{
Error(diagnostics, ErrorCode.ERR_EscapeLocal, node, expr.Syntax);
Error(diagnostics, ErrorCode.ERR_EscapeVariable, node, expr.Syntax);
return false;
}
return true;

case BoundKind.AwaitableValuePlaceholder:
if (((BoundAwaitableValuePlaceholder)expr).ValEscape > escapeTo)
{
Error(diagnostics, ErrorCode.ERR_EscapeLocal, node, expr.Syntax);
Error(diagnostics, ErrorCode.ERR_EscapeVariable, node, expr.Syntax);
return false;
}
return true;

case BoundKind.InterpolatedStringArgumentPlaceholder:
if (((BoundInterpolatedStringArgumentPlaceholder)expr).ValSafeToEscape > escapeTo)
{
Error(diagnostics, ErrorCode.ERR_EscapeLocal, node, expr.Syntax);
Error(diagnostics, ErrorCode.ERR_EscapeVariable, node, expr.Syntax);
return false;
}
return true;
Expand All @@ -3257,7 +3256,7 @@ internal bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint escapeF
var localSymbol = ((BoundLocal)expr).LocalSymbol;
if (localSymbol.ValEscapeScope > escapeTo)
{
Error(diagnostics, ErrorCode.ERR_EscapeLocal, node, localSymbol);
Error(diagnostics, ErrorCode.ERR_EscapeVariable, node, localSymbol);
return false;
}
return true;
Expand Down Expand Up @@ -3374,23 +3373,23 @@ internal bool CheckValEscape(SyntaxNode node, BoundExpression expr, uint escapeF
case BoundKind.ImplicitIndexerReceiverPlaceholder:
if (((BoundImplicitIndexerReceiverPlaceholder)expr).ValEscape > escapeTo)
{
Error(diagnostics, ErrorCode.ERR_EscapeLocal, node, expr.Syntax);
Error(diagnostics, ErrorCode.ERR_EscapeVariable, node, expr.Syntax);
return false;
}
return true;

case BoundKind.ListPatternReceiverPlaceholder:
if (((BoundListPatternReceiverPlaceholder)expr).ValEscape > escapeTo)
{
Error(diagnostics, ErrorCode.ERR_EscapeLocal, node, expr.Syntax);
Error(diagnostics, ErrorCode.ERR_EscapeVariable, node, expr.Syntax);
return false;
}
return true;

case BoundKind.SlicePatternReceiverPlaceholder:
if (((BoundSlicePatternReceiverPlaceholder)expr).ValEscape > escapeTo)
{
Error(diagnostics, ErrorCode.ERR_EscapeLocal, node, expr.Syntax);
Error(diagnostics, ErrorCode.ERR_EscapeVariable, node, expr.Syntax);
return false;
}
return true;
Expand Down
6 changes: 4 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,10 @@ internal bool CheckOverflowAtCompileTime
}
}

internal bool UseUpdatedEscapeRules => Compilation.IsFeatureEnabled(MessageID.IDS_FeatureRefFields) ||
Compilation.Assembly.RuntimeSupportsByRefFields;
// https://github.com/dotnet/roslyn/issues/62131: Enable updated escape rules if
// System.Runtime.CompilerServices.RuntimeFeature.ByRefFields exists.
internal bool UseUpdatedEscapeRules => Compilation.IsFeatureEnabled(MessageID.IDS_FeatureRefFields) /*||
Compilation.Assembly.RuntimeSupportsByRefFields*/;

/// <summary>
/// Some nodes have special binders for their contents (like Blocks)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2814,7 +2814,6 @@ 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
52 changes: 2 additions & 50 deletions src/Compilers/CSharp/Portable/Binder/Binder_Lambda.cs
Original file line number Diff line number Diff line change
Expand Up @@ -141,57 +141,9 @@ private UnboundLambda AnalyzeAnonymousFunction(
}
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;
break;

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

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

case SyntaxKind.ParamsKeyword:
// This was a parse error in the native compiler;
// it is a semantic analysis error in Roslyn. See comments to
// changeset 1674 for details.
Error(diagnostics, ErrorCode.ERR_IllegalParams, p);
break;

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;
}
ParameterHelpers.CheckParameterModifiers(p, diagnostics, parsingFunctionPointerParams: false, parsingLambdaParams: true);
refKind = ParameterHelpers.GetModifiers(p.Modifiers, out _, out _, out _, out scope);
}

namesBuilder.Add(p.Identifier.ValueText);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ protected override ImmutableArray<LocalSymbol> BuildLocals()

foreach (VariableDeclaratorSyntax declarator in _syntax.Declaration.Variables)
{
locals.Add(MakeLocal(_syntax.Declaration, declarator, LocalDeclarationKind.FixedVariable, hasScopedModifier: false)); // PROTOTYPE: Handle 'scoped' modifier.
locals.Add(MakeLocal(_syntax.Declaration, declarator, LocalDeclarationKind.FixedVariable, hasScopedModifier: false));

// also gather expression-declared variables from the bracketed argument lists and the initializers
ExpressionVariableFinder.FindExpressionVariables(this, locals, declarator);
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/ForLoopBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ protected override ImmutableArray<LocalSymbol> BuildLocals()

foreach (var vdecl in _syntax.Declaration.Variables)
{
var localSymbol = MakeLocal(_syntax.Declaration, vdecl, LocalDeclarationKind.RegularVariable, hasScopedModifier: false); // PROTOTYPE: Handle 'scoped' modifier.
var localSymbol = MakeLocal(_syntax.Declaration, vdecl, LocalDeclarationKind.RegularVariable, hasScopedModifier: false); // https://github.com/dotnet/roslyn/issues/62039: Allow 'scoped' modifier.
locals.Add(localSymbol);

// also gather expression-declared variables from the bracketed argument lists and the initializers
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ protected override ImmutableArray<LocalSymbol> BuildLocals()

foreach (VariableDeclaratorSyntax declarator in declarationSyntax.Variables)
{
locals.Add(MakeLocal(declarationSyntax, declarator, LocalDeclarationKind.UsingVariable, hasScopedModifier: false)); // PROTOTYPE: Handle 'scoped' modifier.
locals.Add(MakeLocal(declarationSyntax, declarator, LocalDeclarationKind.UsingVariable, hasScopedModifier: false));
Copy link
Contributor

@RikkiGibson RikkiGibson Jun 24, 2022

Choose a reason for hiding this comment

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

if a tracking issue exists for these, consider replacing the prototype comment with a link to the issue rather than deleting. #Resolved


// also gather expression-declared variables from the bracketed argument lists and the initializers
ExpressionVariableFinder.FindExpressionVariables(this, locals, declarator);
Expand Down
9 changes: 6 additions & 3 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -5069,7 +5069,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<value>Fields of static readonly field '{0}' cannot be returned by writable reference</value>
</data>
<data name="ERR_RefReturnParameter" xml:space="preserve">
<value>Cannot return a parameter by reference '{0}' because it is not a ref or out parameter</value>
<value>Cannot return a parameter by reference '{0}' because it is not a ref parameter</value>
</data>
<data name="ERR_RefReturnParameter2" xml:space="preserve">
<value>Cannot return by reference a member of parameter '{0}' because it is not a ref or out parameter</value>
Expand All @@ -5086,8 +5086,8 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_EscapeOther" xml:space="preserve">
<value>Expression cannot be used in this context because it may indirectly expose variables outside of their declaration scope</value>
</data>
<data name="ERR_EscapeLocal" xml:space="preserve">
<value>Cannot use local '{0}' in this context because it may expose referenced variables outside of their declaration scope</value>
<data name="ERR_EscapeVariable" xml:space="preserve">
<value>Cannot use variable '{0}' in this context because it may expose referenced variables outside of their declaration scope</value>
</data>
<data name="ERR_EscapeCall" xml:space="preserve">
<value>Cannot use a result of '{0}' in this context because it may expose variables referenced by parameter '{1}' outside of their declaration scope</value>
Expand Down Expand Up @@ -6763,6 +6763,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_ScopedMismatchInParameterOfPartial" xml:space="preserve">
<value>The 'scoped' modifier of parameter '{0}' doesn't match partial method declaration.</value>
</data>
<data name="ERR_FixedFieldMustNotBeRef" xml:space="preserve">
<value>A fixed field must not be a ref field.</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
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1538,7 +1538,7 @@ internal enum ErrorCode
ERR_EscapeOther = 8349,
ERR_CallArgMixing = 8350,
ERR_MismatchedRefEscapeInTernary = 8351,
ERR_EscapeLocal = 8352,
ERR_EscapeVariable = 8352,
ERR_EscapeStackAlloc = 8353,
ERR_RefReturnThis = 8354,
ERR_OutAttrOnInParam = 8355,
Expand Down Expand Up @@ -2095,6 +2095,7 @@ internal enum ErrorCode
ERR_ScriptsAndSubmissionsCannotHaveRequiredMembers = 9045,
ERR_BadAbstractEqualityOperatorSignature = 9046,
ERR_ScopedRefAndRefStructOnly = 9048,
ERR_FixedFieldMustNotBeRef = 9049,

#endregion

Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Symbols/DeclarationScope.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
// PROTOTYPE: Internally, scope is represented with this enum, but the public API uses a
// pair of IsRefScoped and IsValueScoped bools (see ILocalSymbol, IParameterSymbol,
// and LifetimeAnnotationAttribute). We should have a common representation.
// 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 LifetimeAnnotationAttribute). We should have a common representation.
// And we should use common terms for the attribute and enum names.
internal enum DeclarationScope : byte
{
Expand Down
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,6 @@ internal bool CalculateUseSiteDiagnostic(ref UseSiteInfo<AssemblySymbol> result)
Debug.Assert(IsDefinition);

// Check type, custom modifiers
// PROTOTYPE: Disallow volatile for ref fields.
if (DeriveUseSiteInfoFromType(ref result, this.TypeWithAnnotations, AllowedRequiredModifierType.System_Runtime_CompilerServices_Volatile) ||
DeriveUseSiteInfoFromCustomModifiers(ref result, this.RefCustomModifiers, AllowedRequiredModifierType.None))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,6 @@ private void EnsureSignatureIsLoaded()
_packedFlags.SetRefKind(refKind);
_packedFlags.SetIsVolatile(customModifiersArray.Any(static m => !m.IsOptional && ((CSharpCustomModifier)m).ModifierSymbol.SpecialType == SpecialType.System_Runtime_CompilerServices_IsVolatile));

// PROTOTYPE: `fixed ref` field use should be disallowed.
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 24, 2022

Choose a reason for hiding this comment

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

// PROTOTYPE: fixed ref field use should be disallowed.

I didn't see any special handling around that for PE. #Closed

Copy link
Contributor

@AlekseyTs AlekseyTs Jun 24, 2022

Choose a reason for hiding this comment

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

// PROTOTYPE: fixed ref field use should be disallowed.

Are we tracking similar issue for volatile or already handling that? #Closed

TypeSymbol fixedElementType;
int fixedSize;
if (customModifiersArray.IsEmpty && IsFixedBuffer(out fixedSize, out fixedElementType))
Expand Down Expand Up @@ -637,6 +636,10 @@ internal override UseSiteInfo<AssemblySymbol> GetUseSiteInfo()
{
UseSiteInfo<AssemblySymbol> result = new UseSiteInfo<AssemblySymbol>(primaryDependency);
CalculateUseSiteDiagnostic(ref result);
if (IsFixedSizeBuffer && RefKind != RefKind.None)
{
MergeUseSiteInfo(ref result, new UseSiteInfo<AssemblySymbol>(new CSDiagnosticInfo(ErrorCode.ERR_BindToBogus, this)));
}
deriveCompilerFeatureRequiredUseSiteInfo(ref result);
_lazyCachedUseSiteInfo.Initialize(primaryDependency, result);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ private ImmutableArray<ParameterSymbol> MakeParameters(
{
type = parameterTypes[p];
refKind = parameterRefKinds[p];
scope = DeclarationScope.Unscoped; // PROTOTYPE: DeclarationScope should be taken from delegate signature.
scope = DeclarationScope.Unscoped; // https://github.com/dotnet/roslyn/issues/62080: DeclarationScope should be taken from delegate signature.
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ internal void ComputeReturnType()

var diagnostics = BindingDiagnosticBag.GetInstance(_declarationDiagnostics);
TypeSyntax returnTypeSyntax = Syntax.ReturnType;
TypeWithAnnotations returnType = WithTypeParametersBinder.BindType(returnTypeSyntax.SkipRef(), diagnostics);
TypeWithAnnotations returnType = WithTypeParametersBinder.BindType(returnTypeSyntax.SkipRef(out _, allowScoped: false, diagnostics), diagnostics);

var compilation = DeclaringCompilation;

Expand Down
Loading