Skip to content

Commit

Permalink
Mechanical revert of InterceptableAttribute implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
RikkiGibson committed May 16, 2023
1 parent f130bb4 commit 5cd9e59
Show file tree
Hide file tree
Showing 42 changed files with 44 additions and 316 deletions.
6 changes: 0 additions & 6 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -7505,12 +7505,6 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_BadCaseInSwitchArm" xml:space="preserve">
<value>A switch expression arm does not begin with a 'case' keyword.</value>
</data>
<data name="WRN_CallNotInterceptable" xml:space="preserve">
<value>Call to '{0}' is intercepted, but the method is not marked with 'System.Runtime.CompilerServices.InterceptableAttribute'.</value>
</data>
<data name="WRN_CallNotInterceptable_Title" xml:space="preserve">
<value>Call to a method is intercepted, but the method is not marked with 'System.Runtime.CompilerServices.InterceptableAttribute'.</value>
</data>
<data name="ERR_InterceptorCannotBeGeneric" xml:space="preserve">
<value>Method '{0}' cannot be used as an interceptor because it or its containing type has type parameters.</value>
</data>
Expand Down
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2193,7 +2193,6 @@ internal enum ErrorCode
ERR_UnsupportedPrimaryConstructorParameterCapturingRefAny = 9136,

// PROTOTYPE(ic): pack errors
WRN_CallNotInterceptable = 27000,
ERR_InterceptorCannotBeGeneric = 27001,
ERR_InterceptorPathNotInCompilation = 27002,
ERR_InterceptorPathNotInCompilationWithCandidate = 27003,
Expand Down
2 changes: 0 additions & 2 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,6 @@ internal static int GetWarningLevel(ErrorCode code)
case ErrorCode.WRN_ParamsArrayInLambdaOnly:
case ErrorCode.WRN_CapturedPrimaryConstructorParameterPassedToBase:
case ErrorCode.WRN_UnreadPrimaryConstructorParameter:
case ErrorCode.WRN_CallNotInterceptable:
case ErrorCode.WRN_InterceptorSignatureMismatch:
case ErrorCode.WRN_NullabilityMismatchInReturnTypeOnInterceptor:
case ErrorCode.WRN_NullabilityMismatchInParameterTypeOnInterceptor:
Expand Down Expand Up @@ -585,7 +584,6 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_EncUpdateFailedDelegateTypeChanged:
case ErrorCode.ERR_CannotBeConvertedToUtf8:
case ErrorCode.ERR_FileTypeNonUniquePath:
case ErrorCode.WRN_CallNotInterceptable:
case ErrorCode.ERR_InterceptorSignatureMismatch:
case ErrorCode.ERR_InterceptorMustHaveMatchingThisParameter:
case ErrorCode.ERR_InterceptorMustNotHaveThisParameter:
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -149,21 +149,5 @@ public UnmanagedCallersOnlyAttributeData? UnmanagedCallersOnlyAttributeData
SetDataStored();
}
}

private bool _hasInterceptableAttribute;
public bool HasInterceptableAttribute
{
get
{
VerifySealed(expected: true);
return _hasInterceptableAttribute;
}
set
{
VerifySealed(expected: false);
_hasInterceptableAttribute = value;
SetDataStored();
}
}
}
}
2 changes: 0 additions & 2 deletions src/Compilers/CSharp/Portable/Symbols/ErrorMethodSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,6 @@ internal override bool GenerateDebugInfo

internal sealed override bool IsNullableAnalysisEnabled() => false;

internal override bool IsInterceptable => false;

