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.

Sign up for GitHub

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: Parameter and property analysis #69774

Merged
merged 14 commits into from
Sep 26, 2023
Merged

EnC: Parameter and property analysis #69774

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
886773f
Refactor parameter and property analysis in EnC analyzer
tmat Aug 17, 2023
c9fa5a8
Fix
tmat Sep 11, 2023
f3053e2
Fixes
tmat Sep 11, 2023
d659a6d
Formatting
tmat Sep 11, 2023
67e3664
Formatting
tmat Sep 11, 2023
cad782a
Fix RefKind checking in the compiler
tmat Sep 11, 2023
31fb12c
Unskip tests, fix reporting rude edit for primary parameter reorder
tmat Sep 12, 2023
580c4e4
Fixes
tmat Sep 15, 2023
1eb77de
Fix
tmat Sep 16, 2023
9f5a456
Fix
tmat Sep 16, 2023
e389489
Formatting
tmat Sep 17, 2023
a340249
More deleted members cleanup
tmat Sep 17, 2023
a7c6428
Formatting
tmat Sep 17, 2023
5d5981f
Apply suggestions from code review
tmat Sep 26, 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
3 changes: 2 additions & 1 deletion SpellingExclusions.dic
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
awaitable
stackalloc
awaitable
Refactorings
Infos
12 changes: 10 additions & 2 deletions src/Compilers/CSharp/Portable/Emitter/EditAndContinue/CSharpSymbolMatcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,10 @@ private bool AreArrayTypesEqual(ArrayTypeSymbol type, ArrayTypeSymbol other)
private bool AreEventsEqual(EventSymbol @event, EventSymbol other)
{
Debug.Assert(StringOrdinalComparer.Equals(@event.Name, other.Name));
return _comparer.Equals(@event.Type, other.Type);

// Events can't be overloaded on type.
// ECMA: Within the rows owned by a given row in the TypeDef table, there shall be no duplicates based upon Name [ERROR]
return true;
}

