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

Implement definite assignment analysis for tuples #13107

Merged
merged 4 commits into from
Aug 17, 2016
Merged
Show file tree
Hide file tree
Changes from 2 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
45 changes: 45 additions & 0 deletions src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -664,6 +664,28 @@ private void NoteWrite(BoundExpression n, BoundExpression value, bool read)
/// </summary>
protected int VariableSlot(Symbol symbol, int containingSlot = 0)
{
var fieldSymbol = symbol as TupleFieldSymbol;
if ((object)fieldSymbol != null)
{
TypeSymbol containingType = ((TupleTypeSymbol)symbol.ContainingType).UnderlyingNamedType;

// for tuple fields the varible indentifier represents the underlying field
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: variable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

symbol = fieldSymbol.TupleUnderlyingField;

// descend through Rest fields
// bail if corresponding slots do not exist
while (containingType != symbol.ContainingType)
{
var restField = containingType.GetMembers(TupleTypeSymbol.RestFieldName).FirstOrDefault() as FieldSymbol;
if((object)restField == null ||
!_variableSlot.TryGetValue(new VariableIdentifier(restField, containingSlot), out containingSlot))
{
return -1;
}
containingType = restField.Type.TupleUnderlyingTypeOrSelf();
}
}

int slot;
return (_variableSlot.TryGetValue(new VariableIdentifier(symbol, containingSlot), out slot)) ? slot : -1;
}
Expand All @@ -674,6 +696,29 @@ protected int VariableSlot(Symbol symbol, int containingSlot = 0)
protected int GetOrCreateSlot(Symbol symbol, int containingSlot = 0)
{
if (symbol is RangeVariableSymbol) return -1;

var fieldSymbol = symbol as TupleFieldSymbol;
if ((object)fieldSymbol != null)
{
TypeSymbol containingType = ((TupleTypeSymbol)symbol.ContainingType).UnderlyingNamedType;

// for tuple fields the varible indentifier represents the underlying field
symbol = fieldSymbol.TupleUnderlyingField;

// descend through Rest fields
// force corresponding slots if do not exist
while (containingType != symbol.ContainingType)
{
var restField = containingType.GetMembers(TupleTypeSymbol.RestFieldName).FirstOrDefault() as FieldSymbol;
if (restField == null)
{
return -1;
}
containingSlot = GetOrCreateSlot(restField, containingSlot);
containingType = restField.Type.TupleUnderlyingTypeOrSelf();
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be worth extracting a helper method for use here and in VariableSlot above: int GetTupleFieldSlot(TupleFieldSymbol field, int containingSlot, bool createIfMissing)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extracted as
int DescendThroughTupleRestFields(ref Symbol symbol, int containingSlot, bool forceContainingSlotsToExist)


In reply to: 74526518 [](ancestors = 74526518)


VariableIdentifier identifier = new VariableIdentifier(symbol, containingSlot);
int slot;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,14 @@ private FieldSymbol GetActualInstanceField(Symbol member, NamedTypeSymbol type)
switch (member.Kind)
{
case SymbolKind.Field:
// Do not report virtual tuple fields.
// They are additional aliases to the fields of the underlying struct or nested extensions.
// and as such are already accounted for via the nonvirtual fields.
if (member is TupleVirtualElementFieldSymbol)
{
return null;
}

var field = (FieldSymbol)member;
return (field.IsFixed || ShouldIgnoreStructField(field, field.Type)) ? null : field.AsMember(type);

Expand Down
26 changes: 21 additions & 5 deletions src/Compilers/CSharp/Portable/Symbols/Tuples/TupleFieldSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ internal class TupleFieldSymbol : WrappedFieldSymbol
public TupleFieldSymbol(TupleTypeSymbol container, FieldSymbol underlyingField, int tupleFieldId)
: base(underlyingField)
{
Debug.Assert(container.UnderlyingNamedType.Equals(underlyingField.ContainingType, TypeCompareKind.IgnoreDynamicAndTupleNames) || this is TupleVirtualElementFieldSymbol,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer we avoid depending on the identity of particular internal implementation classes. Can this is TupleVirtualElementFieldSymbol be replaced by this.IsVirtualTupleField?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of this assert is to actually check the internal implementation. I think it is ok to be strict about types here.

"virtual fields should be represented by " + nameof(TupleVirtualElementFieldSymbol));

_containingTuple = container;
_tupleFieldId = tupleFieldId;
}
Expand Down Expand Up @@ -185,18 +188,31 @@ public override Symbol AssociatedSymbol
}

/// <summary>
/// Represents an element field of a tuple type (such as (int a, byte b).a, or (int a, byte b).b)
/// that is backed by a real field with a different name within the tuple underlying type.
/// Represents an element field of a tuple type that is not backed by a real field
/// with the same name within the tuple underlying type.
///
/// Examples
/// // alias to Item1 with a different name
/// (int a, byte b).a
///
/// // not backed directly by the underlying type
/// (int i1, int i2, int i3, int i4, int i5, int i6, int i7, int i8).i8
///
/// NOTE: For any virtual element, there is a nonvirtual way to access the same underlying field.
/// In scenarios where we need to enumerate actual fields of a struct,
/// virtual fields should be ignored.
/// </summary>
internal sealed class TupleRenamedElementFieldSymbol : TupleElementFieldSymbol
internal sealed class TupleVirtualElementFieldSymbol : TupleElementFieldSymbol
{
private readonly string _name;

public TupleRenamedElementFieldSymbol(TupleTypeSymbol container, FieldSymbol underlyingField, string name, int tupleElementOrdinal, Location location)
public TupleVirtualElementFieldSymbol(TupleTypeSymbol container, FieldSymbol underlyingField, string name, int tupleElementOrdinal, Location location)
: base(container, underlyingField, tupleElementOrdinal, location)
{
Debug.Assert(name != null);
Debug.Assert(name != underlyingField.Name);
Debug.Assert(name != underlyingField.Name || !container.UnderlyingNamedType.Equals(underlyingField.ContainingType, TypeCompareKind.IgnoreDynamicAndTupleNames),
"fields that map directly to underlying should not be represented by " + nameof(TupleVirtualElementFieldSymbol));

_name = name;
}

Expand Down
17 changes: 6 additions & 11 deletions src/Compilers/CSharp/Portable/Symbols/Tuples/TupleTypeSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ internal sealed class TupleTypeSymbol : WrappedNamedTypeSymbol

internal const int RestPosition = 8; // The Rest field is in 8th position
internal const string TupleTypeName = "ValueTuple";
internal const string RestFieldName = "Rest";

private TupleTypeSymbol(Location locationOpt, NamedTypeSymbol underlyingType, ImmutableArray<Location> elementLocations, ImmutableArray<string> elementNames, ImmutableArray<TypeSymbol> elementTypes)
: this(locationOpt == null ? ImmutableArray<Location>.Empty : ImmutableArray.Create(locationOpt),
Expand Down Expand Up @@ -813,21 +814,15 @@ private ImmutableArray<Symbol> CreateMembers()
if (namesOfVirtualFields[tupleFieldIndex] != defaultName)
{
// The name given doesn't match default name Item8, etc.
members.Add(new TupleRenamedElementFieldSymbol(this, fieldSymbol, defaultName, -members.Count - 1, null));
// Add a field with default name and make it virtual since we are not at the top level
members.Add(new TupleVirtualElementFieldSymbol(this, fieldSymbol, defaultName, -members.Count - 1, null));
}

// Add a field with the given name
var location = _elementLocations.IsDefault ? null : _elementLocations[tupleFieldIndex];
members.Add(new TupleVirtualElementFieldSymbol(this, fieldSymbol, namesOfVirtualFields[tupleFieldIndex],
tupleFieldIndex, location));

if (field.Name == namesOfVirtualFields[tupleFieldIndex])
{
members.Add(new TupleElementFieldSymbol(this, fieldSymbol, tupleFieldIndex, location));
}
else
{
members.Add(new TupleRenamedElementFieldSymbol(this, fieldSymbol, namesOfVirtualFields[tupleFieldIndex],
tupleFieldIndex, location));
}
}
else if (field.Name == namesOfVirtualFields[tupleFieldIndex])
{
Expand All @@ -842,7 +837,7 @@ private ImmutableArray<Symbol> CreateMembers()
// Add a field with the given name
if (namesOfVirtualFields[tupleFieldIndex] != null)
{
members.Add(new TupleRenamedElementFieldSymbol(this, fieldSymbol, namesOfVirtualFields[tupleFieldIndex], tupleFieldIndex,
members.Add(new TupleVirtualElementFieldSymbol(this, fieldSymbol, namesOfVirtualFields[tupleFieldIndex], tupleFieldIndex,
_elementLocations.IsDefault ? null : _elementLocations[tupleFieldIndex]));
}
}
Expand Down
Loading