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

EnC: Implement support for partial properties #74494

Merged
merged 4 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
11 changes: 10 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/PropertySymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using Microsoft.CodeAnalysis.Symbols;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
/// <summary>
/// Represents a property or indexer.
/// </summary>
internal abstract partial class PropertySymbol : Symbol
internal abstract partial class PropertySymbol : Symbol, IPropertySymbolInternal
{
/// <summary>
/// As a performance optimization, cache parameter types and refkinds - overload resolution uses them a lot.
Expand Down Expand Up @@ -321,6 +322,14 @@ internal virtual bool IsExplicitInterfaceImplementation
/// </remarks>
public abstract ImmutableArray<PropertySymbol> ExplicitInterfaceImplementations { get; }

#nullable enable
internal virtual PropertySymbol? PartialImplementationPart => null;
internal virtual PropertySymbol? PartialDefinitionPart => null;
tmat marked this conversation as resolved.
Show resolved Hide resolved

IPropertySymbolInternal? IPropertySymbolInternal.PartialImplementationPart => PartialImplementationPart;
IPropertySymbolInternal? IPropertySymbolInternal.PartialDefinitionPart => PartialDefinitionPart;
#nullable disable

/// <summary>
/// Gets the kind of this symbol.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarati
// This is an error scenario (requires using a property initializer and field-targeted attributes on partial property implementation part).
|| this.BackingField is not null);

if (PartialImplementationPart is { } implementationPart)
if (SourcePartialImplementationPart is { } implementationPart)
{
return OneOrMany.Create(
((BasePropertyDeclarationSyntax)CSharpSyntaxNode).AttributeLists,
Expand All @@ -191,7 +191,7 @@ public override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarati
}
}

protected override SourcePropertySymbolBase? BoundAttributesSource => PartialDefinitionPart;
protected override SourcePropertySymbolBase? BoundAttributesSource => SourcePartialDefinitionPart;

public override IAttributeTargetSymbol AttributesOwner => this;

