Skip to content

Commit

Permalink
Avoid generating or emitting NullablePublicOnlyAttribute when no othe…
Browse files Browse the repository at this point in the history 
…r nullable attributes are emitted (#37019)
  • Loading branch information
@cston
cston authored Jul 9, 2019
1 parent dac9135 commit eb170a1
Show file tree
Hide file tree
Showing 6 changed files with 345 additions and 57 deletions.
7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/Emitter/Model/PEAssemblyBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,12 @@ private void CreateEmbeddedAttributesIfNeeded(DiagnosticBag diagnostics)
{
EmbeddableAttributes needsAttributes = GetNeedsGeneratedAttributes();

if (needsAttributes == 0)
if (ShouldEmitNullablePublicOnlyAttribute() &&
Compilation.CheckIfAttributeShouldBeEmbedded(EmbeddableAttributes.NullablePublicOnlyAttribute, diagnostics, Location.None))
{
needsAttributes |= EmbeddableAttributes.NullablePublicOnlyAttribute;
}
else if (needsAttributes == 0)
{
return;
}
Expand Down
9 changes: 8 additions & 1 deletion src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ private EmbeddableAttributes GetNeedsGeneratedAttributesInternal()
return (EmbeddableAttributes)_needsGeneratedAttributes | Compilation.GetNeedsGeneratedAttributes();
}

internal void SetNeedsGeneratedAttributes(EmbeddableAttributes attributes)
private void SetNeedsGeneratedAttributes(EmbeddableAttributes attributes)
{
Debug.Assert(!_needsGeneratedAttributes_IsFrozen);
ThreadSafeFlagOperations.Set(ref _needsGeneratedAttributes, (int)attributes);
Expand Down Expand Up @@ -1542,6 +1542,13 @@ internal virtual SynthesizedAttributeData SynthesizeNullableContextAttribute(Imm
return Compilation.TrySynthesizeAttribute(WellKnownMember.System_Runtime_CompilerServices_NullableContextAttribute__ctor, arguments, isOptionalUse: true);
}

internal bool ShouldEmitNullablePublicOnlyAttribute()
{
// No need to look at this.GetNeedsGeneratedAttributes() since those bits are
// only set for members generated by the rewriter which are not public.
return Compilation.GetUsesNullableAttributes() && Compilation.EmitNullablePublicOnly;
}

internal virtual SynthesizedAttributeData SynthesizeNullablePublicOnlyAttribute(ImmutableArray<TypedConstant> arguments)
{
// For modules, this attribute should be present. Only assemblies generate and embed this type.
Expand Down
26 changes: 20 additions & 6 deletions src/Compilers/CSharp/Portable/Symbols/Compilation_WellKnownMembers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ public partial class CSharpCompilation
/// </summary>
private Symbol[] _lazyWellKnownTypeMembers;

private bool _usesNullableAttributes;
private int _needsGeneratedAttributes;
private bool _needsGeneratedAttributes_IsFrozen;

Expand All @@ -42,12 +43,24 @@ internal EmbeddableAttributes GetNeedsGeneratedAttributes()
return (EmbeddableAttributes)_needsGeneratedAttributes;
}

internal void SetNeedsGeneratedAttributes(EmbeddableAttributes attributes)
private void SetNeedsGeneratedAttributes(EmbeddableAttributes attributes)
{
Debug.Assert(!_needsGeneratedAttributes_IsFrozen);
ThreadSafeFlagOperations.Set(ref _needsGeneratedAttributes, (int)attributes);
}

internal bool GetUsesNullableAttributes()
{
_needsGeneratedAttributes_IsFrozen = true;
return _usesNullableAttributes;
}

private void SetUsesNullableAttributes()
{
Debug.Assert(!_needsGeneratedAttributes_IsFrozen);
_usesNullableAttributes = true;
}

/// <summary>
/// Lookup member declaration in well known type used by this Compilation.
/// </summary>
Expand Down Expand Up @@ -456,6 +469,12 @@ private void EnsureEmbeddableAttributeExists(EmbeddableAttributes attribute, Dia
{
SetNeedsGeneratedAttributes(attribute);
}

if ((attribute & (EmbeddableAttributes.NullableAttribute | EmbeddableAttributes.NullableContextAttribute)) != 0 &&
modifyCompilation)
{
SetUsesNullableAttributes();
}
}

internal void EnsureIsReadOnlyAttributeExists(DiagnosticBag diagnostics, Location location, bool modifyCompilation)
Expand Down Expand Up @@ -483,11 +502,6 @@ internal void EnsureNullableContextAttributeExists(DiagnosticBag diagnostics, Lo
EnsureEmbeddableAttributeExists(EmbeddableAttributes.NullableContextAttribute, diagnostics, location, modifyCompilation);
}

internal void EnsureNullablePublicOnlyAttributeExists(DiagnosticBag diagnostics, Location location, bool modifyCompilation)
{
EnsureEmbeddableAttributeExists(EmbeddableAttributes.NullablePublicOnlyAttribute, diagnostics, location, modifyCompilation);
}

internal bool CheckIfAttributeShouldBeEmbedded(EmbeddableAttributes attribute, DiagnosticBag diagnosticsOpt, Location locationOpt)
{
switch (attribute)
Expand Down
32 changes: 1 addition & 31 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceModuleSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,18 +255,6 @@ internal override void ForceComplete(SourceLocation locationOpt, CancellationTok
_state.SpinWaitComplete(CompletionPart.FinishValidatingReferencedAssemblies, cancellationToken);
break;

case CompletionPart.StartMemberChecks:
case CompletionPart.FinishMemberChecks:
if (_state.NotePartComplete(CompletionPart.StartMemberChecks))
{
var diagnostics = DiagnosticBag.GetInstance();
AfterMembersChecks(diagnostics);
AddDeclarationDiagnostics(diagnostics);
diagnostics.Free();
_state.NotePartComplete(CompletionPart.FinishMemberChecks);
}
break;

case CompletionPart.MembersCompleted:
this.GlobalNamespace.ForceComplete(locationOpt, cancellationToken);

Expand Down Expand Up @@ -533,24 +521,6 @@ internal override void DecodeWellKnownAttribute(ref DecodeWellKnownAttributeArgu
}
}

private bool EmitNullablePublicOnlyAttribute
{
get
{
var compilation = DeclaringCompilation;
return compilation.EmitNullablePublicOnly &&
compilation.IsFeatureEnabled(MessageID.IDS_FeatureNullableReferenceTypes);
}
}

private void AfterMembersChecks(DiagnosticBag diagnostics)
{
if (EmitNullablePublicOnlyAttribute)
{
DeclaringCompilation.EnsureNullablePublicOnlyAttributeExists(diagnostics, location: NoLocation.Singleton, modifyCompilation: true);
}
}

internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, ref ArrayBuilder<SynthesizedAttributeData> attributes)
{
base.AddSynthesizedAttributes(moduleBuilder, ref attributes);
Expand All @@ -566,7 +536,7 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r
}
}

if (EmitNullablePublicOnlyAttribute)
if (moduleBuilder.ShouldEmitNullablePublicOnlyAttribute())
{
var includesInternals = ImmutableArray.Create(
new TypedConstant(compilation.GetSpecialType(SpecialType.System_Boolean), TypedConstantKind.Primitive, _assemblySymbol.InternalsAreVisible));
Expand Down
69 changes: 69 additions & 0 deletions src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_Nullable.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,75 @@ static void F(object? x, object?[] y) { }
Diagnostic(ErrorCode.ERR_MissingPredefinedMember, "object?[] y").WithArguments("System.Runtime.CompilerServices.NullableAttribute", ".ctor").WithLocation(10, 30));
}

