-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Pack bits in SourceOrdinaryMethodSymbol into an existing bitflag structure we have for all source methods #68132
Changes from all commits
e1996ed
f626cdb
a233b06
853a8d2
613d0ac
98f167f
c743d52
73609c7
d22ed19
42120a9
6c9d5a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,8 @@ public SourceEventAccessorSymbol( | |
string aliasQualifierOpt, | ||
bool isAdder, | ||
bool isIterator, | ||
bool isNullableAnalysisEnabled) | ||
bool isNullableAnalysisEnabled, | ||
bool isExpressionBodied) | ||
: base(@event.containingType, syntaxReference, location, isIterator) | ||
{ | ||
_event = @event; | ||
|
@@ -56,6 +57,7 @@ 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); | ||
|
@@ -246,11 +248,5 @@ protected string GetOverriddenAccessorName(SourceEventSymbol @event, bool isAdde | |
|
||
return null; | ||
} | ||
|
||
internal override bool IsExpressionBodied | ||
{ | ||
// Events cannot be expression-bodied | ||
get { return false; } | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is just wrong (but was overridden, so it wasn't a problem). Event accessors can def be expression-bodied. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,22 +24,31 @@ protected struct Flags | |
{ | ||
// We currently pack everything into a 32 bit int with the following layout: | ||
// | ||
// | |n|vvv|yy|s|r|q|z|wwwww| | ||
// | |a|b|e|n|vvv|yy|s|r|q|z|kk|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 = MethodKindOffset + MethodKindSize; | ||
private const int IsExtensionMethodOffset = RefKindOffset + RefKindSize; | ||
private const int IsExtensionMethodSize = 1; | ||
|
||
private const int IsMetadataVirtualIgnoringInterfaceChangesOffset = IsExtensionMethodOffset + IsExtensionMethodSize; | ||
|
@@ -56,24 +65,33 @@ 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; | ||
#pragma warning disable IDE0051 // Remove unused private members | ||
private const int IsNullableAnalysisEnabledSize = 1; | ||
#pragma warning restore IDE0051 // Remove unused private members | ||
|
||
private const int MethodKindMask = (1 << MethodKindSize) - 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; | ||
#pragma warning restore IDE0051 // Remove unused private members | ||
|
||
private const int HasAnyBodyBit = 1 << HasAnyBodyOffset; | ||
private const int IsExpressionBodiedBit = 1 << IsExpressionBodiedOffset; | ||
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) | ||
|
@@ -93,9 +111,25 @@ 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 | ||
|
@@ -108,11 +142,18 @@ public bool IsMetadataVirtualLocked | |
get { return (_flags & IsMetadataVirtualLockedBit) != 0; } | ||
} | ||
|
||
public bool IsVarArg | ||
{ | ||
get { return (_flags & IsVarArgBit) != 0; } | ||
set { ThreadSafeFlagOperations.Set(ref _flags, value ? IsVarArgBit : 0); } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This doesn't look like the right thing to do. If the value is lazy calculated, then there should be a flag indicating that it has been calculated and an attempt to get the value should trigger the calculation. Alternatively, if the value is easy to calculate from the syntax, we should do that and store the final value from the start, eliminating the ability to adjust the value later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. I can move this to be computed up front. |
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: IsVarArg is currently lazily computed. But it woudl be trivial to compute up front in SourceOrdinaryMethod's constructor and just not have to deal with this. But i kept the logic as before. |
||
|
||
#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 | ||
|
@@ -126,19 +167,22 @@ 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 | ||
|
@@ -147,6 +191,16 @@ 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. | ||
|
@@ -292,12 +346,13 @@ 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, isExtensionMethod, isNullableAnalysisEnabled, isMetadataVirtualIgnoringModifiers); | ||
this.flags = new Flags(methodKind, declarationModifiers, returnsVoid, isExpressionBodied, isExtensionMethod, isNullableAnalysisEnabled, isMetadataVirtualIgnoringModifiers); | ||
} | ||
|
||
protected void SetReturnsVoid(bool returnsVoid) | ||
|
@@ -1003,7 +1058,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 abstract bool IsExpressionBodied { get; } | ||
internal bool IsExpressionBodied => flags.IsExpressionBodied; | ||
|
||
internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree localTree) | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
todo: consider adding a flag for this. would take away an unnecessary 32bit in a constructor.