private bool AreFieldsEqual(FieldSymbol field, FieldSymbol other)
Expand Down Expand Up @@ -868,7 +871,9 @@ private bool AreNamespacesEqual(NamespaceSymbol @namespace, NamespaceSymbol othe
private bool AreParametersEqual(ParameterSymbol parameter, ParameterSymbol other)
{
Debug.Assert(parameter.Ordinal == other.Ordinal);
return (parameter.RefKind == other.RefKind) &&

// allow a different ref-kind as long as the runtime time is the same:
return parameter.RefKind is RefKind.None == other.RefKind is RefKind.None &&
_comparer.Equals(parameter.Type, other.Type);
}

Expand Down Expand Up @@ -927,6 +932,9 @@ static bool verifyRefModifiers(ImmutableArray<CustomModifier> modifiers, RefKind
private bool ArePropertiesEqual(PropertySymbol property, PropertySymbol other)
{
Debug.Assert(StringOrdinalComparer.Equals(property.MetadataName, other.MetadataName));

// Properties may be overloaded on their signature.
// ECMA: Within the rows owned by a given row in the TypeDef table, there shall be no duplicates based upon Name+Type [ERROR]
return _comparer.Equals(property.Type, other.Type) &&
property.RefKind.Equals(other.RefKind) &&
property.Parameters.SequenceEqual(other.Parameters, AreParametersEqual);
Expand Down
600 changes: 411 additions & 189 deletions src/Compilers/CSharp/Test/Emit/Emit/EditAndContinue/EditAndContinueTests.cs
View file
Open in desktop

Large diffs are not rendered by default.

86 changes: 0 additions & 86 deletions src/Compilers/Core/Portable/Emit/EditAndContinue/DeletedEventDefinition.cs
View file
Open in desktop

This file was deleted.

105 changes: 0 additions & 105 deletions src/Compilers/Core/Portable/Emit/EditAndContinue/DeletedPropertyDefinition.cs
View file
Open in desktop

This file was deleted.

74 changes: 17 additions & 57 deletions src/Compilers/Core/Portable/Emit/EditAndContinue/DeltaMetadataWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using System.Threading;
using Microsoft.Cci;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.Emit.EditAndContinue;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Symbols;
Expand All @@ -38,7 +39,7 @@ internal sealed class DeltaMetadataWriter : MetadataWriter
/// </summary>
private readonly Dictionary<ITypeDefinition, DeletedTypeDefinition> _typesUsedByDeletedMembers;

private readonly Dictionary<ITypeDefinition, ImmutableDictionary<IMethodDefinition, DeletedMethodDefinition>> _deletedTypeMembers;
private readonly Dictionary<ITypeDefinition, ImmutableArray<DeletedMethodDefinition>> _deletedTypeMembers;

private readonly DefinitionIndex<ITypeDefinition> _typeDefs;
private readonly DefinitionIndex<IEventDefinition> _eventDefs;
Expand Down Expand Up @@ -104,7 +105,7 @@ public DeltaMetadataWriter(

_changedTypeDefs = new List<ITypeDefinition>();
_typesUsedByDeletedMembers = new Dictionary<ITypeDefinition, DeletedTypeDefinition>(ReferenceEqualityComparer.Instance);
_deletedTypeMembers = new Dictionary<ITypeDefinition, ImmutableDictionary<IMethodDefinition, DeletedMethodDefinition>>(ReferenceEqualityComparer.Instance);
_deletedTypeMembers = new Dictionary<ITypeDefinition, ImmutableArray<DeletedMethodDefinition>>(ReferenceEqualityComparer.Instance);
_typeDefs = new DefinitionIndex<ITypeDefinition>(this.TryGetExistingTypeDefIndex, sizes[(int)TableIndex.TypeDef]);
_eventDefs = new DefinitionIndex<IEventDefinition>(this.TryGetExistingEventDefIndex, sizes[(int)TableIndex.Event]);
_fieldDefs = new DefinitionIndex<IFieldDefinition>(this.TryGetExistingFieldDefIndex, sizes[(int)TableIndex.Field]);
Expand Down Expand Up @@ -540,24 +541,6 @@ protected override void CreateIndicesForNonTypeMembers(ITypeDefinition typeDef)

int typeRowId = _typeDefs.GetRowId(typeDef);

// First we find the deleted methods, and add them to our dictionary. This is used later when
// processing events, properties, methods, and references.
ImmutableDictionary<IMethodDefinition, DeletedMethodDefinition>? deletedMethodDefinitions = null;

var deletedMethods = _changes.GetDeletedMethods(typeDef);
if (deletedMethods.Length > 0)
{
var deletedTypeMembers = ImmutableDictionary.CreateBuilder<IMethodDefinition, DeletedMethodDefinition>(ReferenceEqualityComparer.Instance);
foreach (var methodDef in deletedMethods)
{
var oldMethodDef = (IMethodDefinition)methodDef.GetCciAdapter();
deletedTypeMembers.Add(oldMethodDef, new DeletedMethodDefinition(oldMethodDef, typeDef, _typesUsedByDeletedMembers));
}

deletedMethodDefinitions = deletedTypeMembers.ToImmutableDictionary();
_deletedTypeMembers.Add(typeDef, deletedMethodDefinitions);
}

foreach (var eventDef in typeDef.GetEvents(this.Context))
{
if (!_eventMap.Contains(typeRowId))
Expand All @@ -569,21 +552,6 @@ protected override void CreateIndicesForNonTypeMembers(ITypeDefinition typeDef)
this.AddDefIfNecessary(_eventDefs, eventDef, eventChange);
}

foreach (var eventDef in _changes.GetDeletedEvents(typeDef))
{
RoslynDebug.AssertNotNull(deletedMethodDefinitions);

var oldEventDef = (IEventDefinition)eventDef.GetCciAdapter();

// Because deleted event information comes from the associated symbol of the deleted accessors, its safe
// to assume that everything will be in the dictionary. We wouldn't be here it if wasn't.
var adder = deletedMethodDefinitions[(IMethodDefinition)oldEventDef.Adder];
var remover = deletedMethodDefinitions[(IMethodDefinition)oldEventDef.Remover];
var caller = oldEventDef.Caller is null ? null : deletedMethodDefinitions[(IMethodDefinition)oldEventDef.Caller];
var newEventDef = new DeletedEventDefinition(oldEventDef, adder, remover, caller, typeDef, _typesUsedByDeletedMembers);
_eventDefs.AddUpdated(newEventDef);
}

foreach (var fieldDef in typeDef.GetFields(this.Context))
{
var fieldChange = _changes.GetChangeForPossibleReAddedMember(fieldDef, DefinitionExistsInAnyPreviousGeneration);
Expand All @@ -597,15 +565,22 @@ protected override void CreateIndicesForNonTypeMembers(ITypeDefinition typeDef)
CreateIndicesForMethod(methodDef, methodChange);
}

// Because we already processed the deleted methods above, this is a bit easier than
// properties and events, and we just need to make sure we add the right indices
if (deletedMethodDefinitions is not null)
var deletedMethods = _changes.GetDeletedMethods(typeDef);
if (deletedMethods.Length > 0)
{
foreach (var (_, newMethodDef) in deletedMethodDefinitions)
// create representations of the old deleted methods in this compilation:
var newMethodDefs = deletedMethods.SelectAsArray(
static (m, args) => new DeletedMethodDefinition((IMethodDefinition)m.GetCciAdapter(), args.typeDef, args._typesUsedByDeletedMembers),
(typeDef, _typesUsedByDeletedMembers));

// Assign the deleted method and its parameters row ids in the delta metadata:
foreach (var newMethodDef in newMethodDefs)
{
_methodDefs.AddUpdated(newMethodDef);
CreateIndicesForMethod(newMethodDef, SymbolChange.Updated);
}

_deletedTypeMembers.Add(typeDef, newMethodDefs);
}

foreach (var propertyDef in typeDef.GetProperties(this.Context))
Expand All @@ -619,20 +594,6 @@ protected override void CreateIndicesForNonTypeMembers(ITypeDefinition typeDef)
this.AddDefIfNecessary(_propertyDefs, propertyDef, propertyChange);
}

foreach (var propertyDef in _changes.GetDeletedProperties(typeDef))
{
RoslynDebug.AssertNotNull(deletedMethodDefinitions);

var oldPropertyDef = (IPropertyDefinition)propertyDef.GetCciAdapter();

// Because deleted property information comes from the associated symbol of the deleted accessors, its safe
// to assume that everything will be in the dictionary. We wouldn't be here it if wasn't.
var getter = oldPropertyDef.Getter is null ? null : deletedMethodDefinitions[(IMethodDefinition)oldPropertyDef.Getter];
var setter = oldPropertyDef.Setter is null ? null : deletedMethodDefinitions[(IMethodDefinition)oldPropertyDef.Setter];
var newPropertyDef = new DeletedPropertyDefinition(oldPropertyDef, getter, setter, typeDef, _typesUsedByDeletedMembers);
_propertyDefs.AddUpdated(newPropertyDef);
}

var implementingMethods = ArrayBuilder<int>.GetInstance();

// First, visit all MethodImplementations and add to this.methodImplList.
Expand Down Expand Up @@ -765,8 +726,7 @@ private bool AddDefIfNecessary<T>(DefinitionIndex<T> defIndex, T def, SymbolChan
defIndex.AddUpdated(def);
return false;
case SymbolChange.ContainsChanges:
Debug.Assert(def is INestedTypeDefinition);
// Changes to members within nested type only.
Debug.Assert(def is INestedTypeDefinition or IPropertyDefinition or IEventDefinition);
return false;
default:
// No changes to member or container.
Expand Down Expand Up @@ -1744,7 +1704,7 @@ public void Add(MethodImplKey item)
private sealed class DeltaReferenceIndexer : ReferenceIndexer
{
private readonly SymbolChanges _changes;
private readonly Dictionary<ITypeDefinition, ImmutableDictionary<IMethodDefinition, DeletedMethodDefinition>> _deletedTypeMembers;
private readonly IReadOnlyDictionary<ITypeDefinition, ImmutableArray<DeletedMethodDefinition>> _deletedTypeMembers;

public DeltaReferenceIndexer(DeltaMetadataWriter writer)
: base(writer)
Expand Down Expand Up @@ -1824,7 +1784,7 @@ public override void Visit(ITypeDefinition typeDefinition)
// We need to visit deleted members to ensure attribute method references are recorded
if (_deletedTypeMembers.TryGetValue(typeDefinition, out var deletedMembers))
{
this.Visit(deletedMembers.Values);
this.Visit(deletedMembers);
}
}
}
Expand Down
Loading