[Fact]
public void AttributeFromInternalsVisibleTo_01()
{
var sourceA =
@"using System.Runtime.CompilerServices;
[assembly: InternalsVisibleTo(""B"")]
#nullable enable
class A
{
object? F = null;
}";
var options = TestOptions.ReleaseDll.WithMetadataImportOptions(MetadataImportOptions.All);
var comp = CreateCompilation(sourceA, assemblyName: "A", options: options);
CompileAndVerify(comp, symbolValidator: m => CheckAttribute(m.GlobalNamespace.GetMember("A.F").GetAttributes().Single(), "A"));
var refA = comp.EmitToImageReference();

var sourceB =
@"#nullable enable
class B
{
object? G = new A();
}";
comp = CreateCompilation(sourceB, references: new[] { refA }, assemblyName: "B", options: options);
CompileAndVerify(comp, symbolValidator: m => CheckAttribute(m.GlobalNamespace.GetMember("B.G").GetAttributes().Single(), "B"));
}

[Fact]
public void AttributeFromInternalsVisibleTo_02()
{
var sourceAttribute =
@"namespace System.Runtime.CompilerServices
{
internal sealed class NullableAttribute : Attribute
{
public NullableAttribute(byte b) { }
public NullableAttribute(byte[] b) { }
}
}";
var sourceA =
@"using System.Runtime.CompilerServices;
[assembly: InternalsVisibleTo(""B"")]
#nullable enable
class A
{
object? F = null;
}";
var options = TestOptions.ReleaseDll.WithMetadataImportOptions(MetadataImportOptions.All);
var comp = CreateCompilation(new[] { sourceAttribute, sourceA }, assemblyName: "A", options: options);
CompileAndVerify(comp, symbolValidator: m => CheckAttribute(m.GlobalNamespace.GetMember("A.F").GetAttributes().Single(), "A"));
var refA = comp.EmitToImageReference();

var sourceB =
@"#nullable enable
class B
{
object? G = new A();
}";
comp = CreateCompilation(sourceB, references: new[] { refA }, assemblyName: "B", options: options);
CompileAndVerify(comp, symbolValidator: m => CheckAttribute(m.GlobalNamespace.GetMember("B.G").GetAttributes().Single(), "A"));
}

private static void CheckAttribute(CSharpAttributeData attribute, string assemblyName)
{
var attributeType = attribute.AttributeClass;
Assert.Equal("System.Runtime.CompilerServices", attributeType.ContainingNamespace.QualifiedName);
Assert.Equal("NullableAttribute", attributeType.Name);
Assert.Equal(assemblyName, attributeType.ContainingAssembly.Name);
}

[Fact]
public void NullableAttribute_MissingByte()
{
Expand Down
Loading

0 comments on commit eb170a1

Please sign in to comment.