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

Implement stackalloc initializers #24249

Merged
merged 24 commits into from
Feb 8, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/BinderFlagsExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,10 @@ public static bool Includes(this BinderFlags self, BinderFlags other)
{
return (self & other) == other;
}

public static bool IncludesAny(this BinderFlags self, BinderFlags other)
{
return (self & other) != 0;
}
}
}
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ private BoundExpression CreateStackAllocConversion(SyntaxNode syntax, BoundExpre
throw ExceptionUtilities.UnexpectedValue(conversion.Kind);
}

var convertedNode = new BoundConvertedStackAllocExpression(syntax, elementType, boundStackAlloc.Count, stackAllocType, boundStackAlloc.HasErrors);
var convertedNode = new BoundConvertedStackAllocExpression(syntax, elementType, boundStackAlloc.Count, boundStackAlloc.InitializerOpt, stackAllocType, boundStackAlloc.HasErrors);

var underlyingConversion = conversion.UnderlyingConversions.Single();
return CreateConversion(syntax, convertedNode, underlyingConversion, isCast, destination, diagnostics);
Expand Down
173 changes: 137 additions & 36 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,8 @@ private BoundExpression BindExpressionInternal(ExpressionSyntax node, Diagnostic
return BindImplicitArrayCreationExpression((ImplicitArrayCreationExpressionSyntax)node, diagnostics);
case SyntaxKind.StackAllocArrayCreationExpression:
return BindStackAllocArrayCreationExpression((StackAllocArrayCreationExpressionSyntax)node, diagnostics);
case SyntaxKind.ImplicitStackAllocArrayCreationExpression:
return BindImplicitStackAllocArrayCreationExpression((ImplicitStackAllocArrayCreationExpressionSyntax)node, diagnostics);
case SyntaxKind.ObjectCreationExpression:
return BindObjectCreationExpression((ObjectCreationExpressionSyntax)node, diagnostics);
case SyntaxKind.IdentifierName:
Expand Down Expand Up @@ -2714,6 +2716,40 @@ private BoundExpression BindImplicitArrayCreationExpression(ImplicitArrayCreatio
sizes: ImmutableArray<BoundExpression>.Empty, boundInitExprOpt: boundInitializerExpressions);
}

private BoundExpression BindImplicitStackAllocArrayCreationExpression(ImplicitStackAllocArrayCreationExpressionSyntax node, DiagnosticBag diagnostics)
Copy link
Member

@jcouv jcouv Feb 7, 2018

Choose a reason for hiding this comment

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

Should this method check Flags too?
Maybe this will become mute point if the syntax can be unified (use a single syntax node to represent both implicit and explicit cases). #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See the comment above re syntax unification.

{
bool inLegalPosition = ReportBadStackAllocPosition(node, diagnostics);
bool hasErrors = !inLegalPosition;

InitializerExpressionSyntax initializer = node.Initializer;
ImmutableArray<BoundExpression> boundInitializerExpressions = BindArrayInitializerExpressions(initializer, diagnostics, dimension: 1, rank: 1);

HashSet<DiagnosticInfo> useSiteDiagnostics = null;
TypeSymbol bestType = BestTypeInferrer.InferBestType(boundInitializerExpressions, this.Conversions, out bool hadMultipleCandidates, ref useSiteDiagnostics);
diagnostics.Add(node, useSiteDiagnostics);

if ((object)bestType == null || bestType.SpecialType == SpecialType.System_Void)
{
Error(diagnostics, ErrorCode.ERR_ImplicitlyTypedArrayNoBestType, node);
bestType = CreateErrorType();
}

if (!bestType.IsErrorType() && bestType.IsManagedType)
{
Error(diagnostics, ErrorCode.ERR_ManagedAddr, node, bestType);
}

return BindStackAllocWithInitializer(
node,
initializer,
type: inLegalPosition ? GetStackAllocType(node, bestType, diagnostics) : null,
elementType: bestType,
sizeOpt: null,
diagnostics,
hasErrors,
boundInitializerExpressions);
}

