This repository was archived by the owner on Apr 14, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 133
Improve handling of circular and imported class bases #1497
Merged
Merged
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
38ca65b
Add test for GPy (fails)
bef7e5d
Disambiguate bases
3e8f788
Handle base loops better
b390c13
Merge master
41344cc
GPy test (on ignore)
44cc5eb
PT feedback + test
5ab9697
PR feedback
a4578c3
Pass template class
e08ff18
Add declaration position extension
e90bffc
Pass template class
2ea4da0
Change eval to scope
09e88d0
Add test for scoped import
ff4c48e
Adjust comparison
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,13 +23,14 @@ | |
|
||
namespace Microsoft.Python.Analysis.Types { | ||
internal partial class PythonClassType { | ||
MikhailArkhipov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
private readonly ReentrancyGuard<IPythonClassType> _genericSpecializationGuard = new ReentrancyGuard<IPythonClassType>(); | ||
private readonly ReentrancyGuard<IPythonClassType> _genericResolutionGuard = new ReentrancyGuard<IPythonClassType>(); | ||
private readonly object _genericParameterLock = new object(); | ||
|
||
private bool _isGeneric; | ||
private object _genericParameterLock = new object(); | ||
private Dictionary<string, PythonClassType> _specificTypeCache; | ||
private Dictionary<IGenericTypeParameter, IPythonType> _genericParameters; | ||
private IReadOnlyList<IGenericTypeParameter> _parameters = new List<IGenericTypeParameter>(); | ||
private ReentrancyGuard<IPythonClassType> _genericSpecializationGuard = new ReentrancyGuard<IPythonClassType>(); | ||
private ReentrancyGuard<IPythonClassType> _genericResolutionGuard = new ReentrancyGuard<IPythonClassType>(); | ||
|
||
#region IGenericType | ||
/// <summary> | ||
|
@@ -60,23 +61,19 @@ internal partial class PythonClassType { | |
/// </summary> | ||
public IPythonType CreateSpecificType(IArgumentSet args) { | ||
lock (_genericParameterLock) { | ||
var genericTypeParameters = GetTypeParameters(); | ||
var newGenericTypeParameters = GetTypeParameters(); | ||
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.
|
||
var newBases = new List<IPythonType>(); | ||
|
||
// Get map of generic type parameter to specific type - fill in what type goes to what | ||
// type parameter T -> int, U -> str, etc. | ||
var genericTypeToSpecificType = GetSpecificTypes(args, genericTypeParameters, newBases); | ||
var genericTypeToSpecificType = GetSpecificTypes(args, newGenericTypeParameters, newBases); | ||
|
||
PythonClassType classType = new PythonClassType(BaseName, new Location(DeclaringModule)); | ||
var classType = new PythonClassType(BaseName, new Location(DeclaringModule)); | ||
// Storing generic parameters allows methods returning generic types | ||
// to know what type parameter returns what specific type | ||
StoreGenericParameters(classType, genericTypeParameters, genericTypeToSpecificType); | ||
classType.StoreGenericParameters(GenericParameters, newGenericTypeParameters, genericTypeToSpecificType); | ||
|
||
// Set generic name | ||
if (!classType._genericParameters.IsNullOrEmpty()) { | ||
classType._genericName = CodeFormatter.FormatSequence(BaseName, '[', classType._genericParameters.Values); | ||
} | ||
|
||
// Locking so threads can only access class after it's been initialized | ||
// Store generic parameters first so name updates correctly, then check if class type has been cached | ||
_specificTypeCache = _specificTypeCache ?? new Dictionary<string, PythonClassType>(); | ||
|
@@ -105,8 +102,8 @@ public IPythonType CreateSpecificType(IArgumentSet args) { | |
// Get list of bases that are generic but not generic class parameters, e.g A[T], B[T] but not Generic[T1, T2] | ||
var genericTypeBases = bases.Except(genericClassParameters).OfType<IGenericType>().Where(g => g.IsGeneric).ToArray(); | ||
|
||
// Removing all generic bases, and will only specialize genericTypeBases, remove generic class paramters entirely | ||
// We remove generic class paramters entirely because the type information is now stored in GenericParameters field | ||
// Removing all generic bases, and will only specialize genericTypeBases, remove generic class parameters entirely | ||
// We remove generic class parameters entirely because the type information is now stored in GenericParameters field | ||
// We still need generic bases so we can specialize them | ||
var specificBases = bases.Except(genericTypeBases).Except(genericClassParameters).ToList(); | ||
|
||
|
@@ -127,12 +124,12 @@ public IPythonType CreateSpecificType(IArgumentSet args) { | |
} | ||
|
||
// Set specific class bases | ||
classType.SetBases(specificBases.Concat(newBases)); | ||
classType.SetBases(specificBases.Concat(newBases), args.Eval); | ||
// Now that parameters are set, check if class is generic | ||
classType._parameters = classType._genericParameters.Values.Distinct().OfType<IGenericTypeParameter>().ToList(); | ||
classType.DecideGeneric(); | ||
// Transfer members from generic to specific type. | ||
SetClassMembers(classType, args); | ||
classType.SetClassMembers(args); | ||
} | ||
return classType; | ||
} | ||
|
@@ -154,7 +151,7 @@ private IGenericTypeParameter[] GetTypeParameters() { | |
var genericClassParameter = bases.OfType<IGenericClassParameter>().FirstOrDefault(); | ||
|
||
// If Generic[...] is present, ordering of type variables is determined from that | ||
if (genericClassParameter != null && genericClassParameter.TypeParameters != null) { | ||
if (genericClassParameter?.TypeParameters != null) { | ||
fromBases.UnionWith(genericClassParameter.TypeParameters); | ||
} else { | ||
// otherwise look at the generic class bases | ||
|
@@ -214,7 +211,7 @@ private IReadOnlyDictionary<IGenericTypeParameter, IPythonType> GetSpecificTypes | |
// for the copy constructor. Consider 'class A(Generic[K, V], Mapping[K, V])' | ||
// constructed as 'd = {1:'a', 2:'b'}; A(d)'. Here we look through bases | ||
// and see if any matches the builtin type id. For example, Mapping or Dict | ||
// will have BultinTypeId.Dict and we can figure out specific types from | ||
// will have BuiltinTypeId.Dict and we can figure out specific types from | ||
// the content of the collection. | ||
var b = _bases.OfType<IGenericType>().Where(g => g.IsGeneric).FirstOrDefault(x => x.TypeId == type.TypeId); | ||
if (b != null && !b.Parameters.IsNullOrEmpty()) { | ||
|
@@ -259,33 +256,40 @@ private IReadOnlyDictionary<IGenericTypeParameter, IPythonType> GetSpecificTypes | |
/// Points the generic type parameter in class type to their corresponding specific type (or a generic | ||
/// type parameter if no specific type was provided) | ||
/// </summary> | ||
private void StoreGenericParameters(PythonClassType classType, IGenericTypeParameter[] genericParameters, IReadOnlyDictionary<IGenericTypeParameter, IPythonType> genericToSpecificTypes) { | ||
private void StoreGenericParameters( | ||
IReadOnlyDictionary<IGenericTypeParameter, IPythonType> currentGenericParameters, | ||
IEnumerable<IGenericTypeParameter> newGenericParameters, | ||
IReadOnlyDictionary<IGenericTypeParameter, IPythonType> genericToSpecificTypes) { | ||
|
||
// copy original generic parameters over and try to fill them in | ||
classType._genericParameters = new Dictionary<IGenericTypeParameter, IPythonType>(GenericParameters.ToDictionary(k => k.Key, k => k.Value)); | ||
_genericParameters = currentGenericParameters.ToDictionary(k => k.Key, k => k.Value); | ||
|
||
// Case when creating a new specific class type | ||
if (Parameters.Count == 0) { | ||
MikhailArkhipov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Assign class type generic type parameters to specific types | ||
for (var i = 0; i < genericParameters.Length; i++) { | ||
var gb = genericParameters[i]; | ||
classType._genericParameters[gb] = genericToSpecificTypes.TryGetValue(gb, out var v) ? v : null; | ||
foreach (var gb in newGenericParameters) { | ||
_genericParameters[gb] = genericToSpecificTypes.TryGetValue(gb, out var v) ? v : null; | ||
} | ||
} else { | ||
// When Parameters field is not empty then need to update generic parameters field | ||
foreach (var gp in GenericParameters.Keys) { | ||
if (GenericParameters[gp] is IGenericTypeParameter specificType) { | ||
foreach (var gp in currentGenericParameters.Keys) { | ||
if (currentGenericParameters[gp] is IGenericTypeParameter specificType) { | ||
// Get unfilled type parameter or type parameter that was filled with another type parameter | ||
// and try to fill it in | ||
// e.g | ||
// class A(Generic[T]): | ||
// class B(A[U]) | ||
// A has T => U | ||
classType._genericParameters[gp] = genericToSpecificTypes.TryGetValue(specificType, out var v) ? v : null; | ||
_genericParameters[gp] = genericToSpecificTypes.TryGetValue(specificType, out var v) ? v : null; | ||
} | ||
} | ||
} | ||
|
||
if (!_genericParameters.IsNullOrEmpty()) { | ||
_genericName = CodeFormatter.FormatSequence(BaseName, '[', _genericParameters.Values); | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Given generic type such as Generic[T1, T2, ...] attempts to extract specific types | ||
/// for its parameters from an argument value. Handles common cases such as dictionary, | ||
|
@@ -331,24 +335,24 @@ private void GetSpecificTypeFromArgumentValue(IGenericType gt, object argumentVa | |
/// Transfers members from generic class to the specific class type | ||
/// while instantiating specific types for the members. | ||
/// </summary> | ||
private void SetClassMembers(PythonClassType classType, IArgumentSet args) { | ||
private void SetClassMembers(IArgumentSet args) { | ||
// Add members from the template class (this one). | ||
// Members must be clones rather than references since | ||
// we are going to set specific types on them. | ||
classType.AddMembers(this, true); | ||
AddMembers(this, true); | ||
MikhailArkhipov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Resolve return types of methods, if any were annotated as generics | ||
var members = classType.GetMemberNames() | ||
var members = GetMemberNames() | ||
.Except(new[] { "__class__", "__bases__", "__base__" }) | ||
.ToDictionary(n => n, classType.GetMember); | ||
.ToDictionary(n => n, GetMember); | ||
|
||
// Create specific types. | ||
// Functions handle generics internally upon the call to Call. | ||
foreach (var m in members) { | ||
switch (m.Value) { | ||
case IPythonTemplateType tt when tt.IsGeneric(): { | ||
var specificType = tt.CreateSpecificType(args); | ||
classType.AddMember(m.Key, specificType, true); | ||
AddMember(m.Key, specificType, true); | ||
break; | ||
} | ||
case IPythonInstance inst: { | ||
|
@@ -359,12 +363,12 @@ private void SetClassMembers(PythonClassType classType, IArgumentSet args) { | |
specificType = tt.CreateSpecificType(args); | ||
break; | ||
case IGenericTypeParameter gtd: | ||
classType.GenericParameters.TryGetValue(gtd, out specificType); | ||
GenericParameters.TryGetValue(gtd, out specificType); | ||
break; | ||
} | ||
|
||
if (specificType != null) { | ||
classType.AddMember(m.Key, new PythonInstance(specificType), true); | ||
AddMember(m.Key, new PythonInstance(specificType), true); | ||
} | ||
break; | ||
} | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Maybe update the comment now that it loses its old context:
Store declaration source so in case if variable gets overwritten...