Skip to content

Commit

Permalink
Made tests to rely less on type checks when verifying tuple field sym…
Browse files Browse the repository at this point in the history 
…bols

Fixed some typos.
Made virtual tuple elements to have right element IDs (needed since we no longer want to rely on symbol types)
Made virtual tuple field equality be sensitive to names. (needed since we no longer can rely on only default elements having IDs)
  • Loading branch information
@VSadov
VSadov committed Aug 17, 2016
1 parent 2e0451a commit 9d18615
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 43 deletions.
16 changes: 9 additions & 7 deletions src/Compilers/CSharp/Portable/FlowAnalysis/DataFlowPass.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -705,20 +705,22 @@ protected int GetOrCreateSlot(Symbol symbol, int containingSlot = 0)
return slot;
}

// Descends through Rest fields of a tuple is "symbol" is an extended field
// As a result the "symbol" will be adjusted to be the field of the innermost tuple
// and a corresponding containingSlot is returned.
// Return value -1 indicates a failure which could happen for the following reasons
// a) Rest field does not exist, which could happen in rare error scenarios involving broken ValueTuple types
// b) Rest is not tracked already and forceSlotsToExist is false (otherwise we create slots on demand)
/// <summary>
/// Descends through Rest fields of a tuple if "symbol" is an extended field
/// As a result the "symbol" will be adjusted to be the field of the innermost tuple
/// and a corresponding containingSlot is returned.
/// Return value -1 indicates a failure which could happen for the following reasons
/// a) Rest field does not exist, which could happen in rare error scenarios involving broken ValueTuple types
/// b) Rest is not tracked already and forceSlotsToExist is false (otherwise we create slots on demand)
/// </summary>
private int DescendThroughTupleRestFields(ref Symbol symbol, int containingSlot, bool forceContainingSlotsToExist)
{
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
// for tuple fields the variable identifier represents the underlying field
symbol = fieldSymbol.TupleUnderlyingField;

// descend through Rest fields
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Symbols/FieldSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ public virtual bool IsTupleField
}

/// <summary>
/// Returns Ture when field symbol is not mapped directly to a field in the underlying tuple struct.
/// Returns True when field symbol is not mapped directly to a field in the underlying tuple struct.
/// </summary>
internal virtual bool IsVirtualTupleField
{
Expand Down
21 changes: 13 additions & 8 deletions src/Compilers/CSharp/Portable/Symbols/Tuples/TupleFieldSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ internal class TupleFieldSymbol : WrappedFieldSymbol
protected readonly TupleTypeSymbol _containingTuple;

/// <summary>
/// If this field represents a tuple element (including the name match),
/// If this field represents a tuple element,
/// id is an index of the element (zero-based).
/// Otherwise, (-1 - [index in members array]);
/// </summary>
Expand All @@ -40,7 +40,7 @@ public override bool IsTupleField
}

/// <summary>
/// If this field represents a tuple element (including the name match),
/// If this field represents a tuple element,
/// id is an index of the element (zero-based).
/// Otherwise, (-1 - [index in members array]);
/// </summary>i
Expand Down Expand Up @@ -103,22 +103,27 @@ internal override DiagnosticInfo GetUseSiteDiagnostic()

public override sealed int GetHashCode()
{
return Hash.Combine(_containingTuple.GetHashCode(), _tupleFieldId.GetHashCode());
return Hash.Combine(
Hash.Combine(_containingTuple.GetHashCode(), _tupleFieldId.GetHashCode()),
this.Name.GetHashCode());
}

public override sealed bool Equals(object obj)
{
return Equals(obj as TupleFieldSymbol);
}
var other = obj as TupleFieldSymbol;

public bool Equals(TupleFieldSymbol other)
{
if ((object)other == this)
{
return true;
}

return (object)other != null && _tupleFieldId == other._tupleFieldId && _containingTuple == other._containingTuple;
// note we have to compare both index and name because
// in nameless tuple there could be fields that differ only by index
// and in named tupoles there could be fields that differ only by name
return (object)other != null &&
_tupleFieldId == other.TupleElementIndex &&
_containingTuple == other._containingTuple &&
this.Name == other.Name;
}
}

Expand Down
21 changes: 15 additions & 6 deletions src/Compilers/CSharp/Portable/Symbols/Tuples/TupleTypeSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,15 @@ internal static int IsElementNameReserved(string name)
return 0;
}

return MatchesCanonicalElementName(name);
}

