Skip to content
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
14 changes: 9 additions & 5 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2883,6 +2883,7 @@ private FlowAnalysisAnnotations GetRValueAnnotations(Symbol symbol)
MethodSymbol method => method.ReturnTypeFlowAnalysisAnnotations,
PropertySymbol property => property.GetOwnOrInheritedGetMethod()?.ReturnTypeFlowAnalysisAnnotations ?? FlowAnalysisAnnotations.None,
ParameterSymbol parameter => parameter.FlowAnalysisAnnotations,
FieldSymbol field => field.FlowAnalysisAnnotations,
_ => FlowAnalysisAnnotations.None
};

Expand Down Expand Up @@ -5212,15 +5213,18 @@ private FlowAnalysisAnnotations GetLValueAnnotations(BoundExpression expr)
return FlowAnalysisAnnotations.None;
}

var symbol = expr switch
var annotations = expr switch
{
BoundPropertyAccess property => property.PropertySymbol,
BoundIndexerAccess indexer => indexer.Indexer,
_ => null
BoundPropertyAccess property => getSetterAnnotations(property.PropertySymbol),
BoundIndexerAccess indexer => getSetterAnnotations(indexer.Indexer),
BoundFieldAccess field => field.FieldSymbol.FlowAnalysisAnnotations,
_ => FlowAnalysisAnnotations.None
};

var annotations = symbol?.GetOwnOrInheritedSetMethod()?.Parameters.Last()?.FlowAnalysisAnnotations ?? FlowAnalysisAnnotations.None;
return annotations & (FlowAnalysisAnnotations.DisallowNull | FlowAnalysisAnnotations.AllowNull);

static FlowAnalysisAnnotations getSetterAnnotations(PropertySymbol property)
=> property.GetOwnOrInheritedSetMethod()?.Parameters.Last()?.FlowAnalysisAnnotations ?? FlowAnalysisAnnotations.None;
}

private static bool UseLegacyWarnings(BoundExpression expr, TypeWithAnnotations exprType)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ public override string Name
get { return GeneratedNames.MakeAnonymousTypeBackingFieldName(_property.Name); }
}

public override FlowAnalysisAnnotations FlowAnalysisAnnotations
=> FlowAnalysisAnnotations.None;

internal override bool HasSpecialName
{
get { return false; }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
/// <summary>
/// Information decoded from well-known custom attributes applied on a field.
/// </summary>
internal sealed class FieldWellKnownAttributeData : CommonFieldWellKnownAttributeData
{
private bool _hasAllowNullAttribute;
public bool HasAllowNullAttribute
{
get
{
VerifySealed(expected: true);
return _hasAllowNullAttribute;
}
set
{
VerifySealed(expected: false);
_hasAllowNullAttribute = value;
SetDataStored();
}
}

private bool _hasDisallowNullAttribute;
public bool HasDisallowNullAttribute
{
get
{
VerifySealed(expected: true);
return _hasDisallowNullAttribute;
}
set
{
VerifySealed(expected: false);
_hasDisallowNullAttribute = value;
SetDataStored();
}
}

private bool _hasMaybeNullAttribute;
public bool HasMaybeNullAttribute
{
get
{
VerifySealed(expected: true);
return _hasMaybeNullAttribute;
}
set
{
VerifySealed(expected: false);
_hasMaybeNullAttribute = value;
SetDataStored();
}
}

private bool? _maybeNullWhenAttribute;
public bool? MaybeNullWhenAttribute
{
get
{
VerifySealed(expected: true);
return _maybeNullWhenAttribute;
}
set
{
VerifySealed(expected: false);
_maybeNullWhenAttribute = value;
SetDataStored();
}
}

private bool _hasNotNullAttribute;
public bool HasNotNullAttribute
{
get
{
VerifySealed(expected: true);
return _hasNotNullAttribute;
}
set
{
VerifySealed(expected: false);
_hasNotNullAttribute = value;
SetDataStored();
}
}
}
}
2 changes: 2 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ public TypeWithAnnotations TypeWithAnnotations
}
}

public abstract FlowAnalysisAnnotations FlowAnalysisAnnotations { get; }
Copy link
Member

Choose a reason for hiding this comment

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

This name was pretty confusing for me at first. It doesn't really say anything about nullable or that it's only about the attributes, not ?

Copy link
Member Author

Choose a reason for hiding this comment

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