Expand Down Expand Up @@ -727,9 +727,11 @@ private void PartialPropertyChecks(SourcePropertySymbol implementation, BindingD

internal bool IsPartialImplementation => IsPartial && (AccessorsHaveImplementation || HasExternModifier);

internal SourcePropertySymbol? PartialDefinitionPart => IsPartialImplementation ? OtherPartOfPartial : null;
internal SourcePropertySymbol? SourcePartialDefinitionPart => IsPartialImplementation ? OtherPartOfPartial : null;
internal SourcePropertySymbol? SourcePartialImplementationPart => IsPartialDefinition ? OtherPartOfPartial : null;

internal SourcePropertySymbol? PartialImplementationPart => IsPartialDefinition ? OtherPartOfPartial : null;
internal override PropertySymbol? PartialDefinitionPart => SourcePartialDefinitionPart;
internal override PropertySymbol? PartialImplementationPart => SourcePartialImplementationPart;

internal static void InitializePartialPropertyParts(SourcePropertySymbol definition, SourcePropertySymbol implementation)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2567,56 +2567,52 @@ .locals init ([unchanged] V_0,
[Fact]
public void PartialMethod()
{
var source =
@"partial class C
{
static partial void M1();
static partial void M2();
static partial void M3();
static partial void M1() { }
static partial void M2() { }
}";
var compilation0 = CreateCompilation(source, parseOptions: TestOptions.Regular.WithNoRefSafetyRulesAttribute(), options: TestOptions.DebugDll);
var compilation1 = compilation0.WithSource(source);

var bytes0 = compilation0.EmitToArray();
using var md0 = ModuleMetadata.CreateFromImage(bytes0);
var reader0 = md0.MetadataReader;

CheckNames(reader0, reader0.GetMethodDefNames(), "M1", "M2", ".ctor");

var method0 = compilation0.GetMember<MethodSymbol>("C.M2").PartialImplementationPart;
var method1 = compilation1.GetMember<MethodSymbol>("C.M2").PartialImplementationPart;

var generation0 = CreateInitialBaseline(compilation0,
md0,
EmptyLocalsProvider);

var diff1 = compilation1.EmitDifference(
generation0,
ImmutableArray.Create(SemanticEdit.Create(SemanticEditKind.Update, method0, method1)));

var methods = diff1.TestData.GetMethodsByName();
Assert.Equal(1, methods.Count);
Assert.True(methods.ContainsKey("C.M2()"));

using var md1 = diff1.GetMetadata();
var reader1 = md1.Reader;
var readers = new[] { reader0, reader1 };

EncValidation.VerifyModuleMvid(1, reader0, reader1);

CheckNames(readers, reader1.GetMethodDefNames(), "M2");
using var _ = new EditAndContinueTest(options: TestOptions.DebugDll)
.AddBaseline(
source: """
partial class C
{
static partial void M1();
static partial void M2();
static partial void M3();
static partial void M1() { }
static partial void M2() { }
}
""",
validator: v =>
{
v.VerifyMethodDefNames("M1", "M2", ".ctor");
})
.AddGeneration(
source: """
partial class C
{
static partial void M1();
static partial void M2();
static partial void M3();
static partial void M1() { }
static partial void M2() { }
}
""",
edits:
[
Edit(SemanticEditKind.Update, c => c.GetMember<IMethodSymbol>("C.M2").PartialImplementationPart),
],
validator: v =>
{
v.VerifyMethodDefNames("M2");

CheckEncLog(reader1,
Row(2, TableIndex.AssemblyRef, EditAndContinueOperation.Default),
Row(6, TableIndex.TypeRef, EditAndContinueOperation.Default),
Row(2, TableIndex.MethodDef, EditAndContinueOperation.Default));
v.VerifyEncLogDefinitions(
[
Row(2, TableIndex.MethodDef, EditAndContinueOperation.Default)
]);

CheckEncMap(reader1,
Handle(6, TableIndex.TypeRef),
Handle(2, TableIndex.MethodDef),
Handle(2, TableIndex.AssemblyRef));
v.VerifyEncMapDefinitions(
[
Handle(2, TableIndex.MethodDef)
]);
})
.Verify();
}

[WorkItem(60804, "https://github.com/dotnet/roslyn/issues/60804")]
Expand Down Expand Up @@ -2711,6 +2707,100 @@ .maxstack 8
.Verify();
}

[Fact]
public void PartialProperty()
{
using var _ = new EditAndContinueTest(options: TestOptions.DebugDll)
.AddBaseline(
source: """
partial class C
{
partial int P { get; }
partial int P => 1;
}
""",
validator: v =>
{
v.VerifyMethodDefNames("get_P", ".ctor");
})
.AddGeneration(
source: """
partial class C
{
[System.Obsolete]partial int P { get; }
partial int P => 1;
}
""",
edits:
[
Edit(SemanticEditKind.Update, c => c.GetMember<IPropertySymbol>("C.P").PartialImplementationPart),
],
validator: v =>
{
v.VerifyMethodDefNames();

v.VerifyEncLogDefinitions(
[
Row(1, TableIndex.Property, EditAndContinueOperation.Default),
Row(4, TableIndex.CustomAttribute, EditAndContinueOperation.Default),
Row(2, TableIndex.MethodSemantics, EditAndContinueOperation.Default)
]);

v.VerifyEncMapDefinitions(
[
Handle(4, TableIndex.CustomAttribute),
Handle(1, TableIndex.Property),
Handle(2, TableIndex.MethodSemantics)
]);
})
.Verify();
}

[Fact]
public void PartialProperty_Accessor()
{
using var _ = new EditAndContinueTest(options: TestOptions.DebugDll)
.AddBaseline(
source: """
partial class C
{
partial int P { get; }
partial int P => 1;
}
""",
validator: v =>
{
v.VerifyMethodDefNames("get_P", ".ctor");
})
.AddGeneration(
source: """
partial class C
{
partial int P { get; }
partial int P => 2;
}
""",
edits:
[
Edit(SemanticEditKind.Update, c => c.GetMember<IMethodSymbol>("C.get_P").PartialImplementationPart),
],
validator: v =>
{
v.VerifyMethodDefNames("get_P");

v.VerifyEncLogDefinitions(
[
Row(1, TableIndex.MethodDef, EditAndContinueOperation.Default)
]);

v.VerifyEncMapDefinitions(
[
Handle(1, TableIndex.MethodDef)
]);
})
.Verify();
}

[Fact]
public void Method_WithAttributes_Add()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -924,7 +924,7 @@ .maxstack 2
Assert.True(propDefinition.IsPartialDefinition);

var propImplementation = propDefinition.PartialImplementationPart!;
Assert.True(propImplementation.IsPartialImplementation);
Assert.True(propImplementation.IsPartialImplementation());

Assert.Same(propDefinition, propImplementation.PartialDefinitionPart);
Assert.Null(propImplementation.PartialImplementationPart);
Expand Down Expand Up @@ -998,7 +998,7 @@ .maxstack 1
Assert.True(propDefinition.IsPartialDefinition);

var propImplementation = propDefinition.PartialImplementationPart!;
Assert.True(propImplementation.IsPartialImplementation);
Assert.True(propImplementation.IsPartialImplementation());

Assert.Same(propDefinition, propImplementation.PartialDefinitionPart);
Assert.Null(propImplementation.PartialImplementationPart);
Expand Down Expand Up @@ -1084,7 +1084,7 @@ .maxstack 1
Assert.True(propDefinition.IsPartialDefinition);

var propImplementation = propDefinition.PartialImplementationPart!;
Assert.True(propImplementation.IsPartialImplementation);
Assert.True(propImplementation.IsPartialImplementation());

Assert.Same(propDefinition, propImplementation.PartialDefinitionPart);
Assert.Null(propImplementation.PartialImplementationPart);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -412,9 +412,8 @@ private void CalculateChanges(

var newMember = GetRequiredInternalSymbol(edit.NewSymbol);

// Partial methods are supplied as implementations but recorded
// Partial methods/properties/indexers are supplied as implementations but recorded
// internally as definitions since definitions are used in emit.
// https://github.com/dotnet/roslyn/issues/73772: should we also make sure to use the definition for a partial property?
if (newMember.Kind == SymbolKind.Method)
{
var newMethod = (IMethodSymbolInternal)newMember;
Expand All @@ -438,6 +437,16 @@ private void CalculateChanges(
updatedMethodsPerType.Add((oldMethod.PartialDefinitionPart ?? oldMethod, (IMethodSymbolInternal)newMember));
}
}
else if (newMember.Kind == SymbolKind.Property)
{
var newProperty = (IPropertySymbolInternal)newMember;

// Partial properties should be implementations, not definitions.
Debug.Assert(newProperty.PartialImplementationPart == null);
Debug.Assert(edit.OldSymbol == null || ((IPropertySymbol)edit.OldSymbol).PartialImplementationPart == null);

newMember = newProperty.PartialDefinitionPart ?? newMember;
}

AddContainingSymbolChanges(changesBuilder, newMember);

Expand Down
9 changes: 4 additions & 5 deletions src/Compilers/Core/Portable/Emit/SemanticEdit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -142,15 +142,14 @@ public SemanticEdit(SemanticEditKind kind, ISymbol? oldSymbol, ISymbol? newSymbo
}
}

// https://github.com/dotnet/roslyn/issues/73772: should we also do this check for partial properties?
if (oldSymbol is IMethodSymbol { PartialImplementationPart: not null })
if (oldSymbol is IMethodSymbol { PartialImplementationPart: not null } or IPropertySymbol { PartialImplementationPart: not null })
{
throw new ArgumentException("Partial method implementation required", nameof(oldSymbol));
throw new ArgumentException("Partial member implementation required", nameof(oldSymbol));
}

if (newSymbol is IMethodSymbol { PartialImplementationPart: not null })
if (newSymbol is IMethodSymbol { PartialImplementationPart: not null } or IPropertySymbol { PartialImplementationPart: not null })
{
throw new ArgumentException("Partial method implementation required", nameof(newSymbol));
throw new ArgumentException("Partial member implementation required", nameof(newSymbol));
}

if (kind == SemanticEditKind.Delete && oldSymbol is not (IMethodSymbol or IPropertySymbol or IEventSymbol))
Expand Down
11 changes: 11 additions & 0 deletions src/Compilers/Core/Portable/Symbols/IPropertySymbolInternal.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

namespace Microsoft.CodeAnalysis.Symbols;

internal interface IPropertySymbolInternal : ISymbolInternal
{
IPropertySymbolInternal? PartialImplementationPart { get; }
IPropertySymbolInternal? PartialDefinitionPart { get; }
}
20 changes: 16 additions & 4 deletions src/Compilers/VisualBasic/Portable/Symbols/PropertySymbol.vb
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,18 @@
' The .NET Foundation licenses this file to you under the MIT license.
' See the LICENSE file in the project root for more information.

Imports System.Collections.Generic
Imports System.Collections.Immutable
Imports Microsoft.CodeAnalysis.PooledObjects
Imports Microsoft.CodeAnalysis.Text
Imports Microsoft.CodeAnalysis.Symbols
Imports Microsoft.CodeAnalysis.VisualBasic.Symbols
Imports Microsoft.CodeAnalysis.VisualBasic.Syntax

Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
''' <summary>
''' Represents a property.
''' </summary>
Friend MustInherit Class PropertySymbol
Inherits Symbol
Implements IPropertySymbol
Implements IPropertySymbol, IPropertySymbolInternal

' !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
' Changes to the public interface of this class should remain synchronized with the C# version.
Expand Down Expand Up @@ -642,6 +640,20 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Symbols
End Get
End Property

Public ReadOnly Property IPropertySymbolInternal_PartialImplementationPart As IPropertySymbolInternal Implements IPropertySymbolInternal.PartialImplementationPart
Get
' Feature not supported in VB
Return Nothing
End Get
End Property

Public ReadOnly Property IPropertySymbolInternal_PartialDefinitionPart As IPropertySymbolInternal Implements IPropertySymbolInternal.PartialDefinitionPart
Get
' Feature not supported in VB
Return Nothing
End Get
End Property

Public Overrides Sub Accept(visitor As SymbolVisitor)
visitor.VisitProperty(Me)
End Sub
Expand Down
Loading