Skip to content

Commit

Permalink
Remove the dependence between the order in NullableAnnotation and met…
Browse files Browse the repository at this point in the history 
…adata attribute values (#34909)

Fixes #33952
  • Loading branch information
Neal Gafter committed Apr 15, 2019
1 parent 108bfbe commit e922628
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 48 deletions.
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs
View file
Expand Up @@ -1497,14 +1497,14 @@ internal SynthesizedAttributeData SynthesizeNullableAttribute(Symbol symbol, Typ
type.AddNullableTransforms(flagsBuilder);

Debug.Assert(flagsBuilder.Any());
Debug.Assert(flagsBuilder.Contains((byte)NullableAnnotation.NotAnnotated) || flagsBuilder.Contains((byte)NullableAnnotation.Annotated));
Debug.Assert(flagsBuilder.Contains(NullableAnnotationExtensions.NotAnnotatedAttributeValue) || flagsBuilder.Contains(NullableAnnotationExtensions.AnnotatedAttributeValue));

WellKnownMember constructor;
ImmutableArray<TypedConstant> arguments;
NamedTypeSymbol byteType = Compilation.GetSpecialType(SpecialType.System_Byte);
Debug.Assert((object)byteType != null);

if (flagsBuilder.All(flag => flag == (byte)NullableAnnotation.NotAnnotated) || flagsBuilder.All(flag => flag == (byte)NullableAnnotation.Annotated))
if (flagsBuilder.All(flag => flag == NullableAnnotationExtensions.NotAnnotatedAttributeValue) || flagsBuilder.All(flag => flag == NullableAnnotationExtensions.AnnotatedAttributeValue))
{
constructor = WellKnownMember.System_Runtime_CompilerServices_NullableAttribute__ctorByte;
arguments = ImmutableArray.Create(new TypedConstant(byteType, TypedConstantKind.Primitive, flagsBuilder[0]));
Expand All @@ -1515,7 +1515,7 @@ internal SynthesizedAttributeData SynthesizeNullableAttribute(Symbol symbol, Typ

foreach (byte flag in flagsBuilder)
{
Debug.Assert(flag == (byte)NullableAnnotation.Oblivious || flag == (byte)NullableAnnotation.NotAnnotated || flag == (byte)NullableAnnotation.Annotated);
Debug.Assert(flag == NullableAnnotationExtensions.ObliviousAttributeValue || flag == NullableAnnotationExtensions.NotAnnotatedAttributeValue || flag == NullableAnnotationExtensions.AnnotatedAttributeValue);
constantsBuilder.Add(new TypedConstant(byteType, TypedConstantKind.Primitive, flag));
}

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Symbols/ExtraAnnotations.cs
View file
Expand Up @@ -129,7 +129,7 @@ private static ImmutableArray<FlowAnalysisAnnotations> Array(params FlowAnalysis

private static ImmutableArray<byte> Nullable(bool isNullable)
{
return ImmutableArray.Create((byte)(isNullable ? NullableAnnotation.Annotated : NullableAnnotation.NotAnnotated));
return ImmutableArray.Create(isNullable ? NullableAnnotationExtensions.AnnotatedAttributeValue : NullableAnnotationExtensions.NotAnnotatedAttributeValue);
}

internal static ImmutableArray<ImmutableArray<byte>> GetExtraAnnotations(string key)
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PETypeParameterSymbol.cs
View file
Expand Up @@ -288,11 +288,11 @@ public override bool HasReferenceTypeConstraint

if (((PEModuleSymbol)this.ContainingModule).Module.HasNullableAttribute(_handle, out byte transformFlag, out _))
{
switch ((NullableAnnotation)transformFlag)
switch (transformFlag)
{
case NullableAnnotation.Annotated:
case NullableAnnotationExtensions.AnnotatedAttributeValue:
return true;
case NullableAnnotation.NotAnnotated:
case NullableAnnotationExtensions.NotAnnotatedAttributeValue:
return false;
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/Compilers/CSharp/Portable/Symbols/NullableAnnotation.cs
View file
Expand Up @@ -14,15 +14,15 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
internal enum NullableAnnotation : byte
{
/// <summary>
/// The type is not annotated in a context where the nullable feature is not enabled.
/// Used for interoperation with existing pre-nullable code.
/// Type is not annotated - string, int, T (including the case when T is unconstrained).
/// </summary>
Oblivious,
NotAnnotated,

/// <summary>
/// Type is not annotated - string, int, T (including the case when T is unconstrained).
/// The type is not annotated in a context where the nullable feature is not enabled.
/// Used for interoperation with existing pre-nullable code.
/// </summary>
NotAnnotated,
Oblivious,

/// <summary>
/// Type is annotated with '?' - string?, T? where T : class; and for int?, T? where T : struct.
Expand Down
57 changes: 31 additions & 26 deletions src/Compilers/CSharp/Portable/Symbols/NullableAnnotationExtensions.cs
View file
Expand Up @@ -2,6 +2,9 @@

using Roslyn.Utilities;

// https://github.com/dotnet/roslyn/issues/34962 IDE005 "Fix formatting" does a poor job with a switch expression as the body of an expression-bodied method
#pragma warning disable IDE0055

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
internal static class NullableAnnotationExtensions
Expand All @@ -16,49 +19,51 @@ internal static class NullableAnnotationExtensions
/// Join nullable annotations from the set of lower bounds for fixing a type parameter.
/// This uses the covariant merging rules.
/// </summary>
public static NullableAnnotation Join(this NullableAnnotation a, NullableAnnotation b)
{
if (a.IsAnnotated() || b.IsAnnotated())
return NullableAnnotation.Annotated;
return (a < b) ? a : b;
}
public static NullableAnnotation Join(this NullableAnnotation a, NullableAnnotation b) => (a < b) ? b : a;

/// <summary>
/// Meet two nullable annotations for computing the nullable annotation of a type parameter from upper bounds.
/// This uses the contravariant merging rules.
/// </summary>
public static NullableAnnotation Meet(this NullableAnnotation a, NullableAnnotation b)
{
if (a.IsNotAnnotated() || b.IsNotAnnotated())
return NullableAnnotation.NotAnnotated;
return (a < b) ? a : b;
}
public static NullableAnnotation Meet(this NullableAnnotation a, NullableAnnotation b) => (a < b) ? a : b;

/// <summary>
/// Check that two nullable annotations are "compatible", which means they could be the same. Return the
/// nullable annotation to be used as a result. This uses the invariant merging rules.
/// Return the nullable annotation to use when two annotations are expected to be "compatible", which means
/// they could be the same. These are the "invariant" merging rules.
/// </summary>
public static NullableAnnotation EnsureCompatible(this NullableAnnotation a, NullableAnnotation b)
{
if (a.IsOblivious())
return b;
if (b.IsOblivious())
return a;
return (a < b) ? a : b;
}
public static NullableAnnotation EnsureCompatible(this NullableAnnotation a, NullableAnnotation b) =>
(a, b) switch
{
(NullableAnnotation.Oblivious, _) => b,
(_, NullableAnnotation.Oblivious) => a,
_ => a < b ? a : b,
};

/// <summary>
/// Merges nullability.
/// </summary>
public static NullableAnnotation MergeNullableAnnotation(this NullableAnnotation a, NullableAnnotation b, VarianceKind variance)
{
return variance switch
public static NullableAnnotation MergeNullableAnnotation(this NullableAnnotation a, NullableAnnotation b, VarianceKind variance) =>
variance switch
{
VarianceKind.In => a.Meet(b),
VarianceKind.Out => a.Join(b),
VarianceKind.None => a.EnsureCompatible(b),
_ => throw ExceptionUtilities.UnexpectedValue(variance)
};
}

/// <summary>
/// The attribute (metadata) representation of <see cref="NullableAnnotation.NotAnnotated"/>.
/// </summary>
public const byte NotAnnotatedAttributeValue = 1;

/// <summary>
/// The attribute (metadata) representation of <see cref="NullableAnnotation.Annotated"/>.
/// </summary>
public const byte AnnotatedAttributeValue = 2;

/// <summary>
/// The attribute (metadata) representation of <see cref="NullableAnnotation.Oblivious"/>.
/// </summary>
public const byte ObliviousAttributeValue = 0;
}
}
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceTypeParameterSymbol.cs
View file
Expand Up @@ -370,9 +370,9 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r
ref attributes,
moduleBuilder.SynthesizeNullableAttribute(WellKnownMember.System_Runtime_CompilerServices_NullableAttribute__ctorByte,
ImmutableArray.Create(new TypedConstant(byteType, TypedConstantKind.Primitive,
(byte)(this.ReferenceTypeConstraintIsNullable == true ?
NullableAnnotation.Annotated :
NullableAnnotation.NotAnnotated)))));
(this.ReferenceTypeConstraintIsNullable == true ?
NullableAnnotationExtensions.AnnotatedAttributeValue :
NullableAnnotationExtensions.NotAnnotatedAttributeValue)))));
}
}