// This method binds all the array initializer expressions.
// NOTE: It doesn't convert the bound initializer expressions to array's element type.
// NOTE: This is done separately in ConvertAndBindArrayInitialization method below.
Expand Down Expand Up @@ -3032,25 +3068,8 @@ private BoundArrayCreation BindArrayCreationWithInitializer(
private BoundExpression BindStackAllocArrayCreationExpression(
StackAllocArrayCreationExpressionSyntax node, DiagnosticBag diagnostics)
{
bool hasErrors = false;
var inLegalPosition = (IsInMethodBody || IsLocalFunctionsScopeBinder) && node.IsLegalSpanStackAllocPosition();

if (!inLegalPosition)
{
hasErrors = true;
diagnostics.Add(
ErrorCode.ERR_InvalidExprTerm,
node.StackAllocKeyword.GetLocation(),
SyntaxFacts.GetText(SyntaxKind.StackAllocKeyword));
}

// Check if we're syntactically within a catch or finally clause.
if (this.Flags.Includes(BinderFlags.InCatchBlock) ||
this.Flags.Includes(BinderFlags.InCatchFilter) ||
this.Flags.Includes(BinderFlags.InFinallyBlock))
{
Error(diagnostics, ErrorCode.ERR_StackallocInCatchFinally, node);
}
bool inLegalPosition = ReportBadStackAllocPosition(node, diagnostics);
bool hasErrors = !inLegalPosition;

TypeSyntax typeSyntax = node.Type;

Expand All @@ -3070,24 +3089,22 @@ private BoundExpression BindStackAllocArrayCreationExpression(
TypeSyntax elementTypeSyntax = arrayTypeSyntax.ElementType;
TypeSymbol elementType = BindType(elementTypeSyntax, diagnostics);

bool typeHasErrors = elementType.IsErrorType();
if (!typeHasErrors && elementType.IsManagedType)
if (!elementType.IsErrorType() && elementType.IsManagedType)
{
Error(diagnostics, ErrorCode.ERR_ManagedAddr, elementTypeSyntax, elementType);
typeHasErrors = true;
hasErrors = true;
}

SyntaxList<ArrayRankSpecifierSyntax> rankSpecifiers = arrayTypeSyntax.RankSpecifiers;

if (rankSpecifiers.Count != 1 ||
rankSpecifiers[0].Sizes.Count != 1 ||
rankSpecifiers[0].Sizes[0].Kind() == SyntaxKind.OmittedArraySizeExpression)
rankSpecifiers[0].Sizes.Count != 1)
{
// NOTE: Dev10 reported several parse errors here.
Error(diagnostics, ErrorCode.ERR_BadStackAllocExpr, typeSyntax);

var builder = ArrayBuilder<BoundExpression>.GetInstance();
DiagnosticBag discardedDiagnostics = DiagnosticBag.GetInstance();
var discardedDiagnostics = DiagnosticBag.GetInstance();
foreach (ArrayRankSpecifierSyntax rankSpecifier in rankSpecifiers)
{
foreach (ExpressionSyntax size in rankSpecifier.Sizes)
Expand All @@ -3098,6 +3115,7 @@ private BoundExpression BindStackAllocArrayCreationExpression(
}
}
}

discardedDiagnostics.Free();

return new BoundBadExpression(
Expand All @@ -3108,33 +3126,116 @@ private BoundExpression BindStackAllocArrayCreationExpression(
new PointerTypeSymbol(elementType));
}

TypeSymbol type = inLegalPosition ? GetStackAllocType(node, elementType, diagnostics) : null;
Copy link
Member

Choose a reason for hiding this comment

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

Not related to this PR: I'm surprised that the type isn't set when stackalloc is bad position. I would think that would negatively affect the semantic model behavior.
Would you mind adding a test to confirm, and if there is a problem, either fixing it or filing an issue? Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm surprised that the type isn't set when stackalloc is bad position.

in addition to that, type isn't set when stackalloc is not directly in a local declaration init.


ExpressionSyntax countSyntax = rankSpecifiers[0].Sizes[0];
var count = BindValue(countSyntax, diagnostics, BindValueKind.RValue);
if (!count.HasAnyErrors)
BoundExpression count = null;
if (countSyntax.Kind() != SyntaxKind.OmittedArraySizeExpression)
{
// NOTE: this is different from how we would bind an array size (in which case we would allow uint, long, or ulong).
count = GenerateConversionForAssignment(GetSpecialType(SpecialType.System_Int32, diagnostics, node), count, diagnostics);
if (!count.HasAnyErrors && IsNegativeConstantForArraySize(count))
count = BindValue(countSyntax, diagnostics, BindValueKind.RValue);
if (!count.HasAnyErrors)
{
Error(diagnostics, ErrorCode.ERR_NegativeStackAllocSize, countSyntax);
hasErrors = true;
// NOTE: this is different from how we would bind an array size (in which case we would allow uint, long, or ulong).
count = GenerateConversionForAssignment(GetSpecialType(SpecialType.System_Int32, diagnostics, node), count, diagnostics);
if (!count.HasAnyErrors && IsNegativeConstantForArraySize(count))
{
Error(diagnostics, ErrorCode.ERR_NegativeStackAllocSize, countSyntax);
hasErrors = true;
}
}
}
else if (node.Initializer == null)
{
// ERR_MissingArraySize is already reported
count = BadExpression(countSyntax);
hasErrors = true;
}

TypeSymbol type = null;
if (inLegalPosition && !node.IsVariableDeclarationInitialization())
return node.Initializer == null
? new BoundStackAllocArrayCreation(node, elementType, count, initializerOpt: null, type, hasErrors: hasErrors)
: BindStackAllocWithInitializer(node, node.Initializer, type, elementType, count, diagnostics, hasErrors);
}

private bool ReportBadStackAllocPosition(SyntaxNode node, DiagnosticBag diagnostics)
{
Debug.Assert(node is StackAllocArrayCreationExpressionSyntax || node is ImplicitStackAllocArrayCreationExpressionSyntax);

var inLegalPosition = (IsInMethodBody || IsLocalFunctionsScopeBinder) && node.IsLegalSpanStackAllocPosition();
if (!inLegalPosition)
{
diagnostics.Add(
ErrorCode.ERR_InvalidExprTerm,
node.GetFirstToken().GetLocation(),
SyntaxFacts.GetText(SyntaxKind.StackAllocKeyword));
}

// Check if we're syntactically within a catch or finally clause.
if (this.Flags.IncludesAny(BinderFlags.InCatchBlock | BinderFlags.InCatchFilter | BinderFlags.InFinallyBlock))
{
Error(diagnostics, ErrorCode.ERR_StackallocInCatchFinally, node);
}

return inLegalPosition;
}

private TypeSymbol GetStackAllocType(SyntaxNode node, TypeSymbol elementType, DiagnosticBag diagnostics)
{
if (!node.IsVariableDeclarationInitialization())
{
CheckFeatureAvailability(node, MessageID.IDS_FeatureRefStructs, diagnostics);
GetWellKnownTypeMember(Compilation, WellKnownMember.System_Span_T__ctor, diagnostics, syntax: node);

var spanType = GetWellKnownType(WellKnownType.System_Span_T, diagnostics, node);
if (!spanType.IsErrorType())
{
type = spanType.Construct(elementType);
return spanType.Construct(elementType);
}
}

return new BoundStackAllocArrayCreation(node, elementType, count, type, hasErrors: hasErrors || typeHasErrors);
return null;
}

private BoundExpression BindStackAllocWithInitializer(
SyntaxNode node,
InitializerExpressionSyntax initSyntax,
TypeSymbol type,
TypeSymbol elementType,
BoundExpression sizeOpt,
DiagnosticBag diagnostics,
bool hasErrors,
ImmutableArray<BoundExpression> boundInitExprOpt = default)
{
if (boundInitExprOpt.IsDefault)
{
boundInitExprOpt = BindArrayInitializerExpressions(initSyntax, diagnostics, dimension: 1, rank: 1);
}

boundInitExprOpt = boundInitExprOpt.SelectAsArray((expr, t) => GenerateConversionForAssignment(t.elementType, expr, t.diagnostics), (elementType, diagnostics));

if (sizeOpt != null)
{
int? constantSizeOpt = GetIntegerConstantForArraySize(sizeOpt);
if (!sizeOpt.HasAnyErrors && constantSizeOpt == null)
{
Error(diagnostics, ErrorCode.ERR_ConstantExpected, sizeOpt.Syntax);
hasErrors = true;
}
else if (boundInitExprOpt.Length != constantSizeOpt)
{
Error(diagnostics, ErrorCode.ERR_ArrayInitializerIncorrectLength, node, constantSizeOpt.Value);
hasErrors = true;
}
}
else
{
sizeOpt = new BoundLiteral(
node,
ConstantValue.Create(boundInitExprOpt.Length),
GetSpecialType(SpecialType.System_Int32, diagnostics, node))
{ WasCompilerGenerated = true };
}

return new BoundStackAllocArrayCreation(node, elementType, sizeOpt, new BoundArrayInitialization(initSyntax, boundInitExprOpt), type, hasErrors);
}

private static int? GetIntegerConstantForArraySize(BoundExpression expression)
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1444,7 +1444,8 @@

<Node Name="BoundStackAllocArrayCreation" Base="BoundExpression">
<Field Name="ElementType" Type="TypeSymbol" Null="disallow"/>
<Field Name="Count" Type="BoundExpression"/>
<Field Name="Count" Type="BoundExpression" Null="disallow" />
<Field Name="InitializerOpt" Type="BoundArrayInitialization" Null="allow"/>
</Node>

<Node Name="BoundConvertedStackAllocExpression" Base="BoundStackAllocArrayCreation">
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/BoundTree/Formatting.cs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ internal partial class BoundStackAllocArrayCreation
{
public override object Display
{
get { return string.Format(MessageID.IDS_StackAllocExpression.Localize().ToString(), ElementType, Count.Syntax); }
get { return string.Format(MessageID.IDS_StackAllocExpression.Localize().ToString(), ElementType, Count.WasCompilerGenerated ? null : Count.Syntax); }
}
}
}
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/CSharpResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -5253,4 +5253,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_InDynamicMethodArg" xml:space="preserve">
<value>Arguments with 'in' modifier cannot be used in dynamically dispatched expessions.</value>
</data>
</root>
<data name="IDS_FeatureStackAllocInitializer" xml:space="preserve">
<value>stackalloc initializer</value>
</data>
</root>
17 changes: 17 additions & 0 deletions src/Compilers/CSharp/Portable/CodeGen/EmitExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1889,6 +1889,23 @@ private void EmitConvertedStackAllocExpression(BoundConvertedStackAllocExpressio
{
_builder.EmitOpCode(ILOpCode.Localloc);
}

var initializer = expression.InitializerOpt;
if (initializer != null)
{
if (used)
{
EmitStackAllocInitializers(expression.Type, initializer);
}
else
{
// If not used, just emit initializer elements to preserve possible sideeffects
Copy link
Member

@jcouv jcouv Feb 7, 2018

Choose a reason for hiding this comment

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

Could you point me to the test that covers that? I'm not sure how to hit this.
Never mind, found it (TestUnused) #Resolved

foreach (var init in initializer.Initializers)
{
EmitExpression(init, used: false);
}
}
}
}

private void EmitObjectCreationExpression(BoundObjectCreationExpression expression, bool used)
Expand Down
Loading