-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Changes from 3 commits
56f7010
fc01769
2e0451a
9d18615
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -664,6 +664,8 @@ private void NoteWrite(BoundExpression n, BoundExpression value, bool read) | |
/// </summary> | ||
protected int VariableSlot(Symbol symbol, int containingSlot = 0) | ||
{ | ||
containingSlot = DescendThroughTupleRestFields(ref symbol, containingSlot, forceContainingSlotsToExist: false); | ||
|
||
int slot; | ||
return (_variableSlot.TryGetValue(new VariableIdentifier(symbol, containingSlot), out slot)) ? slot : -1; | ||
} | ||
|
@@ -674,6 +676,9 @@ protected int VariableSlot(Symbol symbol, int containingSlot = 0) | |
protected int GetOrCreateSlot(Symbol symbol, int containingSlot = 0) | ||
{ | ||
if (symbol is RangeVariableSymbol) return -1; | ||
|
||
containingSlot = DescendThroughTupleRestFields(ref symbol, containingSlot, forceContainingSlotsToExist: true); | ||
|
||
VariableIdentifier identifier = new VariableIdentifier(symbol, containingSlot); | ||
int slot; | ||
|
||
|
@@ -700,6 +705,52 @@ 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider making this into xml doc, so it pops up in intellisense. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
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 | ||
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 ((object)restField == null) | ||
{ | ||
return -1; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
|
||
if (forceContainingSlotsToExist) | ||
{ | ||
containingSlot = GetOrCreateSlot(restField, containingSlot); | ||
} | ||
else | ||
{ | ||
if (!_variableSlot.TryGetValue(new VariableIdentifier(restField, containingSlot), out containingSlot)) | ||
{ | ||
return -1; | ||
} | ||
} | ||
|
||
containingType = restField.Type.TupleUnderlyingTypeOrSelf(); | ||
} | ||
} | ||
|
||
return containingSlot; | ||
} | ||
|
||
|
||
private void Normalize(ref LocalState state) | ||
{ | ||
int oldNext = state.Assigned.Capacity; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -384,6 +384,17 @@ public virtual bool IsTupleField | |
} | ||
} | ||
|
||
/// <summary> | ||
/// Returns Ture when field symbol is not mapped directly to a field in the underlying tuple struct. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: |
||
/// </summary> | ||
internal virtual bool IsVirtualTupleField | ||
{ | ||
get | ||
{ | ||
return false; | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// If this is a field of a tuple type, return corresponding underlying field from the | ||
/// tuple underlying type. Otherwise, null. In case of a malformed underlying type | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
} | ||
|
@@ -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; | ||
} | ||
|
||
|
@@ -223,5 +239,13 @@ public override Symbol AssociatedSymbol | |
return null; | ||
} | ||
} | ||
|
||
internal override bool IsVirtualTupleField | ||
{ | ||
get | ||
{ | ||
return true; | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First "is" on this line should be an "if".