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

Store nullable attribute constructor params in a field: #33668

Merged
merged 4 commits into from
Feb 27, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
56 changes: 24 additions & 32 deletions src/Compilers/CSharp/Portable/Emitter/Model/PEAssemblyBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -286,11 +286,9 @@ private void CreateEmbeddedAttributesIfNeeded(DiagnosticBag diagnostics)
{
CreateEmbeddedAttributeItselfIfNeeded(diagnostics);

CreateEmbeddedAttributeIfNeeded(
CreateEmbeddedNullableAttributeIfNeeded(
ref _lazyNullableAttribute,
diagnostics,
AttributeDescription.NullableAttribute,
GetNullableAttributeConstructors);
diagnostics);
}
}

Expand All @@ -305,42 +303,36 @@ private void CreateEmbeddedAttributeItselfIfNeeded(DiagnosticBag diagnostics)
private void CreateEmbeddedAttributeIfNeeded(
ref SynthesizedEmbeddedAttributeSymbol symbol,
DiagnosticBag diagnostics,
AttributeDescription description,
Func<CSharpCompilation, NamedTypeSymbol, DiagnosticBag, ImmutableArray<MethodSymbol>> getConstructors = null)
AttributeDescription description)
{
if ((object)symbol == null)
Copy link
Member

@jaredpar jaredpar Feb 27, 2019

Choose a reason for hiding this comment

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

symbol is null #Resolved

{
var attributeMetadataName = MetadataTypeName.FromFullName(description.FullName);
var userDefinedAttribute = _sourceAssembly.SourceModule.LookupTopLevelMetadataType(ref attributeMetadataName);
Debug.Assert((object)userDefinedAttribute.ContainingModule == _sourceAssembly.SourceModule);

if (!(userDefinedAttribute is MissingMetadataTypeSymbol))
{
diagnostics.Add(ErrorCode.ERR_TypeReserved, userDefinedAttribute.Locations[0], description.FullName);
}

symbol = new SynthesizedEmbeddedAttributeSymbol(description, _sourceAssembly.DeclaringCompilation, getConstructors, diagnostics);
AddDiagnosticsForExistingAttribute(description, diagnostics);
symbol = new SynthesizedEmbeddedAttributeSymbol(description, _sourceAssembly.DeclaringCompilation, diagnostics);
}
}

