Skip to content

Commit

Permalink
Revert "Pack bits in SourceOrdinaryMethodSymbol into an existing bitf…
Browse files Browse the repository at this point in the history
…lag structure we have for all source methods (dotnet#68132)"

This reverts commit e4c723e.
  • Loading branch information
CyrusNajmabadi authored May 10, 2023
1 parent e123745 commit d51ec38
Show file tree
Hide file tree
Showing 16 changed files with 116 additions and 107 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ protected override ImmutableArray<TypeSymbol> ExtraSynthesizedRefParameters

internal override bool InheritsBaseMethodAttributes => true;
internal override bool GenerateDebugInfo => !this.IsAsync;
internal override bool IsExpressionBodied => false;

internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree localTree)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,7 @@ protected SynthesizedMethodBaseSymbol(NamedTypeSymbol containingType,
returnsVoid: baseMethod.ReturnsVoid,
isExtensionMethod: false,
isNullableAnalysisEnabled: false,
isMetadataVirtualIgnoringModifiers: false,
isExpressionBodied: false);
isMetadataVirtualIgnoringModifiers: false);
}

protected void AssignTypeMapAndTypeParameters(TypeMap typeMap, ImmutableArray<TypeParameterSymbol> typeParameters)
Expand Down Expand Up @@ -227,5 +226,10 @@ public sealed override bool IsImplicitlyDeclared
{
get { return true; }
}