/// <summary>
/// Returns 3 for "Item3".
/// Returns -1 otherwise.
/// </summary>
private static int MatchesCanonicalElementName(string name)
{
if (name.StartsWith("Item", StringComparison.Ordinal))
{
string tail = name.Substring(4);
Expand Down Expand Up @@ -724,13 +733,13 @@ private ImmutableArray<FieldSymbol> CollectTupleElementFields()
continue;
}

int index = (member as TupleFieldSymbol)?.TupleElementIndex ??
((TupleErrorFieldSymbol)member).TupleElementIndex;
var field = (FieldSymbol)member;
var index = field.TupleElementIndex;

if (index >= 0)
if (index >= 0 && index == MatchesCanonicalElementName(field.Name) - 1)
{
Debug.Assert((object)builder[index] == null);
builder[index] = (FieldSymbol)member;
builder[index] = field;
}
}

Expand Down Expand Up @@ -815,7 +824,7 @@ private ImmutableArray<Symbol> CreateMembers()
{
// The name given doesn't match default name Item8, etc.
// 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));
members.Add(new TupleVirtualElementFieldSymbol(this, fieldSymbol, defaultName, tupleFieldIndex, null));
}

// Add a field with the given name
Expand All @@ -832,7 +841,7 @@ private ImmutableArray<Symbol> CreateMembers()
else
{
// Add a field with default name
members.Add(new TupleFieldSymbol(this, fieldSymbol, -members.Count - 1));
members.Add(new TupleFieldSymbol(this, fieldSymbol, tupleFieldIndex));

// Add a field with the given name
if (namesOfVirtualFields[tupleFieldIndex] != null)
Expand Down
72 changes: 51 additions & 21 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenTupleTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8630,9 +8630,9 @@ static void Main()
var m2Item1 = (FieldSymbol)m2Tuple.GetMembers()[0];
var m2a2 = (FieldSymbol)m2Tuple.GetMembers()[1];

Assert.IsType<TupleElementFieldSymbol>(m1Item1);
Assert.IsType<TupleFieldSymbol>(m2Item1);
Assert.IsType<TupleVirtualElementFieldSymbol>(m2a2);
AssertNonvirtualTupleElementField(m1Item1);
AssertNonvirtualTupleElementField(m2Item1);
AssertVirtualTupleElementField(m2a2);

Assert.True(m1Item1.IsTupleField);
Assert.Same(m1Item1, m1Item1.OriginalDefinition);
Expand Down Expand Up @@ -8879,7 +8879,7 @@ static void Main()

var m3Item8 = (FieldSymbol)m3Tuple.GetMembers("Item8").Single();

Assert.IsType<TupleVirtualElementFieldSymbol>(m3Item8);
AssertVirtualTupleElementField(m3Item8);

Assert.True(m3Item8.IsTupleField);
Assert.Same(m3Item8, m3Item8.OriginalDefinition);
Expand Down Expand Up @@ -9073,7 +9073,7 @@ static void Main()

var m4Item8 = (FieldSymbol)m4Tuple.GetMembers("Item8").Single();

Assert.IsType<TupleVirtualElementFieldSymbol>(m4Item8);
AssertVirtualTupleElementField(m4Item8);

Assert.True(m4Item8.IsTupleField);
Assert.Same(m4Item8, m4Item8.OriginalDefinition);
Expand All @@ -9092,7 +9092,7 @@ static void Main()

var m4h4 = (FieldSymbol)m4Tuple.GetMembers("h4").Single();

Assert.IsType<TupleVirtualElementFieldSymbol>(m4h4);
AssertVirtualTupleElementField(m4h4);

Assert.True(m4h4.IsTupleField);
Assert.Same(m4h4, m4h4.OriginalDefinition);
Expand Down Expand Up @@ -9321,7 +9321,7 @@ static void Main()

var m5Item8 = (FieldSymbol)m5Tuple.GetMembers("Item8").Single();

Assert.IsType<TupleVirtualElementFieldSymbol>(m5Item8);
AssertVirtualTupleElementField(m5Item8);

Assert.True(m5Item8.IsTupleField);
Assert.Same(m5Item8, m5Item8.OriginalDefinition);
Expand Down Expand Up @@ -9691,7 +9691,7 @@ static void Main()

var m8Item8 = (FieldSymbol)m8Tuple.GetMembers("Item8").Single();

Assert.IsType<TupleVirtualElementFieldSymbol>(m8Item8);
AssertVirtualTupleElementField(m8Item8);

Assert.True(m8Item8.IsTupleField);
Assert.Same(m8Item8, m8Item8.OriginalDefinition);
Expand All @@ -9711,7 +9711,7 @@ static void Main()

var m8Item1 = (FieldSymbol)m8Tuple.GetMembers("Item1").Last();

Assert.IsType<TupleVirtualElementFieldSymbol>(m8Item1);
AssertVirtualTupleElementField(m8Item1);

Assert.True(m8Item1.IsTupleField);
Assert.Same(m8Item1, m8Item1.OriginalDefinition);
Expand Down Expand Up @@ -9890,9 +9890,9 @@ static void Main()
var m2Item1 = (FieldSymbol)m2Tuple.GetMembers()[0];
var m2a2 = (FieldSymbol)m2Tuple.GetMembers()[1];

Assert.IsType<TupleElementFieldSymbol>(m1Item1);
Assert.IsType<TupleFieldSymbol>(m2Item1);
Assert.IsType<TupleVirtualElementFieldSymbol>(m2a2);
AssertNonvirtualTupleElementField(m1Item1);
AssertNonvirtualTupleElementField(m2Item1);
AssertVirtualTupleElementField(m2a2);

Assert.True(m1Item1.IsTupleField);
Assert.Same(m1Item1, m1Item1.OriginalDefinition);
Expand Down Expand Up @@ -10226,9 +10226,9 @@ static void Test03()
var m102Item20 = (FieldSymbol)m102Tuple.GetMembers("Item20").Single();
var m102a = (FieldSymbol)m102Tuple.GetMembers("a").Single();

Assert.IsType<TupleElementFieldSymbol>(m10Item1);
Assert.IsType<TupleFieldSymbol>(m102Item20);
Assert.IsType<TupleVirtualElementFieldSymbol>(m102a);
AssertNonvirtualTupleElementField(m10Item1);
AssertTupleNonElementField(m102Item20);
AssertVirtualTupleElementField(m102a);

Assert.Equal("System.ObsoleteAttribute", m10Item1.GetAttributes().Single().ToString());
Assert.Equal("System.ObsoleteAttribute", m102Item20.GetAttributes().Single().ToString());
Expand All @@ -10239,10 +10239,10 @@ static void Test03()
var m102Item2 = (FieldSymbol)m102Tuple.GetMembers("Item2").Single();
var m102b = (FieldSymbol)m102Tuple.GetMembers("b").Single();

Assert.IsType<TupleElementFieldSymbol>(m10Item2);
Assert.IsType<TupleFieldSymbol>(m102Item2);
Assert.IsType<TupleFieldSymbol>(m102Item21);
Assert.IsType<TupleVirtualElementFieldSymbol>(m102b);
AssertNonvirtualTupleElementField(m10Item2);
AssertNonvirtualTupleElementField(m102Item2);
AssertTupleNonElementField(m102Item21);
AssertVirtualTupleElementField(m102b);

Assert.Equal(20, m10Item2.TypeLayoutOffset);
Assert.Equal(20, m102Item2.TypeLayoutOffset);
Expand All @@ -10256,8 +10256,8 @@ static void Test03()
var m103Item2 = (FieldSymbol)m103Tuple.GetMembers("Item2").Last();
var m103Item9 = (FieldSymbol)m103Tuple.GetMembers("Item9").Single();

Assert.IsType<TupleVirtualElementFieldSymbol>(m103Item2);
Assert.IsType<TupleVirtualElementFieldSymbol>(m103Item9);
AssertVirtualTupleElementField(m103Item2);
AssertVirtualTupleElementField(m103Item9);
Assert.Null(m103Item2.TypeLayoutOffset);
Assert.Equal(20, m103Item2.TupleUnderlyingField.TypeLayoutOffset);
Assert.Null(m103Item9.TypeLayoutOffset);
Expand Down Expand Up @@ -10296,6 +10296,36 @@ static void Test03()
Assert.Equal("System.ObsoleteAttribute", m10E2.GetAttributes().Single().ToString());
}

private void AssertTupleNonElementField(FieldSymbol sym)
{
Assert.True(sym.IsTupleField);
Assert.False(sym.IsVirtualTupleField);

//it is not an element so index must be negative
Assert.True(sym.TupleElementIndex < 0);
}

private void AssertVirtualTupleElementField(FieldSymbol sym)
{
Assert.True(sym.IsTupleField);
Assert.True(sym.IsVirtualTupleField);

//it is an element so must have nonnegative index
Assert.True(sym.TupleElementIndex >= 0);
}

private void AssertNonvirtualTupleElementField(FieldSymbol sym)
{
Assert.True(sym.IsTupleField);
Assert.False(sym.IsVirtualTupleField);

//it is an element so must have nonnegative index
Assert.True(sym.TupleElementIndex >= 0);

//if it was 8th or after, it would be virtual
Assert.True(sym.TupleElementIndex < TupleTypeSymbol.RestPosition - 1);
}

[Fact]
public void CustomValueTupleWithGenericMethod()
{
Expand Down

0 comments on commit 9d18615

Please sign in to comment.