Skip to content
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

Merged
merged 37 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
3e67b7c
Pack bits in SourceOrdinaryMethodSymbol into an existing bitflag stru…
CyrusNajmabadi May 10, 2023
ce343a6
Compute IsVarArg up front
CyrusNajmabadi May 10, 2023
1319147
Remove unused usings
CyrusNajmabadi May 10, 2023
eee77ee
Remove
CyrusNajmabadi May 10, 2023
d2a9c02
Remove passing of flag
CyrusNajmabadi May 11, 2023
824b5b1
move up and make non-virtual
CyrusNajmabadi May 11, 2023
1932ed3
"move up and make sealed"
CyrusNajmabadi May 11, 2023
9f7d914
move up and make sealed
CyrusNajmabadi May 11, 2023
a0295b7
use properties instead of flags
CyrusNajmabadi May 11, 2023
27e28b7
Merge remote-tracking branch 'upstream/main' into revertRevert
CyrusNajmabadi May 11, 2023
54c18cf
Make parameters non-optional
CyrusNajmabadi May 11, 2023
d3a9476
Pass all args
CyrusNajmabadi May 11, 2023
c74ca1a
move more down
CyrusNajmabadi May 11, 2023
e73807b
move more down
CyrusNajmabadi May 11, 2023
2ad5f4c
Pass all args
CyrusNajmabadi May 11, 2023
5eb5236
move more down
CyrusNajmabadi May 11, 2023
08eee6e
move more down
CyrusNajmabadi May 11, 2023
5640b00
move more down
CyrusNajmabadi May 11, 2023
0e4f6fb
move more down
CyrusNajmabadi May 11, 2023
d02d68e
Name consistently
CyrusNajmabadi May 11, 2023
5d757af
Set flag consistently
CyrusNajmabadi May 11, 2023
94a74c9
Rename
CyrusNajmabadi May 11, 2023
fb5eaa9
Merge remote-tracking branch 'upstream/main' into revertRevert
CyrusNajmabadi May 12, 2023
48a1a41
Fix usage of hasBody where it means hasBlockBody
CyrusNajmabadi May 12, 2023
d4bfe7e
Change to true
CyrusNajmabadi May 12, 2023
43aab35
Change to true
CyrusNajmabadi May 12, 2023
efe37fc
Fix vararg check
CyrusNajmabadi May 12, 2023
2e34e56
Change to true
CyrusNajmabadi May 12, 2023
ff7192b
NRT
CyrusNajmabadi May 12, 2023
a27e807
Pass values explicitly
CyrusNajmabadi May 12, 2023
c11f580
Add assert
CyrusNajmabadi May 12, 2023
aa120d2
Flip depending on if something is abstract or not.
CyrusNajmabadi May 12, 2023
31a6277
Merge remote-tracking branch 'upstream/main' into revertRevert
CyrusNajmabadi May 13, 2023
fc9d571
Update check
CyrusNajmabadi May 13, 2023
77474b1
Fix ocmment
CyrusNajmabadi May 15, 2023
d3b2b40
Fix ocmment
CyrusNajmabadi May 15, 2023
b76e78e
Update src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructor…
CyrusNajmabadi May 16, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,6 @@ 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 @@ -46,11 +46,16 @@ protected SynthesizedMethodBaseSymbol(NamedTypeSymbol containingType,

this.MakeFlags(
methodKind: MethodKind.Ordinary,
refKind: baseMethod.RefKind,
declarationModifiers: declarationModifiers,
returnsVoid: baseMethod.ReturnsVoid,
// Consider synthesized methods to always have bodies.
hasAnyBody: true,
isExtensionMethod: false,
isNullableAnalysisEnabled: false,
isMetadataVirtualIgnoringModifiers: false);
isVarArg: baseMethod.IsVararg,
isMetadataVirtualIgnoringModifiers: false,
isExpressionBodied: false);
}

protected void AssignTypeMapAndTypeParameters(TypeMap typeMap, ImmutableArray<TypeParameterSymbol> typeParameters)
Expand Down Expand Up @@ -196,11 +201,6 @@ internal sealed override IEnumerable<SecurityAttribute> GetSecurityInformation()

#nullable disable