private static ImmutableArray<MethodSymbol> GetNullableAttributeConstructors(
CSharpCompilation compilation,
NamedTypeSymbol containingType,
private void CreateEmbeddedNullableAttributeIfNeeded(
ref SynthesizedEmbeddedAttributeSymbol symbol,
DiagnosticBag diagnostics)
{
var byteType = TypeSymbolWithAnnotations.Create(compilation.GetSpecialType(SpecialType.System_Byte));
Binder.ReportUseSiteDiagnostics(byteType.TypeSymbol, diagnostics, Location.None);
var byteArray = TypeSymbolWithAnnotations.Create(
ArrayTypeSymbol.CreateSZArray(
byteType.TypeSymbol.ContainingAssembly,
byteType));
return ImmutableArray.Create<MethodSymbol>(
new SynthesizedEmbeddedAttributeConstructorSymbol(
containingType,
m => ImmutableArray.Create(SynthesizedParameterSymbol.Create(m, byteType, 0, RefKind.None))),
new SynthesizedEmbeddedAttributeConstructorSymbol(
containingType,
m => ImmutableArray.Create(SynthesizedParameterSymbol.Create(m, byteArray, 0, RefKind.None))));
if ((object)symbol == null)
Copy link
Member

@jaredpar jaredpar Feb 27, 2019

Choose a reason for hiding this comment

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

symbol is null #Resolved

{
AddDiagnosticsForExistingAttribute(AttributeDescription.NullableAttribute, diagnostics);
symbol = new SynthesizedEmbeddedNullableAttributeSymbol(_sourceAssembly.DeclaringCompilation, diagnostics);
}
}

private void AddDiagnosticsForExistingAttribute(AttributeDescription description, DiagnosticBag diagnostics)
{
var attributeMetadataName = MetadataTypeName.FromFullName(description.FullName);
var userDefinedAttribute = _sourceAssembly.SourceModule.LookupTopLevelMetadataType(ref attributeMetadataName);
Debug.Assert((object)userDefinedAttribute.ContainingModule == _sourceAssembly.SourceModule);

if (!(userDefinedAttribute is MissingMetadataTypeSymbol))
{
diagnostics.Add(ErrorCode.ERR_TypeReserved, userDefinedAttribute.Locations[0], description.FullName);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,25 @@ internal class SynthesizedEmbeddedAttributeSymbol : NamedTypeSymbol
private readonly string _name;
private readonly NamedTypeSymbol _baseType;
private readonly NamespaceSymbol _namespace;
private readonly ImmutableArray<MethodSymbol> _constructors;
private readonly ModuleSymbol _module;

protected ImmutableArray<MethodSymbol> _constructors;
Copy link
Member

Choose a reason for hiding this comment

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

protected values should be PascalCased and not _ prefixed.


public SynthesizedEmbeddedAttributeSymbol(
AttributeDescription description,
CSharpCompilation compilation,
Func<CSharpCompilation, NamedTypeSymbol, DiagnosticBag, ImmutableArray<MethodSymbol>> getConstructors,
DiagnosticBag diagnostics)
DiagnosticBag diagnostics,
bool generateDefaultConstructors = true)
{
_name = description.Name;
_baseType = MakeBaseType(compilation, diagnostics);

if (getConstructors != null)
{
_constructors = getConstructors(compilation, this, diagnostics);
}
else
if (generateDefaultConstructors)
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 hate this. Any ideas on a better approach most welcome.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps replace the _constructors field with an abstract Constructors property, and have two derived types, one that returns a single constructor with no parameters.


In reply to: 260050571 [](ancestors = 260050571)

{
_constructors = ImmutableArray.Create<MethodSymbol>(new SynthesizedEmbeddedAttributeConstructorSymbol(this, m => ImmutableArray<ParameterSymbol>.Empty));
Debug.Assert(_constructors.Length == description.Signatures.Length);
}

Debug.Assert(_constructors.Length == description.Signatures.Length);

_module = compilation.SourceModule;

_namespace = _module.GlobalNamespace;
Expand Down Expand Up @@ -217,4 +213,5 @@ internal override void GenerateMethodBody(TypeCompilationState compilationState,
GenerateMethodBodyCore(compilationState, diagnostics);
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using Microsoft.Cci;
using Roslyn.Utilities;
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Runtime.InteropServices;
using System.Linq;
using Microsoft.CodeAnalysis.CSharp.Emit;
using Microsoft.CodeAnalysis.PooledObjects;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{

internal class SynthesizedEmbeddedNullableAttributeSymbol : SynthesizedEmbeddedAttributeSymbol
Copy link
Member

Choose a reason for hiding this comment

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

class [](start = 13, length = 5)

sealed

{
private readonly ImmutableArray<FieldSymbol> _fields;

private readonly TypeSymbolWithAnnotations _byteType;
Copy link
Member

Choose a reason for hiding this comment

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

TypeSymbolWithAnnotations [](start = 25, length = 25)

Can be stored as TypeSymbol.


public SynthesizedEmbeddedNullableAttributeSymbol(
CSharpCompilation compilation,
DiagnosticBag diagnostics)
: base(AttributeDescription.NullableAttribute, compilation, diagnostics, generateDefaultConstructors: false)
{
_byteType = TypeSymbolWithAnnotations.Create(compilation.GetSpecialType(SpecialType.System_Byte));
Binder.ReportUseSiteDiagnostics(_byteType.TypeSymbol, diagnostics, Location.None);
var byteArrayType = TypeSymbolWithAnnotations.Create(
ArrayTypeSymbol.CreateSZArray(
_byteType.TypeSymbol.ContainingAssembly,
_byteType));

_fields = ImmutableArray.Create<FieldSymbol>(
new SynthesizedFieldSymbol(
this,
byteArrayType.TypeSymbol,
"nullableFlags",
Copy link
Member

Choose a reason for hiding this comment

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

Given this is public it should be PascalCased

isPublic: true,
isReadOnly: true,
isStatic: false));

_constructors = ImmutableArray.Create<MethodSymbol>(
new SynthesizedEmbeddedAttributeConstructorWithBodySymbol(
this,
m => ImmutableArray.Create(SynthesizedParameterSymbol.Create(m, _byteType, 0, RefKind.None)),
GenerateSingleByteConstructorBody
),
new SynthesizedEmbeddedAttributeConstructorWithBodySymbol(
this,
m => ImmutableArray.Create(SynthesizedParameterSymbol.Create(m, byteArrayType, 0, RefKind.None)),
GenerateByteArrayConstructorBody
)
);

// Ensure we never get out of sync with the description
Debug.Assert(_constructors.Length == AttributeDescription.NullableAttribute.Signatures.Length);
}

internal override IEnumerable<FieldSymbol> GetFieldsToEmit() => _fields;

private void GenerateByteArrayConstructorBody(SyntheticBoundNodeFactory factory, ArrayBuilder<BoundStatement> statements, ImmutableArray<ParameterSymbol> parameters)
{
statements.Add(
factory.ExpressionStatement(
factory.AssignmentExpression(
factory.Field(
factory.This(),
_fields.Single()),
factory.Parameter(parameters.Single())
)
)
);
}

private void GenerateSingleByteConstructorBody(SyntheticBoundNodeFactory factory, ArrayBuilder<BoundStatement> statements, ImmutableArray<ParameterSymbol> parameters)
{
statements.Add(
factory.ExpressionStatement(
factory.AssignmentExpression(
factory.Field(
factory.This(),
_fields.Single()),
factory.Array(
_byteType.TypeSymbol,
ImmutableArray.Create<BoundExpression>(
factory.Parameter(parameters.Single())
)
)
)
)
);
}

internal sealed class SynthesizedEmbeddedAttributeConstructorWithBodySymbol : SynthesizedInstanceConstructor
Copy link
Member

Choose a reason for hiding this comment

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

internal [](start = 8, length = 8)

private

{
ImmutableArray<ParameterSymbol> _parameters;
Copy link
Member

Choose a reason for hiding this comment

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

ImmutableArray [](start = 12, length = 14)

private readonly


readonly Action<SyntheticBoundNodeFactory, ArrayBuilder<BoundStatement>, ImmutableArray<ParameterSymbol>> _getConstructorBody;
Copy link
Member

Choose a reason for hiding this comment

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

readonly [](start = 12, length = 8)

private


internal SynthesizedEmbeddedAttributeConstructorWithBodySymbol(
NamedTypeSymbol containingType,
Func<MethodSymbol, ImmutableArray<ParameterSymbol>> getParameters,
Action<SyntheticBoundNodeFactory, ArrayBuilder<BoundStatement>, ImmutableArray<ParameterSymbol>> getConstructorBody) :
base(containingType)
{
_parameters = getParameters(this);
_getConstructorBody = getConstructorBody;
}

public override ImmutableArray<ParameterSymbol> Parameters => _parameters;

internal override void GenerateMethodBody(TypeCompilationState compilationState, DiagnosticBag diagnostics)
{
GenerateMethodBodyCore(compilationState, diagnostics);
}

protected override void GenerateMethodBodyStatements(SyntheticBoundNodeFactory factory, ArrayBuilder<BoundStatement> statements, DiagnosticBag diagnostics) => _getConstructorBody(factory, statements, _parameters);
}


}
}

Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
Expand Down Expand Up @@ -286,11 +287,20 @@ protected void GenerateMethodBodyCore(TypeCompilationState compilationState, Dia
return;
}

var block = factory.Block(
factory.ExpressionStatement(baseConstructorCall),
factory.Return());
var statements = ArrayBuilder<BoundStatement>.GetInstance();
statements.Add(factory.ExpressionStatement(baseConstructorCall));
GenerateMethodBodyStatements(factory, statements, diagnostics);
statements.Add(factory.Return());

var block = factory.Block(statements.ToImmutableAndFree());

factory.CloseMethod(block);
}

protected virtual void GenerateMethodBodyStatements(SyntheticBoundNodeFactory factory, ArrayBuilder<BoundStatement> statements, DiagnosticBag diagnostics)
{
// overridden in a derived class to add extra statements to the body of the generated constructor
}

}
}
Loading