This only represents the annotations from user-specified attributes. I agree this name is a bit confusing (we have annotations like ? and flow analysis annotations like [DisallowNull]). I'll think about this for a subsequent PR.
Thanks


/// <summary>
/// Gets the type of this field.
/// </summary>
Expand Down
69 changes: 69 additions & 0 deletions src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,49 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE
/// </summary>
internal sealed class PEFieldSymbol : FieldSymbol
{
private struct PackedFlags
{
// Layout:
// |..............................|vvvvv|
//
// f = FlowAnalysisAnnotations. 5 bits (4 value bits + 1 completion bit).

private const int HasDisallowNullAttribute = 0x1 << 0;
private const int HasAllowNullAttribute = 0x1 << 1;
private const int HasMaybeNullAttribute = 0x1 << 2;
private const int HasNotNullAttribute = 0x1 << 3;
Copy link
Contributor

@cston cston Jun 24, 2019

Choose a reason for hiding this comment

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

Handling individual bits here is not necessary since FlowAnalysisAnnotations is already stored as a set of bits (see PEParameterSymbol.PackedFlags). #ByDesign

Copy link
Member Author

Choose a reason for hiding this comment

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

PEParameterSymbol can store all the possible annotations, but here we only need to store only four, which are not even contiguous (see below). So I think we have to deal with individual bits/flags.