public sealed override RefKind RefKind
{
get { return this.BaseMethod.RefKind; }
}

public sealed override TypeWithAnnotations ReturnTypeWithAnnotations
{
get { return this.TypeMap.SubstituteType(this.BaseMethod.OriginalDefinition.ReturnTypeWithAnnotations); }
Expand All @@ -212,11 +212,6 @@ public sealed override TypeWithAnnotations ReturnTypeWithAnnotations

public sealed override FlowAnalysisAnnotations FlowAnalysisAnnotations => FlowAnalysisAnnotations.None;

public sealed override bool IsVararg
{
get { return this.BaseMethod.IsVararg; }
}

public sealed override string Name
{
get { return _name; }
Expand All @@ -226,10 +221,5 @@ 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 @@ -2,6 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

// Ignore Spelling: Nullable
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved

CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
#nullable disable

using System.Diagnostics;
Expand All @@ -12,7 +14,6 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
internal sealed class SourceConstructorSymbol : SourceConstructorSymbolBase
{
private readonly bool _isExpressionBodied;
private readonly bool _hasThisInitializer;
Copy link
Member Author

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.


public static SourceConstructorSymbol CreateConstructorSymbol(
Expand All @@ -35,14 +36,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)
{
Expand All @@ -57,22 +60,22 @@ private SourceConstructorSymbol(
diagnostics.Add(ErrorCode.ERR_ExternHasConstructorInitializer, location, this);
}

if (hasBody)
if (hasAnyBody)
Copy link
Member Author

Choose a reason for hiding this comment

The 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.

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 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(
Expand Down Expand Up @@ -163,14 +166,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 ?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ internal abstract class SourceConstructorSymbolBase : SourceMemberMethodSymbol
{
protected ImmutableArray<ParameterSymbol> _lazyParameters;
private TypeWithAnnotations _lazyReturnType;
private bool _lazyIsVararg;
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 is another nice 32bits of savings.


protected SourceConstructorSymbolBase(
SourceMemberContainerTypeSymbol containingType,
Expand Down Expand Up @@ -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();
Expand All @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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; }
}
Copy link
Member Author

Choose a reason for hiding this comment

The 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#nullable disable

using System.Collections.Immutable;
using System.Diagnostics;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Roslyn.Utilities;
Expand Down Expand Up @@ -32,7 +31,9 @@ internal SourceCustomEventAccessorSymbol(
syntax.Keyword.GetLocation(), explicitlyImplementedEventOpt, aliasQualifierOpt,
isAdder: syntax.Kind() == SyntaxKind.AddAccessorDeclaration,
isIterator: SyntaxFacts.HasYieldOperations(syntax.Body),
isNullableAnalysisEnabled: isNullableAnalysisEnabled)
isNullableAnalysisEnabled: isNullableAnalysisEnabled,
hasAnyBody: syntax.Body is not null || syntax.ExpressionBody is not null,
isExpressionBodied: syntax is { Body: null, ExpressionBody: not null })
{
Debug.Assert(syntax != null);
Debug.Assert(syntax.Kind() == SyntaxKind.AddAccessorDeclaration || syntax.Kind() == SyntaxKind.RemoveAccessorDeclaration);
Expand Down Expand Up @@ -91,16 +92,5 @@ 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 @@ -155,8 +155,9 @@ internal SourceCustomEventSymbol(SourceMemberContainerTypeSymbol containingType,
diagnostics.Add(ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation, this.GetFirstLocation());
}

_addMethod = new SynthesizedEventAccessorSymbol(this, isAdder: true, explicitlyImplementedEvent, aliasQualifierOpt);
_removeMethod = new SynthesizedEventAccessorSymbol(this, isAdder: false, explicitlyImplementedEvent, aliasQualifierOpt);
// No body as this was an abstract event.
_addMethod = new SynthesizedEventAccessorSymbol(this, isAdder: true, hasAnyBody: false, isExpressionBodied: false, explicitlyImplementedEvent, aliasQualifierOpt);
_removeMethod = new SynthesizedEventAccessorSymbol(this, isAdder: false, hasAnyBody: false, isExpressionBodied: false, explicitlyImplementedEvent, aliasQualifierOpt);
}
else
{
Expand Down
Loading