protected override bool HasSetsRequiredMembersImpl
{
get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -831,7 +831,6 @@ public override bool IsVararg
public override FlowAnalysisAnnotations FlowAnalysisAnnotations => FlowAnalysisAnnotations.None;
internal override bool IsMetadataNewSlot(bool ignoreInterfaceImplementationChanges = false) => false;
internal override bool IsMetadataVirtual(bool ignoreInterfaceImplementationChanges = false) => false;
internal sealed override bool IsInterceptable => false;
internal sealed override UnmanagedCallersOnlyAttributeData? GetUnmanagedCallersOnlyAttributeData(bool forceComplete) => null;

internal override bool GenerateDebugInfo => throw ExceptionUtilities.Unreachable();
Expand Down
126 changes: 44 additions & 82 deletions src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,10 @@ public SignatureData(SignatureHeader header, ImmutableArray<ParameterSymbol> par
// This type is used to compact many different bits of information efficiently.
private struct PackedFlags
{
// We currently pack everything into a 64-bit long with the following layout:
// We currently pack everything into a 32-bit int with the following layout:
//
// |________|________|________|_______|A|z|y|x|w|v|u|t|s|r|q|p|ooo|n|m|l|k|j|i|h|g|f|e|d|c|b|aaaaa|
// |y|x|w|v|u|t|s|r|q|p|ooo|n|m|l|k|j|i|h|g|f|e|d|c|b|aaaaa|
//
// _ = unused
//
// a = method kind. 5 bits.
// b = method kind populated. 1 bit.
//
Expand Down Expand Up @@ -78,49 +76,38 @@ private struct PackedFlags
// w = IsSetsRequiredMembersPopulated. 1 bit.
// x = IsUnscopedRef. 1 bit.
// y = IsUnscopedRefPopulated. 1 bit.
// z = IsInterceptable. 1 bit.
// A = IsInterceptablePopulated. 1 bit.
// 31 bits remain for future purposes.

// PROTOTYPE(ic): consider reverting back to 'int' for these flags.
// - There may not be a need to cache 'IsInterceptable' on the symbol. We could just look it up from attributes each time.
// Since the number of calls which are intercepted is relatively small, this may be a negligible additional cost.
// - Otherwise, there may be more space efficient method of storing the flags which we should be using instead.
// For example, writing a byte is atomic, so if multiple *bits of information* aren't being stored independently in it,
// we could store 'IsInterceptable' in a 'ThreeState' field, for example.
// 2 bits remain for future purposes.

private const int MethodKindOffset = 0;
private const long MethodKindMask = 0x1F;

private const long MethodKindIsPopulatedBit = 0x1L << 5;
private const long IsExtensionMethodBit = 0x1L << 6;
private const long IsExtensionMethodIsPopulatedBit = 0x1L << 7;
private const long IsExplicitFinalizerOverrideBit = 0x1L << 8;
private const long IsExplicitClassOverrideBit = 0x1L << 9;
private const long IsExplicitOverrideIsPopulatedBit = 0x1L << 10;
private const long IsObsoleteAttributePopulatedBit = 0x1L << 11;
private const long IsCustomAttributesPopulatedBit = 0x1L << 12;
private const long IsUseSiteDiagnosticPopulatedBit = 0x1L << 13;
private const long IsConditionalPopulatedBit = 0x1L << 14;
private const long IsOverriddenOrHiddenMembersPopulatedBit = 0x1L << 15;
private const long IsReadOnlyBit = 0x1L << 16;
private const long IsReadOnlyPopulatedBit = 0x1L << 17;
private const int MethodKindMask = 0x1F;

private const int MethodKindIsPopulatedBit = 0x1 << 5;
private const int IsExtensionMethodBit = 0x1 << 6;
private const int IsExtensionMethodIsPopulatedBit = 0x1 << 7;
private const int IsExplicitFinalizerOverrideBit = 0x1 << 8;
private const int IsExplicitClassOverrideBit = 0x1 << 9;
private const int IsExplicitOverrideIsPopulatedBit = 0x1 << 10;
private const int IsObsoleteAttributePopulatedBit = 0x1 << 11;
private const int IsCustomAttributesPopulatedBit = 0x1 << 12;
private const int IsUseSiteDiagnosticPopulatedBit = 0x1 << 13;
private const int IsConditionalPopulatedBit = 0x1 << 14;
private const int IsOverriddenOrHiddenMembersPopulatedBit = 0x1 << 15;
private const int IsReadOnlyBit = 0x1 << 16;
private const int IsReadOnlyPopulatedBit = 0x1 << 17;
private const int NullableContextOffset = 18;
private const long NullableContextMask = 0x7;
private const long DoesNotReturnBit = 0x1L << 21;
private const long IsDoesNotReturnPopulatedBit = 0x1L << 22;
private const long IsMemberNotNullPopulatedBit = 0x1L << 23;
private const long IsInitOnlyBit = 0x1L << 24;
private const long IsInitOnlyPopulatedBit = 0x1L << 25;
private const long IsUnmanagedCallersOnlyAttributePopulatedBit = 0x1L << 26;
private const long HasSetsRequiredMembersBit = 0x1L << 27;
private const long HasSetsRequiredMembersPopulatedBit = 0x1L << 28;
private const long IsUnscopedRefBit = 0x1L << 29;
private const long IsUnscopedRefPopulatedBit = 0x1L << 30;
private const long IsInterceptableBit = 0x1L << 31;
private const long IsInterceptablePopulatedBit = 0x1L << 32;

private long _bits;
private const int NullableContextMask = 0x7;
private const int DoesNotReturnBit = 0x1 << 21;
private const int IsDoesNotReturnPopulatedBit = 0x1 << 22;
private const int IsMemberNotNullPopulatedBit = 0x1 << 23;
private const int IsInitOnlyBit = 0x1 << 24;
private const int IsInitOnlyPopulatedBit = 0x1 << 25;
private const int IsUnmanagedCallersOnlyAttributePopulatedBit = 0x1 << 26;
private const int HasSetsRequiredMembersBit = 0x1 << 27;
private const int HasSetsRequiredMembersPopulatedBit = 0x1 << 28;
private const int IsUnscopedRefBit = 0x1 << 29;
private const int IsUnscopedRefPopulatedBit = 0x1 << 30;

private int _bits;

public MethodKind MethodKind
{
Expand All @@ -131,8 +118,8 @@ public MethodKind MethodKind

set
{
Debug.Assert((long)value == ((long)value & MethodKindMask));
_bits = (_bits & ~(MethodKindMask << MethodKindOffset)) | (((long)value & MethodKindMask) << MethodKindOffset) | MethodKindIsPopulatedBit;
Debug.Assert((int)value == ((int)value & MethodKindMask));
_bits = (_bits & ~(MethodKindMask << MethodKindOffset)) | (((int)value & MethodKindMask) << MethodKindOffset) | MethodKindIsPopulatedBit;
}
}

Expand All @@ -159,8 +146,6 @@ public MethodKind MethodKind
public bool HasSetsRequiredMembersPopulated => (_bits & HasSetsRequiredMembersPopulatedBit) != 0;
public bool IsUnscopedRef => (_bits & IsUnscopedRefBit) != 0;
public bool IsUnscopedRefPopulated => (_bits & IsUnscopedRefPopulatedBit) != 0;
public bool IsInterceptable => (_bits & IsInterceptableBit) != 0;
public bool IsInterceptablePopulated => (_bits & IsInterceptablePopulatedBit) != 0;

#if DEBUG
static PackedFlags()
Expand All @@ -171,36 +156,36 @@ static PackedFlags()
}
#endif

private static bool BitsAreUnsetOrSame(long bits, long mask)
private static bool BitsAreUnsetOrSame(int bits, int mask)
{
return (bits & mask) == 0 || (bits & mask) == mask;
}

public void InitializeIsExtensionMethod(bool isExtensionMethod)
{
var bitsToSet = (isExtensionMethod ? IsExtensionMethodBit : 0) | IsExtensionMethodIsPopulatedBit;
int bitsToSet = (isExtensionMethod ? IsExtensionMethodBit : 0) | IsExtensionMethodIsPopulatedBit;
Debug.Assert(BitsAreUnsetOrSame(_bits, bitsToSet));
ThreadSafeFlagOperations.Set(ref _bits, bitsToSet);
}

public void InitializeIsReadOnly(bool isReadOnly)
{
var bitsToSet = (isReadOnly ? IsReadOnlyBit : 0) | IsReadOnlyPopulatedBit;
int bitsToSet = (isReadOnly ? IsReadOnlyBit : 0) | IsReadOnlyPopulatedBit;
Debug.Assert(BitsAreUnsetOrSame(_bits, bitsToSet));
ThreadSafeFlagOperations.Set(ref _bits, bitsToSet);
}

public void InitializeMethodKind(MethodKind methodKind)
{
Debug.Assert((long)methodKind == ((long)methodKind & MethodKindMask));
var bitsToSet = (((long)methodKind & MethodKindMask) << MethodKindOffset) | MethodKindIsPopulatedBit;
Debug.Assert((int)methodKind == ((int)methodKind & MethodKindMask));
int bitsToSet = (((int)methodKind & MethodKindMask) << MethodKindOffset) | MethodKindIsPopulatedBit;
Debug.Assert(BitsAreUnsetOrSame(_bits, bitsToSet));
ThreadSafeFlagOperations.Set(ref _bits, bitsToSet);
}

public void InitializeIsExplicitOverride(bool isExplicitFinalizerOverride, bool isExplicitClassOverride)
{
var bitsToSet =
int bitsToSet =
(isExplicitFinalizerOverride ? IsExplicitFinalizerOverrideBit : 0) |
(isExplicitClassOverride ? IsExplicitClassOverrideBit : 0) |
IsExplicitOverrideIsPopulatedBit;
Expand Down Expand Up @@ -240,12 +225,12 @@ public bool TryGetNullableContext(out byte? value)

public bool SetNullableContext(byte? value)
{
return ThreadSafeFlagOperations.Set(ref _bits, (((long)value.ToNullableContextFlags() & NullableContextMask) << NullableContextOffset));
return ThreadSafeFlagOperations.Set(ref _bits, (((int)value.ToNullableContextFlags() & NullableContextMask) << NullableContextOffset));
}

public bool InitializeDoesNotReturn(bool value)
{
var bitsToSet = IsDoesNotReturnPopulatedBit;
int bitsToSet = IsDoesNotReturnPopulatedBit;
if (value) bitsToSet |= DoesNotReturnBit;

return ThreadSafeFlagOperations.Set(ref _bits, bitsToSet);
Expand All @@ -258,7 +243,7 @@ public void SetIsMemberNotNullPopulated()

public void InitializeIsInitOnly(bool isInitOnly)
{
var bitsToSet = (isInitOnly ? IsInitOnlyBit : 0) | IsInitOnlyPopulatedBit;
int bitsToSet = (isInitOnly ? IsInitOnlyBit : 0) | IsInitOnlyPopulatedBit;
Debug.Assert(BitsAreUnsetOrSame(_bits, bitsToSet));
ThreadSafeFlagOperations.Set(ref _bits, bitsToSet);
}
Expand All @@ -270,27 +255,19 @@ public void SetIsUnmanagedCallersOnlyAttributePopulated()

public bool InitializeSetsRequiredMembersBit(bool value)
{
var bitsToSet = HasSetsRequiredMembersPopulatedBit;
int bitsToSet = HasSetsRequiredMembersPopulatedBit;
if (value) bitsToSet |= HasSetsRequiredMembersBit;

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

public bool InitializeIsUnscopedRef(bool value)
{
var bitsToSet = IsUnscopedRefPopulatedBit;
int bitsToSet = IsUnscopedRefPopulatedBit;
if (value) bitsToSet |= IsUnscopedRefBit;

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

public bool InitializeInterceptable(bool value)
{
var bitsToSet = IsInterceptablePopulatedBit;
if (value) bitsToSet |= IsInterceptableBit;

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

/// <summary>
Expand Down Expand Up @@ -1672,21 +1649,6 @@ internal sealed override bool HasUnscopedRefAttribute
}
}

internal sealed override bool IsInterceptable
{
get
{
if (!_packedFlags.IsInterceptablePopulated)
{
var moduleSymbol = _containingType.ContainingPEModule;
bool interceptable = moduleSymbol.Module.HasInterceptableAttribute(_handle);
_packedFlags.InitializeInterceptable(interceptable);
}

return _packedFlags.IsInterceptable;
}
}

internal sealed override bool UseUpdatedEscapeRules => ContainingModule.UseUpdatedEscapeRules;
}
}
5 changes: 0 additions & 5 deletions src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,6 @@ public virtual bool IsGenericMethod
/// </summary>
internal virtual bool IsDirectlyExcludedFromCodeCoverage { get => false; }

/// <summary>
/// True if the method is annotated with the `[Interceptable]` attribute.
/// </summary>
internal abstract bool IsInterceptable { get; }

/// <summary>
/// If a method is annotated with `[MemberNotNull(...)]` attributes, returns the list of members
/// listed in those attributes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,6 @@ public override ImmutableArray<ParameterSymbol> Parameters

internal override UnmanagedCallersOnlyAttributeData? GetUnmanagedCallersOnlyAttributeData(bool forceComplete) => UnderlyingMethod.GetUnmanagedCallersOnlyAttributeData(forceComplete);

internal override bool IsInterceptable => UnderlyingMethod.IsInterceptable;

public override Symbol? AssociatedSymbol => _associatedSymbol;

internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree localTree)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -580,8 +580,6 @@ internal override int CalculateLocalSyntaxOffset(int localPosition, SyntaxTree l

internal override bool IsNullableAnalysisEnabled() => throw ExceptionUtilities.Unreachable();

internal override bool IsInterceptable => throw ExceptionUtilities.Unreachable();

public override bool Equals(Symbol obj, TypeCompareKind compareKind)
{
if ((object)this == obj) return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,6 @@ public override ImmutableArray<CSharpAttributeData> GetReturnTypeAttributes()
return _lazyUnmanagedAttributeData;
}

internal override bool IsInterceptable => UnderlyingMethod.IsInterceptable;

internal override bool TryGetThisParameter(out ParameterSymbol? thisParameter)
{
if (!_underlyingMethod.TryGetThisParameter(out var underlyingParameter))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,6 @@ public SignatureOnlyMethodSymbol(

internal sealed override bool IsNullableAnalysisEnabled() => throw ExceptionUtilities.Unreachable();

internal sealed override bool IsInterceptable => throw ExceptionUtilities.Unreachable();

#region Not used by MethodSignatureComparer

internal override bool GenerateDebugInfo { get { throw ExceptionUtilities.Unreachable(); } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1394,8 +1394,6 @@ internal override bool HasSpecialName
internal sealed override bool IsDirectlyExcludedFromCodeCoverage =>
GetDecodedWellKnownAttributeData()?.HasExcludeFromCodeCoverageAttribute == true;

internal sealed override bool IsInterceptable => GetDecodedWellKnownAttributeData()?.HasInterceptableAttribute == true;

internal override bool RequiresSecurityObject
{
get
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,6 @@ public override ImmutableArray<CSharpAttributeData> GetReturnTypeAttributes()
internal sealed override UnmanagedCallersOnlyAttributeData GetUnmanagedCallersOnlyAttributeData(bool forceComplete)
=> this.OriginalDefinition.GetUnmanagedCallersOnlyAttributeData(forceComplete);

internal sealed override bool IsInterceptable => UnderlyingMethod.IsInterceptable;

public sealed override Symbol AssociatedSymbol
{
get
Expand Down
Loading

0 comments on commit 5cd9e59

Please sign in to comment.