-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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 #68158
Changes from all commits
3e67b7c
ce343a6
1319147
eee77ee
d2a9c02
824b5b1
1932ed3
9f7d914
a0295b7
27e28b7
54c18cf
d3a9476
c74ca1a
e73807b
2ad5f4c
5eb5236
08eee6e
5640b00
0e4f6fb
d02d68e
5d757af
94a74c9
fb5eaa9
48a1a41
d4bfe7e
43aab35
efe37fc
2e34e56
ff7192b
a27e807
c11f580
aa120d2
31a6277
fc9d571
77474b1
d3b2b40
b76e78e
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 |
---|---|---|
|
@@ -12,7 +12,6 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols | |
{ | ||
internal sealed class SourceConstructorSymbol : SourceConstructorSymbolBase | ||
{ | ||
private readonly bool _isExpressionBodied; | ||
private readonly bool _hasThisInitializer; | ||
|
||
public static SourceConstructorSymbol CreateConstructorSymbol( | ||
|
@@ -35,14 +34,16 @@ private SourceConstructorSymbol( | |
base(containingType, location, syntax, SyntaxFacts.HasYieldOperations(syntax)) | ||
{ | ||
bool hasBlockBody = syntax.Body != null; | ||
_isExpressionBodied = !hasBlockBody && syntax.ExpressionBody != null; | ||
bool hasBody = hasBlockBody || _isExpressionBodied; | ||
bool isExpressionBodied = !hasBlockBody && syntax.ExpressionBody != null; | ||
bool hasAnyBody = 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, isExtensionMethod: false, isNullableAnalysisEnabled: isNullableAnalysisEnabled); | ||
var declarationModifiers = this.MakeModifiers(syntax.Modifiers, methodKind, hasAnyBody, location, diagnostics, out modifierErrors); | ||
this.MakeFlags( | ||
methodKind, RefKind.None, declarationModifiers, returnsVoid: true, hasAnyBody: hasAnyBody, isExpressionBodied: isExpressionBodied, | ||
isExtensionMethod: false, isVarArg: syntax.ParameterList.Parameters.Any(static p => p.IsArgList), isNullableAnalysisEnabled: isNullableAnalysisEnabled); | ||
|
||
if (syntax.Identifier.ValueText != containingType.Name) | ||
{ | ||
|
@@ -57,22 +58,22 @@ private SourceConstructorSymbol( | |
diagnostics.Add(ErrorCode.ERR_ExternHasConstructorInitializer, location, this); | ||
} | ||
|
||
if (hasBody) | ||
if (hasAnyBody) | ||
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. different code used hasBody to mean different things. Some code uses it to mean "has a block body" other code uses it to mean "has a block or expression body". I've introduced a helper and have made this consistent. Now it's "hasBlockBody" and "hasAnyBody" to make it clear across all symbols. 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 was done as it actually caught bugs as i was doing this work. |
||
{ | ||
diagnostics.Add(ErrorCode.ERR_ExternHasBody, location, this); | ||
} | ||
} | ||
|
||
if (methodKind == MethodKind.StaticConstructor) | ||
{ | ||
CheckFeatureAvailabilityAndRuntimeSupport(syntax, location, hasBody, diagnostics); | ||
CheckFeatureAvailabilityAndRuntimeSupport(syntax, location, hasAnyBody, diagnostics); | ||
} | ||
|
||
ModifierUtils.CheckAccessibility(this.DeclarationModifiers, this, isExplicitInterfaceImplementation: false, diagnostics, location); | ||
|
||
if (!modifierErrors) | ||
{ | ||
this.CheckModifiers(methodKind, hasBody, location, diagnostics); | ||
this.CheckModifiers(methodKind, hasAnyBody, location, diagnostics); | ||
} | ||
|
||
CheckForBlockAndExpressionBody( | ||
|
@@ -163,14 +164,6 @@ 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 ? | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,6 @@ internal abstract class SourceConstructorSymbolBase : SourceMemberMethodSymbol | |
{ | ||
protected ImmutableArray<ParameterSymbol> _lazyParameters; | ||
private TypeWithAnnotations _lazyReturnType; | ||
private bool _lazyIsVararg; | ||
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 another nice 32bits of savings. |
||
|
||
protected SourceConstructorSymbolBase( | ||
SourceMemberContainerTypeSymbol containingType, | ||
|
@@ -48,15 +47,13 @@ protected sealed override void MethodChecks(BindingDiagnosticBag diagnostics) | |
// instance). Constraints are checked in AfterAddingTypeMembersChecks. | ||
var signatureBinder = bodyBinder.WithAdditionalFlagsAndContainingMemberOrLambda(BinderFlags.SuppressConstraintChecks, this); | ||
|
||
SyntaxToken arglistToken; | ||
_lazyParameters = ParameterHelpers.MakeParameters( | ||
signatureBinder, this, parameterList, out arglistToken, | ||
signatureBinder, this, parameterList, out _, | ||
allowRefOrOut: AllowRefOrOut, | ||
allowThis: false, | ||
addRefReadOnlyModifier: false, | ||
diagnostics: diagnostics).Cast<SourceParameterSymbol, ParameterSymbol>(); | ||
|
||
_lazyIsVararg = (arglistToken.Kind() == SyntaxKind.ArgListKeyword); | ||
_lazyReturnType = TypeWithAnnotations.Create(bodyBinder.GetSpecialType(SpecialType.System_Void, diagnostics, syntax)); | ||
|
||
var location = this.GetFirstLocation(); | ||
|
@@ -72,7 +69,7 @@ protected sealed override void MethodChecks(BindingDiagnosticBag diagnostics) | |
this.CheckEffectiveAccessibility(_lazyReturnType, _lazyParameters, diagnostics); | ||
this.CheckFileTypeUsage(_lazyReturnType, _lazyParameters, diagnostics); | ||
|
||
if (_lazyIsVararg && (IsGenericMethod || ContainingType.IsGenericType || _lazyParameters.Length > 0 && _lazyParameters[_lazyParameters.Length - 1].IsParams)) | ||
if (this.IsVararg && (IsGenericMethod || ContainingType.IsGenericType || _lazyParameters.Length > 0 && _lazyParameters[_lazyParameters.Length - 1].IsParams)) | ||
{ | ||
diagnostics.Add(ErrorCode.ERR_BadVarargs, location); | ||
} | ||
|
@@ -100,15 +97,6 @@ internal sealed override void AfterAddingTypeMembersChecks(ConversionsBase conve | |
} | ||
} | ||
|
||
public sealed override bool IsVararg | ||
{ | ||
get | ||
{ | ||
LazyMethodChecks(); | ||
return _lazyIsVararg; | ||
} | ||
} | ||
|
||
public sealed override bool IsImplicitlyDeclared | ||
{ | ||
get | ||
|
@@ -150,11 +138,6 @@ public sealed override ImmutableArray<ImmutableArray<TypeWithAnnotations>> GetTy | |
public sealed override ImmutableArray<TypeParameterConstraintKind> GetTypeParameterConstraintKinds() | ||
=> ImmutableArray<TypeParameterConstraintKind>.Empty; | ||
|
||
public override RefKind RefKind | ||
{ | ||
get { return RefKind.None; } | ||
} | ||
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. derived types will pass this in when calling MakeFlags. |
||
|
||
public sealed override TypeWithAnnotations ReturnTypeWithAnnotations | ||
{ | ||
get | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,11 +24,14 @@ protected SourceDelegateMethodSymbol( | |
TypeWithAnnotations returnType, | ||
DelegateDeclarationSyntax syntax, | ||
MethodKind methodKind, | ||
RefKind refKind, | ||
DeclarationModifiers declarationModifiers) | ||
: base(delegateType, syntax.GetReference(), location: syntax.Identifier.GetLocation(), isIterator: false) | ||
{ | ||
_returnType = returnType; | ||
this.MakeFlags(methodKind, declarationModifiers, _returnType.IsVoidType(), isExtensionMethod: false, isNullableAnalysisEnabled: false); | ||
this.MakeFlags( | ||
methodKind, refKind, declarationModifiers, _returnType.IsVoidType(), hasAnyBody: false, isExpressionBodied: false, | ||
isExtensionMethod: false, isVarArg: false, isNullableAnalysisEnabled: false); | ||
} | ||
|
||
internal sealed override ExecutableCodeBinder TryGetBodyBinder(BinderFactory binderFactoryOpt = null, bool ignoreAccessibility = false) => throw ExceptionUtilities.Unreachable(); | ||
|
@@ -129,14 +132,6 @@ protected override void MethodChecks(BindingDiagnosticBag diagnostics) | |
// TODO: move more functionality into here, making these symbols more lazy | ||
} | ||
|
||
public sealed override bool IsVararg | ||
{ | ||
get | ||
{ | ||
return false; | ||
} | ||
} | ||
|
||
public sealed override ImmutableArray<ParameterSymbol> Parameters | ||
{ | ||
get | ||
|
@@ -176,11 +171,6 @@ public sealed override bool IsImplicitlyDeclared | |
} | ||
} | ||
|
||
internal override bool IsExpressionBodied | ||
{ | ||
get { return false; } | ||
} | ||
|
||
internal override bool GenerateDebugInfo | ||
{ | ||
get { return false; } | ||
|
@@ -219,7 +209,7 @@ internal Constructor( | |
TypeWithAnnotations objectType, | ||
TypeWithAnnotations intPtrType, | ||
DelegateDeclarationSyntax syntax) | ||
: base(delegateType, voidType, syntax, MethodKind.Constructor, DeclarationModifiers.Public) | ||
: base(delegateType, voidType, syntax, MethodKind.Constructor, RefKind.None, DeclarationModifiers.Public) | ||
{ | ||
InitializeParameters(ImmutableArray.Create<ParameterSymbol>( | ||
SynthesizedParameterSymbol.Create(this, objectType, 0, RefKind.None, "object"), | ||
|
@@ -231,11 +221,6 @@ public override string Name | |
get { return WellKnownMemberNames.InstanceConstructorName; } | ||
} | ||
|
||
public override RefKind RefKind | ||
{ | ||
get { return RefKind.None; } | ||
} | ||
|
||
internal override OneOrMany<SyntaxList<AttributeListSyntax>> GetReturnTypeAttributeDeclarations() | ||
{ | ||
// Constructors don't have return type attributes | ||
|
@@ -258,7 +243,6 @@ internal override LexicalSortKey GetLexicalSortKey() | |
|
||
private sealed class InvokeMethod : SourceDelegateMethodSymbol | ||
{ | ||
private readonly RefKind _refKind; | ||
private readonly ImmutableArray<CustomModifier> _refCustomModifiers; | ||
|
||
internal InvokeMethod( | ||
|
@@ -268,10 +252,8 @@ internal InvokeMethod( | |
DelegateDeclarationSyntax syntax, | ||
Binder binder, | ||
BindingDiagnosticBag diagnostics) | ||
: base(delegateType, returnType, syntax, MethodKind.DelegateInvoke, DeclarationModifiers.Virtual | DeclarationModifiers.Public) | ||
: base(delegateType, returnType, syntax, MethodKind.DelegateInvoke, refKind, DeclarationModifiers.Virtual | DeclarationModifiers.Public) | ||
{ | ||
this._refKind = refKind; | ||
|
||
SyntaxToken arglistToken; | ||
var parameters = ParameterHelpers.MakeParameters( | ||
binder, this, syntax.ParameterList, out arglistToken, | ||
|
@@ -288,7 +270,7 @@ internal InvokeMethod( | |
diagnostics.Add(ErrorCode.ERR_IllegalVarArgs, new SourceLocation(arglistToken)); | ||
} | ||
|
||
if (_refKind == RefKind.RefReadOnly) | ||
if (this.RefKind == RefKind.RefReadOnly) | ||
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 was set in teh base constructor call. |
||
{ | ||
var modifierType = binder.GetWellKnownType(WellKnownType.System_Runtime_InteropServices_InAttribute, diagnostics, syntax.ReturnType); | ||
_refCustomModifiers = ImmutableArray.Create(CSharpCustomModifier.CreateRequired(modifierType)); | ||
|
@@ -306,11 +288,6 @@ public override string Name | |
get { return WellKnownMemberNames.DelegateInvokeName; } | ||
} | ||
|
||
public override RefKind RefKind | ||
{ | ||
get { return _refKind; } | ||
} | ||
|
||
internal override LexicalSortKey GetLexicalSortKey() | ||
{ | ||
// associate "Invoke and .ctor" with whole delegate declaration for the sorting purposes | ||
|
@@ -332,7 +309,7 @@ internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions, | |
|
||
base.AfterAddingTypeMembersChecks(conversions, diagnostics); | ||
|
||
if (_refKind == RefKind.RefReadOnly) | ||
if (this.RefKind == RefKind.RefReadOnly) | ||
{ | ||
compilation.EnsureIsReadOnlyAttributeExists(diagnostics, location, modifyCompilation: true); | ||
} | ||
|
@@ -367,7 +344,7 @@ internal BeginInvokeMethod( | |
TypeWithAnnotations objectType, | ||
TypeWithAnnotations asyncCallbackType, | ||
DelegateDeclarationSyntax syntax) | ||
: base((SourceNamedTypeSymbol)invoke.ContainingType, iAsyncResultType, syntax, MethodKind.Ordinary, DeclarationModifiers.Virtual | DeclarationModifiers.Public) | ||
: base((SourceNamedTypeSymbol)invoke.ContainingType, iAsyncResultType, syntax, MethodKind.Ordinary, RefKind.None, DeclarationModifiers.Virtual | DeclarationModifiers.Public) | ||
{ | ||
var parameters = ArrayBuilder<ParameterSymbol>.GetInstance(); | ||
foreach (SourceParameterSymbol p in invoke.Parameters) | ||
|
@@ -388,11 +365,6 @@ public override string Name | |
get { return WellKnownMemberNames.DelegateBeginInvokeName; } | ||
} | ||
|
||
public override RefKind RefKind | ||
{ | ||
get { return RefKind.None; } | ||
} | ||
|
||
internal override OneOrMany<SyntaxList<AttributeListSyntax>> GetReturnTypeAttributeDeclarations() | ||
{ | ||
// BeginInvoke method doesn't have return type attributes | ||
|
@@ -410,7 +382,7 @@ internal EndInvokeMethod( | |
InvokeMethod invoke, | ||
TypeWithAnnotations iAsyncResultType, | ||
DelegateDeclarationSyntax syntax) | ||
: base((SourceNamedTypeSymbol)invoke.ContainingType, invoke.ReturnTypeWithAnnotations, syntax, MethodKind.Ordinary, DeclarationModifiers.Virtual | DeclarationModifiers.Public) | ||
: base((SourceNamedTypeSymbol)invoke.ContainingType, invoke.ReturnTypeWithAnnotations, syntax, MethodKind.Ordinary, invoke.RefKind, DeclarationModifiers.Virtual | DeclarationModifiers.Public) | ||
{ | ||
_invoke = invoke; | ||
|
||
|
@@ -434,8 +406,6 @@ internal EndInvokeMethod( | |
|
||
public override string Name => WellKnownMemberNames.DelegateEndInvokeName; | ||
|
||
public override RefKind RefKind => _invoke.RefKind; | ||
|
||
public override ImmutableArray<CustomModifier> RefCustomModifiers => _invoke.RefCustomModifiers; | ||
} | ||
|
||
|
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.
we could consider moving this bit down as well. That would save another unnecessary 32bits in constructors. however, constructors seem to be orders of magnitude less common than methods, so it's not high on my priority list.