    [Flags]
    internal enum FlowAnalysisAnnotations
    {
        None = 0,
        AllowNull = 1 << 0,
        DisallowNull = 1 << 1,
        MaybeNullWhenTrue = 1 << 2,
        MaybeNullWhenFalse = 1 << 3,
        MaybeNull = MaybeNullWhenTrue | MaybeNullWhenFalse,
        NotNullWhenTrue = 1 << 4,
        NotNullWhenFalse = 1 << 5,
        NotNull = NotNullWhenTrue | NotNullWhenFalse,
        AssertsTrue = 1 << 6,
        AssertsFalse = 1 << 7,
    }

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

private const int FlowAnalysisAnnotationsCompletionBit = 0x1 << 4;

private int _bits;

public bool SetFlowAnalysisAnnotations(FlowAnalysisAnnotations value)
{
Debug.Assert((value & ~(FlowAnalysisAnnotations.DisallowNull | FlowAnalysisAnnotations.AllowNull | FlowAnalysisAnnotations.MaybeNull | FlowAnalysisAnnotations.NotNull)) == 0);

int bitsToSet = FlowAnalysisAnnotationsCompletionBit;
if ((value & FlowAnalysisAnnotations.DisallowNull) != 0) bitsToSet |= PackedFlags.HasDisallowNullAttribute;
if ((value & FlowAnalysisAnnotations.AllowNull) != 0) bitsToSet |= PackedFlags.HasAllowNullAttribute;
if ((value & FlowAnalysisAnnotations.MaybeNull) != 0) bitsToSet |= PackedFlags.HasMaybeNullAttribute;
if ((value & FlowAnalysisAnnotations.NotNull) != 0) bitsToSet |= PackedFlags.HasNotNullAttribute;

return ThreadSafeFlagOperations.Set(ref _bits, bitsToSet);
}

public bool TryGetFlowAnalysisAnnotations(out FlowAnalysisAnnotations value)
{
int theBits = _bits; // Read this.bits once to ensure the consistency of the value and completion flags.
value = FlowAnalysisAnnotations.None;
if ((theBits & PackedFlags.HasDisallowNullAttribute) != 0) value |= FlowAnalysisAnnotations.DisallowNull;
if ((theBits & PackedFlags.HasAllowNullAttribute) != 0) value |= FlowAnalysisAnnotations.AllowNull;
if ((theBits & PackedFlags.HasMaybeNullAttribute) != 0) value |= FlowAnalysisAnnotations.MaybeNull;
if ((theBits & PackedFlags.HasNotNullAttribute) != 0) value |= FlowAnalysisAnnotations.NotNull;

var result = (theBits & FlowAnalysisAnnotationsCompletionBit) != 0;
Debug.Assert(value == 0 || result);
return result;
}
}

private readonly FieldDefinitionHandle _handle;
private readonly string _name;
private readonly FieldAttributes _flags;
Expand All @@ -36,6 +79,7 @@ internal sealed class PEFieldSymbol : FieldSymbol
private int _lazyFixedSize;
private NamedTypeSymbol _lazyFixedImplementationType;
private PEEventSymbol _associatedEventOpt;
private PackedFlags _packedFlags;

internal PEFieldSymbol(
PEModuleSymbol moduleSymbol,
Expand All @@ -48,6 +92,7 @@ internal PEFieldSymbol(

_handle = fieldDef;
_containingType = containingType;
_packedFlags = new PackedFlags();

try
{
Expand Down Expand Up @@ -272,6 +317,30 @@ internal override TypeWithAnnotations GetFieldType(ConsList<FieldSymbol> fieldsB
return _lazyType.Value;
}

public override FlowAnalysisAnnotations FlowAnalysisAnnotations
{
get
{
FlowAnalysisAnnotations value;
if (!_packedFlags.TryGetFlowAnalysisAnnotations(out value))
{
value = DecodeFlowAnalysisAttributes(_containingType.ContainingPEModule.Module, _handle);
_packedFlags.SetFlowAnalysisAnnotations(value);
}
return value;
}
}

private static FlowAnalysisAnnotations DecodeFlowAnalysisAttributes(PEModule module, FieldDefinitionHandle handle)
{
FlowAnalysisAnnotations annotations = FlowAnalysisAnnotations.None;
if (module.HasAttribute(handle, AttributeDescription.AllowNullAttribute)) annotations |= FlowAnalysisAnnotations.AllowNull;
if (module.HasAttribute(handle, AttributeDescription.DisallowNullAttribute)) annotations |= FlowAnalysisAnnotations.DisallowNull;
if (module.HasAttribute(handle, AttributeDescription.MaybeNullAttribute)) annotations |= FlowAnalysisAnnotations.MaybeNull;
if (module.HasAttribute(handle, AttributeDescription.NotNullAttribute)) annotations |= FlowAnalysisAnnotations.NotNull;
return annotations;
}

public override bool IsFixedSizeBuffer
{
get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,15 @@ private CustomAttributesBag<CSharpAttributeData> GetAttributesBag()
/// <remarks>
/// Forces binding and decoding of attributes.
/// </remarks>
protected CommonFieldWellKnownAttributeData GetDecodedWellKnownAttributeData()
protected FieldWellKnownAttributeData GetDecodedWellKnownAttributeData()
{
var attributesBag = _lazyCustomAttributesBag;
if (attributesBag == null || !attributesBag.IsDecodedWellKnownAttributeDataComputed)
{
attributesBag = this.GetAttributesBag();
}

return (CommonFieldWellKnownAttributeData)attributesBag.DecodedWellKnownAttributeData;
return (FieldWellKnownAttributeData)attributesBag.DecodedWellKnownAttributeData;
}

internal sealed override CSharpAttributeData EarlyDecodeWellKnownAttribute(ref EarlyDecodeWellKnownAttributeArguments<EarlyWellKnownAttributeBinder, NamedTypeSymbol, AttributeSyntax, AttributeLocation> arguments)
Expand Down Expand Up @@ -163,11 +163,11 @@ internal override void DecodeWellKnownAttribute(ref DecodeWellKnownAttributeArgu

if (attribute.IsTargetAttribute(this, AttributeDescription.SpecialNameAttribute))
{
arguments.GetOrCreateData<CommonFieldWellKnownAttributeData>().HasSpecialNameAttribute = true;
arguments.GetOrCreateData<FieldWellKnownAttributeData>().HasSpecialNameAttribute = true;
}
else if (attribute.IsTargetAttribute(this, AttributeDescription.NonSerializedAttribute))
{
arguments.GetOrCreateData<CommonFieldWellKnownAttributeData>().HasNonSerializedAttribute = true;
arguments.GetOrCreateData<FieldWellKnownAttributeData>().HasNonSerializedAttribute = true;
}
else if (attribute.IsTargetAttribute(this, AttributeDescription.FieldOffsetAttribute))
{
Expand All @@ -189,12 +189,12 @@ internal override void DecodeWellKnownAttribute(ref DecodeWellKnownAttributeArgu

// Set field offset even if the attribute specifies an invalid value, so that
// post-validation knows that the attribute is applied and reports better errors.
arguments.GetOrCreateData<CommonFieldWellKnownAttributeData>().SetFieldOffset(offset);
arguments.GetOrCreateData<FieldWellKnownAttributeData>().SetFieldOffset(offset);
}
}
else if (attribute.IsTargetAttribute(this, AttributeDescription.MarshalAsAttribute))
{
MarshalAsAttributeDecoder<CommonFieldWellKnownAttributeData, AttributeSyntax, CSharpAttributeData, AttributeLocation>.Decode(ref arguments, AttributeTargets.Field, MessageProvider.Instance);
MarshalAsAttributeDecoder<FieldWellKnownAttributeData, AttributeSyntax, CSharpAttributeData, AttributeLocation>.Decode(ref arguments, AttributeTargets.Field, MessageProvider.Instance);
}
else if (attribute.IsTargetAttribute(this, AttributeDescription.DynamicAttribute))
{
Expand Down Expand Up @@ -233,6 +233,38 @@ internal override void DecodeWellKnownAttribute(ref DecodeWellKnownAttributeArgu
// NullableAttribute should not be set explicitly.
arguments.Diagnostics.Add(ErrorCode.ERR_ExplicitNullableAttribute, arguments.AttributeSyntaxOpt.Location);
}
else if (attribute.IsTargetAttribute(this, AttributeDescription.AllowNullAttribute))
{
arguments.GetOrCreateData<FieldWellKnownAttributeData>().HasAllowNullAttribute = true;
}
else if (attribute.IsTargetAttribute(this, AttributeDescription.DisallowNullAttribute))
{
arguments.GetOrCreateData<FieldWellKnownAttributeData>().HasDisallowNullAttribute = true;
}
else if (attribute.IsTargetAttribute(this, AttributeDescription.MaybeNullAttribute))
{
arguments.GetOrCreateData<FieldWellKnownAttributeData>().HasMaybeNullAttribute = true;
}
else if (attribute.IsTargetAttribute(this, AttributeDescription.NotNullAttribute))
{
arguments.GetOrCreateData<FieldWellKnownAttributeData>().HasNotNullAttribute = true;
}
}

public override FlowAnalysisAnnotations FlowAnalysisAnnotations
=> DecodeFlowAnalysisAttributes(GetDecodedWellKnownAttributeData());

private static FlowAnalysisAnnotations DecodeFlowAnalysisAttributes(FieldWellKnownAttributeData attributeData)
{
var annotations = FlowAnalysisAnnotations.None;
if (attributeData != null)
{
if (attributeData.HasAllowNullAttribute) annotations |= FlowAnalysisAnnotations.AllowNull;
if (attributeData.HasDisallowNullAttribute) annotations |= FlowAnalysisAnnotations.DisallowNull;
if (attributeData.HasMaybeNullAttribute) annotations |= FlowAnalysisAnnotations.MaybeNull;
if (attributeData.HasNotNullAttribute) annotations |= FlowAnalysisAnnotations.NotNull;
}
return annotations;
}

/// <summary>
Expand All @@ -244,7 +276,7 @@ private void VerifyConstantValueMatches(ConstantValue attrValue, ref DecodeWellK
{
if (!attrValue.IsBad)
{
var data = arguments.GetOrCreateData<CommonFieldWellKnownAttributeData>();
var data = arguments.GetOrCreateData<FieldWellKnownAttributeData>();
ConstantValue constValue;

if (this.IsConst)
Expand Down Expand Up @@ -296,7 +328,7 @@ internal override void PostDecodeWellKnownAttributes(ImmutableArray<CSharpAttrib
Debug.Assert(_lazyCustomAttributesBag.IsDecodedWellKnownAttributeDataComputed);
Debug.Assert(symbolPart == AttributeLocation.None);

var data = (CommonFieldWellKnownAttributeData)decodedData;
var data = (FieldWellKnownAttributeData)decodedData;
int? fieldOffset = data != null ? data.Offset : null;

if (fieldOffset.HasValue)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r

internal abstract override TypeWithAnnotations GetFieldType(ConsList<FieldSymbol> fieldsBeingBound);

public override FlowAnalysisAnnotations FlowAnalysisAnnotations
=> FlowAnalysisAnnotations.None;

public override string Name
{
get { return _name; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ public override bool IsImplicitlyDeclared
get { return _underlyingField.IsImplicitlyDeclared; }
}

public override FlowAnalysisAnnotations FlowAnalysisAnnotations
{
get { return _underlyingField.FlowAnalysisAnnotations; }
}

public override Accessibility DeclaredAccessibility
{
get
Expand Down
Loading