internal override bool IsExpressionBodied
{
get { return false; }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
internal sealed class SourceConstructorSymbol : SourceConstructorSymbolBase
{
private readonly bool _isExpressionBodied;
private readonly bool _hasThisInitializer;

public static SourceConstructorSymbol CreateConstructorSymbol(
Expand All @@ -34,14 +35,14 @@ private SourceConstructorSymbol(
base(containingType, location, syntax, SyntaxFacts.HasYieldOperations(syntax))
{
bool hasBlockBody = syntax.Body != null;
bool isExpressionBodied = !hasBlockBody && syntax.ExpressionBody != null;
bool hasBody = hasBlockBody || isExpressionBodied;
_isExpressionBodied = !hasBlockBody && syntax.ExpressionBody != null;
bool hasBody = hasBlockBody || _isExpressionBodied;

_hasThisInitializer = syntax.Initializer?.Kind() == SyntaxKind.ThisConstructorInitializer;

bool modifierErrors;
var declarationModifiers = this.MakeModifiers(syntax.Modifiers, methodKind, hasBody, location, diagnostics, out modifierErrors);
this.MakeFlags(methodKind, declarationModifiers, returnsVoid: true, isExpressionBodied: isExpressionBodied, isExtensionMethod: false, isNullableAnalysisEnabled: isNullableAnalysisEnabled);
this.MakeFlags(methodKind, declarationModifiers, returnsVoid: true, isExtensionMethod: false, isNullableAnalysisEnabled: isNullableAnalysisEnabled);

if (syntax.Identifier.ValueText != containingType.Name)
{
Expand Down Expand Up @@ -162,6 +163,14 @@ internal override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclara
return OneOrMany.Create(((ConstructorDeclarationSyntax)this.SyntaxNode).AttributeLists);
}

internal override bool IsExpressionBodied
{
get
{
return _isExpressionBodied;
}
}

internal override bool IsNullableAnalysisEnabled()
{
return _hasThisInitializer ?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#nullable disable

using System;
using System.Collections.Immutable;
using System.Diagnostics;
using Microsoft.CodeAnalysis.CSharp.Syntax;
Expand Down Expand Up @@ -33,8 +32,7 @@ internal SourceCustomEventAccessorSymbol(
syntax.Keyword.GetLocation(), explicitlyImplementedEventOpt, aliasQualifierOpt,
isAdder: syntax.Kind() == SyntaxKind.AddAccessorDeclaration,
isIterator: SyntaxFacts.HasYieldOperations(syntax.Body),
isNullableAnalysisEnabled: isNullableAnalysisEnabled,
isExpressionBodied: syntax is { Body: null, ExpressionBody: not null })
isNullableAnalysisEnabled: isNullableAnalysisEnabled)
{
Debug.Assert(syntax != null);
Debug.Assert(syntax.Kind() == SyntaxKind.AddAccessorDeclaration || syntax.Kind() == SyntaxKind.RemoveAccessorDeclaration);
Expand Down Expand Up @@ -93,5 +91,16 @@ internal override bool GenerateDebugInfo
{
get { return true; }
}

internal override bool IsExpressionBodied
{
get
{
var syntax = GetSyntax();
var hasBody = syntax.Body != null;
var hasExpressionBody = syntax.ExpressionBody != null;
return !hasBody && hasExpressionBody;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ protected SourceDelegateMethodSymbol(
: base(delegateType, syntax.GetReference(), location: syntax.Identifier.GetLocation(), isIterator: false)
{
_returnType = returnType;
this.MakeFlags(
methodKind, declarationModifiers, _returnType.IsVoidType(), isExpressionBodied: false, isExtensionMethod: false, isNullableAnalysisEnabled: false);
this.MakeFlags(methodKind, declarationModifiers, _returnType.IsVoidType(), isExtensionMethod: false, isNullableAnalysisEnabled: false);
}

internal sealed override ExecutableCodeBinder TryGetBodyBinder(BinderFactory binderFactoryOpt = null, bool ignoreAccessibility = false) => throw ExceptionUtilities.Unreachable();
Expand Down Expand Up @@ -177,6 +176,11 @@ public sealed override bool IsImplicitlyDeclared
}
}

internal override bool IsExpressionBodied
{
get { return false; }
}

internal override bool GenerateDebugInfo
{
get { return false; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
internal sealed class SourceDestructorSymbol : SourceMemberMethodSymbol
{
private TypeWithAnnotations _lazyReturnType;
private readonly bool _isExpressionBodied;

internal SourceDestructorSymbol(
SourceMemberContainerTypeSymbol containingType,
Expand All @@ -27,26 +28,25 @@ internal SourceDestructorSymbol(

bool modifierErrors;
var declarationModifiers = MakeModifiers(syntax.Modifiers, location, diagnostics, out modifierErrors);

bool hasBlockBody = syntax.Body != null;
bool isExpressionBodied = !hasBlockBody && syntax.ExpressionBody != null;

this.MakeFlags(methodKind, declarationModifiers, returnsVoid: true, isExpressionBodied: isExpressionBodied, isExtensionMethod: false, isNullableAnalysisEnabled: isNullableAnalysisEnabled);
this.MakeFlags(methodKind, declarationModifiers, returnsVoid: true, isExtensionMethod: false, isNullableAnalysisEnabled: isNullableAnalysisEnabled);

if (syntax.Identifier.ValueText != containingType.Name)
{
diagnostics.Add(ErrorCode.ERR_BadDestructorName, syntax.Identifier.GetLocation());
}

if (hasBlockBody || isExpressionBodied)
bool hasBlockBody = syntax.Body != null;
_isExpressionBodied = !hasBlockBody && syntax.ExpressionBody != null;

if (hasBlockBody || _isExpressionBodied)
{
if (IsExtern)
{
diagnostics.Add(ErrorCode.ERR_ExternHasBody, location, this);
}
}

if (!modifierErrors && !hasBlockBody && !isExpressionBodied && !IsExtern)
if (!modifierErrors && !hasBlockBody && !_isExpressionBodied && !IsExtern)
{
diagnostics.Add(ErrorCode.ERR_ConcreteMissingBody, location, this);
}
Expand Down Expand Up @@ -142,6 +142,14 @@ public override string Name
get { return WellKnownMemberNames.DestructorName; }
}

internal override bool IsExpressionBodied
{
get
{
return _isExpressionBodied;
}
}

internal override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarations()
{
// destructors can't have return type attributes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ public SourceEventAccessorSymbol(
string aliasQualifierOpt,
bool isAdder,
bool isIterator,
bool isNullableAnalysisEnabled,
bool isExpressionBodied)
bool isNullableAnalysisEnabled)
: base(@event.containingType, syntaxReference, location, isIterator)
{
_event = @event;
Expand All @@ -57,7 +56,6 @@ public SourceEventAccessorSymbol(
isAdder ? MethodKind.EventAdd : MethodKind.EventRemove,
@event.Modifiers,
returnsVoid: false, // until we learn otherwise (in LazyMethodChecks).
isExpressionBodied: isExpressionBodied,
isExtensionMethod: false,
isNullableAnalysisEnabled: isNullableAnalysisEnabled,
isMetadataVirtualIgnoringModifiers: @event.IsExplicitInterfaceImplementation && (@event.Modifiers & DeclarationModifiers.Static) == 0);
Expand Down Expand Up @@ -248,5 +246,11 @@ protected string GetOverriddenAccessorName(SourceEventSymbol @event, bool isAdde

return null;
}

internal override bool IsExpressionBodied
{
// Events cannot be expression-bodied
get { return false; }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,31 +24,22 @@ protected struct Flags
{
// We currently pack everything into a 32 bit int with the following layout:
//
// | |a|b|e|n|vvv|yy|s|r|q|z|kk|wwwww|
// | |n|vvv|yy|s|r|q|z|wwwww|
//
// w = method kind. 5 bits.
// k = ref kind. 2 bits.
// z = isExtensionMethod. 1 bit.
// q = isMetadataVirtualIgnoringInterfaceChanges. 1 bit.
// r = isMetadataVirtual. 1 bit. (At least as true as isMetadataVirtualIgnoringInterfaceChanges.)
// s = isMetadataVirtualLocked. 1 bit.
// y = ReturnsVoid. 2 bits.
// v = NullableContext. 3 bits.
// n = IsNullableAnalysisEnabled. 1 bit.
// e = IsExpressionBody. 1 bit.
// b = HasAnyBody. 1 bit.
// a = IsVarArg. 1 bit
private int _flags;

private const int MethodKindOffset = 0;
private const int MethodKindSize = 5;
private const int MethodKindMask = (1 << MethodKindSize) - 1;

private const int RefKindOffset = MethodKindOffset + MethodKindSize;
private const int RefKindSize = 2;
private const int RefKindMask = (1 << RefKindSize) - 1;

private const int IsExtensionMethodOffset = RefKindOffset + RefKindSize;
private const int IsExtensionMethodOffset = MethodKindOffset + MethodKindSize;
private const int IsExtensionMethodSize = 1;

private const int IsMetadataVirtualIgnoringInterfaceChangesOffset = IsExtensionMethodOffset + IsExtensionMethodSize;
Expand All @@ -65,33 +56,24 @@ protected struct Flags

private const int NullableContextOffset = ReturnsVoidOffset + ReturnsVoidSize;
private const int NullableContextSize = 3;
private const int NullableContextMask = (1 << NullableContextSize) - 1;

private const int IsNullableAnalysisEnabledOffset = NullableContextOffset + NullableContextSize;
private const int IsNullableAnalysisEnabledSize = 1;

private const int IsExpressionBodiedOffset = IsNullableAnalysisEnabledOffset + IsNullableAnalysisEnabledSize;
private const int IsExpressionBodiedSize = 1;

private const int HasAnyBodyOffset = IsExpressionBodiedOffset + IsExpressionBodiedSize;
private const int HasAnyBodySize = 1;

private const int IsVarArgOffset = HasAnyBodyOffset + HasAnyBodySize;
#pragma warning disable IDE0051 // Remove unused private members
private const int IsVarArgSize = 1;
private const int IsNullableAnalysisEnabledSize = 1;
#pragma warning restore IDE0051 // Remove unused private members

private const int HasAnyBodyBit = 1 << HasAnyBodyOffset;
private const int IsExpressionBodiedBit = 1 << IsExpressionBodiedOffset;
private const int MethodKindMask = (1 << MethodKindSize) - 1;

private const int IsExtensionMethodBit = 1 << IsExtensionMethodOffset;
private const int IsMetadataVirtualIgnoringInterfaceChangesBit = 1 << IsMetadataVirtualIgnoringInterfaceChangesOffset;
private const int IsMetadataVirtualBit = 1 << IsMetadataVirtualIgnoringInterfaceChangesOffset;
private const int IsMetadataVirtualLockedBit = 1 << IsMetadataVirtualLockedOffset;
private const int IsVarArgBit = 1 << IsVarArgOffset;

private const int ReturnsVoidBit = 1 << ReturnsVoidOffset;
private const int ReturnsVoidIsSetBit = 1 << ReturnsVoidOffset + 1;

private const int NullableContextMask = (1 << NullableContextSize) - 1;

private const int IsNullableAnalysisEnabledBit = 1 << IsNullableAnalysisEnabledOffset;

public bool TryGetReturnsVoid(out bool value)
Expand All @@ -111,25 +93,9 @@ public MethodKind MethodKind
get { return (MethodKind)((_flags >> MethodKindOffset) & MethodKindMask); }
}

public RefKind RefKind
{
get { return (RefKind)((_flags >> RefKindOffset) & RefKindMask); }
}

public bool HasAnyBody
{
get { return (_flags & HasAnyBodyBit) != 0; }
}

public bool IsExpressionBodied
{
get { return (_flags & IsExpressionBodiedBit) != 0; }
}

public bool IsExtensionMethod
{
get { return (_flags & IsExtensionMethodBit) != 0; }
set { ThreadSafeFlagOperations.Set(ref _flags, value ? IsExtensionMethodBit : 0); }
}

public bool IsNullableAnalysisEnabled
Expand All @@ -142,18 +108,11 @@ public bool IsMetadataVirtualLocked
get { return (_flags & IsMetadataVirtualLockedBit) != 0; }
}

public bool IsVarArg
{
get { return (_flags & IsVarArgBit) != 0; }
set { ThreadSafeFlagOperations.Set(ref _flags, value ? IsVarArgBit : 0); }
}

#if DEBUG
static Flags()
{
// Verify masks are sufficient for values.
Debug.Assert(EnumUtilities.ContainsAllValues<MethodKind>(MethodKindMask));
Debug.Assert(EnumUtilities.ContainsAllValues<RefKind>(RefKindMask));
Debug.Assert(EnumUtilities.ContainsAllValues<NullableContextKind>(NullableContextMask));
}
#endif
Expand All @@ -167,22 +126,19 @@ public Flags(
MethodKind methodKind,
DeclarationModifiers declarationModifiers,
bool returnsVoid,
bool isExpressionBodied,
bool isExtensionMethod,
bool isNullableAnalysisEnabled,
bool isMetadataVirtualIgnoringModifiers = false)
{
bool isMetadataVirtual = isMetadataVirtualIgnoringModifiers || ModifiersRequireMetadataVirtual(declarationModifiers);

int methodKindInt = ((int)methodKind & MethodKindMask) << MethodKindOffset;
int isExpressionBodyInt = isExpressionBodied ? IsExpressionBodiedBit : 0;
int isExtensionMethodInt = isExtensionMethod ? IsExtensionMethodBit : 0;
int isNullableAnalysisEnabledInt = isNullableAnalysisEnabled ? IsNullableAnalysisEnabledBit : 0;
int isMetadataVirtualIgnoringInterfaceImplementationChangesInt = isMetadataVirtual ? IsMetadataVirtualIgnoringInterfaceChangesBit : 0;
int isMetadataVirtualInt = isMetadataVirtual ? IsMetadataVirtualBit : 0;

_flags = methodKindInt
| isExpressionBodyInt
| isExtensionMethodInt
| isNullableAnalysisEnabledInt
| isMetadataVirtualIgnoringInterfaceImplementationChangesInt
Expand All @@ -191,16 +147,6 @@ public Flags(
| ReturnsVoidIsSetBit;
}

/// <summary>
/// Only allowed to be called in constructors of SourceMemberMethodSymbol (and derived constructors), so
/// does not need ThreadSafe operations.
/// </summary>
public void SetOrdinaryMethodFlags(RefKind refKind, bool hasAnyBody)
{
_flags |= ((int)refKind & RefKindMask) << RefKindOffset;
_flags |= hasAnyBody ? HasAnyBodyBit : 0;
}

public bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false)
{
// This flag is immutable, so there's no reason to set a lock bit, as we do below.
Expand Down Expand Up @@ -346,13 +292,12 @@ protected void MakeFlags(
MethodKind methodKind,
DeclarationModifiers declarationModifiers,
bool returnsVoid,
bool isExpressionBodied,
bool isExtensionMethod,
bool isNullableAnalysisEnabled,
bool isMetadataVirtualIgnoringModifiers = false)
{
DeclarationModifiers = declarationModifiers;
this.flags = new Flags(methodKind, declarationModifiers, returnsVoid, isExpressionBodied, isExtensionMethod, isNullableAnalysisEnabled, isMetadataVirtualIgnoringModifiers);
this.flags = new Flags(methodKind, declarationModifiers, returnsVoid, isExtensionMethod, isNullableAnalysisEnabled, isMetadataVirtualIgnoringModifiers);
}

protected void SetReturnsVoid(bool returnsVoid)
Expand Down Expand Up @@ -1058,7 +1003,7 @@ protected void CheckFeatureAvailabilityAndRuntimeSupport(SyntaxNode declarationS
/// If the method has both block body and an expression body
/// present, this is not treated as expression-bodied.
/// </remarks>
internal bool IsExpressionBodied => flags.IsExpressionBodied;
internal abstract bool IsExpressionBodied { get; }

internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree localTree)
{
Expand Down
Loading

0 comments on commit d51ec38

Please sign in to comment.