Expand Down
14 changes: 7 additions & 7 deletions src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs
View file
Expand Up @@ -551,15 +551,15 @@ public void AddNullableTransforms(ArrayBuilder<byte> transforms)

if (NullableAnnotation.IsOblivious() || typeSymbol.IsValueType)
{
flag = (byte)NullableAnnotation.Oblivious;
flag = NullableAnnotationExtensions.ObliviousAttributeValue;
}
else if (NullableAnnotation.IsAnnotated())
{
flag = (byte)NullableAnnotation.Annotated;
flag = NullableAnnotationExtensions.AnnotatedAttributeValue;
}
else
{
flag = (byte)NullableAnnotation.NotAnnotated;
flag = NullableAnnotationExtensions.NotAnnotatedAttributeValue;
}

transforms.Add(flag);
Expand Down Expand Up @@ -597,17 +597,17 @@ public bool ApplyNullableTransforms(byte defaultTransformFlag, ImmutableArray<by
result = result.WithTypeAndModifiers(newTypeSymbol, result.CustomModifiers);
}

switch ((NullableAnnotation)transformFlag)
switch (transformFlag)
{
case NullableAnnotation.Annotated:
case NullableAnnotationExtensions.AnnotatedAttributeValue:
result = result.AsNullableReferenceType();
break;

case NullableAnnotation.NotAnnotated:
case NullableAnnotationExtensions.NotAnnotatedAttributeValue:
result = result.AsNotNullableReferenceType();
break;

case NullableAnnotation.Oblivious:
case NullableAnnotationExtensions.ObliviousAttributeValue:
if (result.NullableAnnotation != NullableAnnotation.Oblivious &&
!(result.NullableAnnotation.IsAnnotated() && oldTypeSymbol.IsNullableType())) // Preserve nullable annotation on Nullable<T>.
{
Expand Down

0 comments on commit e922628

Please sign in to comment.