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

Remove the dependence between the order in NullableAnnotation and metadata attribute values #34909

Merged
merged 5 commits into from
Apr 15, 2019
Merged
Show file tree
Hide file tree
Changes from 4 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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,11 @@ internal override bool? ReferenceTypeConstraintIsNullable

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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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,76 @@ 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;

///// <summary>
///// The mapping from NullableAnnotation to the attribute (metadata) representations. This
///// method is not actually used today, but is here to document the mapping.
///// </summary>
//public static byte ToAttributeValue(this NullableAnnotation self) =>
// self switch
// {
// NullableAnnotation.NotAnnotated => NotAnnotatedAttributeValue,
// NullableAnnotation.Annotated => AnnotatedAttributeValue,
// NullableAnnotation.Oblivious => ObliviousAttributeValue,
// _ => throw ExceptionUtilities.UnexpectedValue(self)
// };

///// <summary>
///// The mapping from the attribute (metadata) representations to NullableAnnotation. This
///// method is not actually used today, but is here to document the mapping.
///// </summary>
//public static NullableAnnotation AttributeValueToNullableAnnotation(this byte value) =>
// value switch
// {
// NotAnnotatedAttributeValue => NullableAnnotation.NotAnnotated,
// AnnotatedAttributeValue => NullableAnnotation.Annotated,
// _ => NullableAnnotation.Oblivious, // metadata readers should somehow be resilient to bad metadata
// };
}
}
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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