From 975120be155e0acb1d32b90325491cd5f2d88ea2 Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Sun, 29 Mar 2020 20:06:20 -0700 Subject: [PATCH 01/25] replaced GetCombinedTypeResolution with a more general method that performs additional checks --- src/QsCompiler/Core/ConstructorExtensions.fs | 2 +- src/QsCompiler/DataStructures/SyntaxTree.fs | 5 + .../Transformations/ClassicallyControlled.cs | 114 +++++++++++++----- 3 files changed, 89 insertions(+), 32 deletions(-) diff --git a/src/QsCompiler/Core/ConstructorExtensions.fs b/src/QsCompiler/Core/ConstructorExtensions.fs index 2c3a8226ec..65cf870ed0 100644 --- a/src/QsCompiler/Core/ConstructorExtensions.fs +++ b/src/QsCompiler/Core/ConstructorExtensions.fs @@ -83,7 +83,7 @@ type TypedExpression with /// the ResolvedType is set to the type constructed by resolving it using ResolveTypeParameters and the given look-up. static member New (expr, typeParamResolutions : ImmutableDictionary<_,_>, exType, exInfo, range) = { Expression = expr - TypeArguments = typeParamResolutions |> Seq.map (fun kv -> fst kv.Key, snd kv.Key, kv.Value) |> ImmutableArray.CreateRange + TypeArguments = typeParamResolutions |> TypedExpression.AsTypeArguments ResolvedType = ResolvedType.ResolveTypeParameters typeParamResolutions exType InferredInformation = exInfo Range = range diff --git a/src/QsCompiler/DataStructures/SyntaxTree.fs b/src/QsCompiler/DataStructures/SyntaxTree.fs index 6ebbbb14a3..b2db81ed6c 100644 --- a/src/QsCompiler/DataStructures/SyntaxTree.fs +++ b/src/QsCompiler/DataStructures/SyntaxTree.fs @@ -364,6 +364,11 @@ type TypedExpression = { member this.TypeParameterResolutions = this.TypeArguments.ToImmutableDictionary((fun (origin, name, _) -> origin, name), (fun (_,_,t) -> t)) + /// Given a dictionary containing the type resolutions for an expression, + /// returns the corresponding ImmutableArray to initialize the TypeArguments with. + static member AsTypeArguments (typeParamResolutions : ImmutableDictionary<_,_>) = + typeParamResolutions |> Seq.map (fun kv -> fst kv.Key, snd kv.Key, kv.Value) |> ImmutableArray.CreateRange + /// Returns true if the expression is a call-like expression, and the arguments contain a missing expression. /// Returns false otherwise. static member public IsPartialApplication kind = diff --git a/src/QsCompiler/Transformations/ClassicallyControlled.cs b/src/QsCompiler/Transformations/ClassicallyControlled.cs index c39d2838b7..adca6b6c31 100644 --- a/src/QsCompiler/Transformations/ClassicallyControlled.cs +++ b/src/QsCompiler/Transformations/ClassicallyControlled.cs @@ -72,29 +72,80 @@ private class StatementTransformation : StatementTransformation parent) : base(parent) { } - /// - /// Get the combined type resolutions for a pair of nested resolutions, - /// resolving references in the inner resolutions to the outer resolutions. - /// - private TypeArgsResolution GetCombinedTypeResolution(TypeArgsResolution outer, TypeArgsResolution inner) + private static ImmutableDictionary>, ResolvedType> TryCompineTypeResolution + (QsQualifiedName parent, params ImmutableDictionary>, ResolvedType>[] resolutions) { - var outerDict = outer.ToDictionary(x => (x.Item1, x.Item2), x => x.Item3); - return inner.Select(innerRes => + var compined = ImmutableDictionary.CreateBuilder>, ResolvedType>(); + if (resolutions.Length == 0) return compined.ToImmutable(); + + //var parent = resolutions[0].Keys.First().Item1; WRONG... + bool EqualsParent(QsQualifiedName origin) => origin.Namespace.Value == parent.Namespace.Value && origin.Name.Value == parent.Name.Value; + static Tuple> AsTypeResolutionKey(QsTypeParameter tp) => Tuple.Create(tp.Origin, tp.TypeName); + + foreach (var resolution in resolutions) { - if (innerRes.Item3.Resolution is ResolvedTypeKind.TypeParameter typeParam && - outerDict.TryGetValue((typeParam.Item.Origin, typeParam.Item.TypeName), out var outerRes)) - { - outerDict.Remove((typeParam.Item.Origin, typeParam.Item.TypeName)); - return Tuple.Create(innerRes.Item1, innerRes.Item2, outerRes); - } - else + // Contains a lookup of all the keys in compined whose value needs to be updated + // if a certain type parameter is resolved by the corrently processed dictionary. + var mayBeReplaced = compined + .Where(kv => kv.Value.Resolution.IsTypeParameter) + .ToLookup( + kv => AsTypeResolutionKey((kv.Value.Resolution as ResolvedTypeKind.TypeParameter).Item), + kv => kv.Key); + + foreach (var keyValue in resolution) { - return innerRes; + foreach (var keyInCombined in mayBeReplaced[keyValue.Key]) + { + // Give an error if one of the values is a type parameter from the parent callable, + // but it isn't mapped to itself. + if (keyValue.Value.Resolution is ResolvedTypeKind.TypeParameter tp + && EqualsParent(tp.Item.Origin) + && (keyInCombined.Item2.Value != tp.Item.TypeName.Value || !EqualsParent(keyInCombined.Item1))) + { + throw new Exception("TOO RESTRICTIVE"); + // FIXME: PROPER DIAGNOSTIC + } + compined[keyInCombined] = keyValue.Value; + } + + if (EqualsParent(keyValue.Key.Item1)) + { + compined[keyValue.Key] = keyValue.Value; + } + else if (!mayBeReplaced.Contains(keyValue.Key)) + { + // FIXME: WE MAY NEED TO ALLOW THIS TO AVOID CHECKING EACH NODE IN A CYCLE + throw new Exception("ATTEMPTING TO RESOLVE TYPE PARAMETER THAT DOES NOT BELONG TO THIS CALLABLE"); + } } - }) - .Concat(outerDict.Select(x => Tuple.Create(x.Key.Item1, x.Key.Item2, x.Value))).ToImmutableArray(); + } + + return compined.ToImmutable(); } + /// + /// Get the combined type resolutions for a pair of nested resolutions, + /// resolving references in the inner resolutions to the outer resolutions. + /// + //private static TypeArgsResolution GetCombinedTypeResolution(TypeArgsResolution outer, TypeArgsResolution inner) + //{ + // var outerDict = outer.ToDictionary(x => (x.Item1, x.Item2), x => x.Item3); + // return inner.Select(innerRes => + // { + // if (innerRes.Item3.Resolution is ResolvedTypeKind.TypeParameter typeParam && + // outerDict.TryGetValue((typeParam.Item.Origin, typeParam.Item.TypeName), out var outerRes)) + // { + // outerDict.Remove((typeParam.Item.Origin, typeParam.Item.TypeName)); // FIXME: THERE IS A BUG HERE: THIS COULD OCCUR SEVERAL TIMES AS VALUE IN THE INNER DICT + // return Tuple.Create(innerRes.Item1, innerRes.Item2, outerRes); + // } + // else + // { + // return innerRes; + // } + // }) + // .Concat(outerDict.Select(x => Tuple.Create(x.Key.Item1, x.Key.Item2, x.Value))).ToImmutableArray(); + //} + /// /// Checks if the scope is valid for conversion to an operation call from the conditional control API. /// It is valid if there is exactly one statement in it and that statement is a call like expression statement. @@ -112,19 +163,20 @@ private TypeArgsResolution GetCombinedTypeResolution(TypeArgsResolution outer, T && !TypedExpression.IsPartialApplication(expr.Item.Expression) && call.Item1.Expression is ExpressionKind.Identifier) { - // We are dissolving the application of arguments here, so the call's type argument - // resolutions have to be moved to the 'identifier' sub expression. - - var callTypeArguments = expr.Item.TypeArguments; - var idTypeArguments = call.Item1.TypeArguments; - var combinedTypeArguments = GetCombinedTypeResolution(callTypeArguments, idTypeArguments); + var newCallIdentifier = call.Item1; + var callTypeArguments = expr.Item.TypeParameterResolutions; + var idTypeArguments = call.Item1.TypeParameterResolutions; // This relies on anything having type parameters must be a global callable. - var newCallIdentifier = call.Item1; - if (combinedTypeArguments.Any() - && newCallIdentifier.Expression is ExpressionKind.Identifier id - && id.Item1 is Identifier.GlobalCallable global) + if ( + newCallIdentifier.Expression is ExpressionKind.Identifier id + && id.Item1 is Identifier.GlobalCallable global + && (callTypeArguments.Any() || idTypeArguments.Any())) { + // We are dissolving the application of arguments here, so the call's type argument + // resolutions have to be moved to the 'identifier' sub expression. + var combinedTypeArguments = TryCompineTypeResolution(global.Item, idTypeArguments, callTypeArguments); + var globalCallable = SharedState.Compilation.Namespaces .Where(ns => ns.Name.Equals(global.Item.Namespace)) .Callables() @@ -142,8 +194,8 @@ private TypeArgsResolution GetCombinedTypeResolution(TypeArgsResolution outer, T id.Item1, QsNullable>.NewValue( callableTypeParameters - .Select(x => combinedTypeArguments.First(y => y.Item2.Equals(x.Item)).Item3).ToImmutableArray())), - combinedTypeArguments, + .Select(x => combinedTypeArguments.First(y => y.Key.Item2.Equals(x.Item)).Value).ToImmutableArray())), // FIXME: THIS IS WITHOUT ANY CHECK OF THE PARENT... + TypedExpression.AsTypeArguments(combinedTypeArguments), call.Item1.ResolvedType, call.Item1.InferredInformation, call.Item1.Range); @@ -322,7 +374,7 @@ private TypedExpression CreateApplyConditionallyExpression(TypedExpression condi // Takes a single TypedExpression of type Result and puts in into a // value array expression with the given expression as its only item. - TypedExpression BoxResultInArray(TypedExpression expression) => + static TypedExpression BoxResultInArray(TypedExpression expression) => new TypedExpression( ExpressionKind.NewValueArray(ImmutableArray.Create(expression)), TypeArgsResolution.Empty, @@ -551,7 +603,7 @@ private bool IsConditionedOnResultLiteralExpression(TypedExpression expression, } else if (expression.Expression is ExpressionKind.NEQ neq) { - QsResult FlipResult(QsResult result) => result.IsZero ? QsResult.One : QsResult.Zero; + static QsResult FlipResult(QsResult result) => result.IsZero ? QsResult.One : QsResult.Zero; if (neq.Item1.Expression is ExpressionKind.ResultLiteral literal1) { From 3b32b6e89fc6cb901c38d12a261bea965b69c36c Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 30 Mar 2020 07:54:59 -0700 Subject: [PATCH 02/25] one more case to handle --- .../SyntaxProcessor/SyntaxExtensions.fs | 4 +- src/QsCompiler/TextProcessor/QsKeywords.fs | 2 +- src/QsCompiler/TextProcessor/QsTypeParsing.fs | 4 +- .../TextProcessor/SyntaxExtensions.fs | 2 +- .../Transformations/CallGraphWalker.cs | 69 ++++++++++++++++- .../Transformations/ClassicallyControlled.cs | 77 +------------------ 6 files changed, 75 insertions(+), 83 deletions(-) diff --git a/src/QsCompiler/SyntaxProcessor/SyntaxExtensions.fs b/src/QsCompiler/SyntaxProcessor/SyntaxExtensions.fs index 3158e6fda2..6282abcb54 100644 --- a/src/QsCompiler/SyntaxProcessor/SyntaxExtensions.fs +++ b/src/QsCompiler/SyntaxProcessor/SyntaxExtensions.fs @@ -101,9 +101,9 @@ and private SymbolsFromExpr item : QsSymbol list * QsType list * QsExpression li | QsExpressionKind.InvalidExpr -> [], [], [item] let private AttributeAsCallExpr (sym : QsSymbol, ex : QsExpression) = - let compinedRange = QsPositionInfo.CombinedRange (sym.Range, ex.Range) + let combinedRange = QsPositionInfo.CombinedRange (sym.Range, ex.Range) let id = {Expression = QsExpressionKind.Identifier (sym, Null); Range = sym.Range} - {Expression = QsExpressionKind.CallLikeExpression(id, ex); Range = compinedRange} + {Expression = QsExpressionKind.CallLikeExpression(id, ex); Range = combinedRange} let rec private SymbolDeclarations (sym : QsSymbol) = match sym.Symbol with diff --git a/src/QsCompiler/TextProcessor/QsKeywords.fs b/src/QsCompiler/TextProcessor/QsKeywords.fs index b873770526..903955efd0 100644 --- a/src/QsCompiler/TextProcessor/QsKeywords.fs +++ b/src/QsCompiler/TextProcessor/QsKeywords.fs @@ -47,7 +47,7 @@ let private addFragmentHeader word = _FragmentHeaders.Add word |> ignore qsKeyword word -/// Given the keyword for two functors, constructs and returns the keyword for the compined functor +/// Given the keyword for two functors, constructs and returns the keyword for the combined functor /// under the assumption tha the order of the functors does not matter. /// Adds the keyword for the combined functor to the list of QsReservedKeywords, QsLanguageKeywords and QsFragmentHeaders. let private addFunctorCombination (word1 : QsKeyword, word2 : QsKeyword) = diff --git a/src/QsCompiler/TextProcessor/QsTypeParsing.fs b/src/QsCompiler/TextProcessor/QsTypeParsing.fs index 04947fb6cb..711112f72f 100644 --- a/src/QsCompiler/TextProcessor/QsTypeParsing.fs +++ b/src/QsCompiler/TextProcessor/QsTypeParsing.fs @@ -25,7 +25,7 @@ let private characteristicsExpression = new OperatorPrecedenceParser Characteristics.New // *needs* to be invalid if the compined range is Null! + QsPositionInfo.WithCombinedRange (lRange, rRange) kind InvalidSetExpr |> Characteristics.New // *needs* to be invalid if the combined range is Null! let private applyBinary operator _ (left : Characteristics) (right : Characteristics) = buildCombinedExpression (operator (left, right)) (left.Range, right.Range) @@ -186,7 +186,7 @@ let internal typeParser tupleType = ] let buildArrays p = let combine kind (lRange, rRange) = - QsPositionInfo.WithCombinedRange (lRange, rRange) kind InvalidType |> QsType.New // *needs* to be invalid if the compined range is Null! + QsPositionInfo.WithCombinedRange (lRange, rRange) kind InvalidType |> QsType.New // *needs* to be invalid if the combined range is Null! let rec applyArrays (t : QsType, item) = match item with | [] -> t diff --git a/src/QsCompiler/TextProcessor/SyntaxExtensions.fs b/src/QsCompiler/TextProcessor/SyntaxExtensions.fs index b90542b2a8..ce0b087e07 100644 --- a/src/QsCompiler/TextProcessor/SyntaxExtensions.fs +++ b/src/QsCompiler/TextProcessor/SyntaxExtensions.fs @@ -16,7 +16,7 @@ type QsPositionInfo with static member Range (pos1, pos2) = Value (QsPositionInfo.New pos1, QsPositionInfo.New pos2) - /// If the given right and left range both have contain a Value, returns the given kind as well as a Value with the compined range. + /// If the given right and left range both have contain a Value, returns the given kind as well as a Value with the combined range. /// Returns the given fallback and Null otherwise. static member WithCombinedRange (lRange, rRange) kind fallback = match (lRange, rRange) with diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index 4b962b7d3d..c454dc48ec 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -11,7 +11,7 @@ using Microsoft.Quantum.QsCompiler.Transformations.Core; -namespace Microsoft.Quantum.QsCompiler.Transformations.CallGraphWalker +namespace Microsoft.Quantum.QsCompiler.Transformations { using ExpressionKind = QsExpressionKind; using ResolvedTypeKind = QsTypeKind; @@ -31,11 +31,76 @@ public struct CallGraphNode public QsNullable> TypeArgs; } + /// + /// Get the combined type resolutions for a pair of nested resolutions, + /// resolving references in the inner resolutions to the outer resolutions. + /// + public static bool TryCombineTypeResolutions + (QsQualifiedName parent, out ImmutableDictionary>, ResolvedType> combined, + params ImmutableDictionary>, ResolvedType>[] resolutions) + { + if (parent == null) throw new ArgumentNullException(nameof(parent)); + var combinedBuilder = ImmutableDictionary.CreateBuilder>, ResolvedType>(); + var success = true; + + bool EqualsParent(QsQualifiedName origin) => origin.Namespace.Value == parent.Namespace.Value && origin.Name.Value == parent.Name.Value; + static Tuple> AsTypeResolutionKey(QsTypeParameter tp) => Tuple.Create(tp.Origin, tp.TypeName); + + foreach (var resolution in resolutions) + { + // Contains a lookup of all the keys in the combined resolutions whose value needs to be updated + // if a certain non-native type parameter is resolved by the currently processed dictionary. + var mayBeReplaced = combinedBuilder + .Select(kv => (kv.Key, kv.Value.Resolution as ResolvedTypeKind.TypeParameter)) + .Where(entry => entry.Item2 != null && !EqualsParent(entry.Item2.Item.Origin)) + .ToLookup( + entry => AsTypeResolutionKey(entry.Item2.Item), + entry => entry.Key); + + foreach (var entry in resolution) + { + var resolutionToTypeParam = entry.Value.Resolution as ResolvedTypeKind.TypeParameter; + var isResolutionToNative = resolutionToTypeParam != null && EqualsParent(resolutionToTypeParam.Item.Origin); + + // resolution of a non-native type parameter that is currently listed as value in the combined type resolution dictionary + foreach (var keyInCombined in mayBeReplaced[entry.Key]) + { + // If one of the values is a type parameter from the parent callable, + // but it isn't mapped to itself then the combined resolution is invalid. + var nontrivialResolutionToNative = isResolutionToNative && keyInCombined.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; + success = success && !nontrivialResolutionToNative; + combinedBuilder[keyInCombined] = entry.Value; + } + + // resolution of a native type parameter, i.e. a type parameter that belongs to the parent callable + if (EqualsParent(entry.Key.Item1)) + { + // A native type parameter cannot be resolved to another native type parameter, since this would constrain them. + var nontrivialResolutionToNative = isResolutionToNative && entry.Key.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; + success = success && !nontrivialResolutionToNative; + // TODO: ALSO CHECK IF THERE IS An EXISTING AND CONFLICTING NON-TRIVIAL RESOLUTION FOR A NATIVE ONE + combinedBuilder[entry.Key] = entry.Value; + } + else if (!mayBeReplaced.Contains(entry.Key)) + { + throw new ArgumentException("attempting to define resolution for type parameter that does not belong to parent callable"); + } + + } + } + + QsCompilerError.Verify( + combinedBuilder.Keys.All(key => EqualsParent(key.Item1)), + "for type parameter that does not belong to parent callable"); + combined = combinedBuilder.ToImmutable(); + return success; + } + /// /// This is a dictionary mapping source nodes to information about target nodes. This information is represented /// by a dictionary mapping target node to the edges pointing from the source node to the target node. /// - private Dictionary>> _Dependencies = + private readonly Dictionary>> _Dependencies = new Dictionary>>(); private QsNullable> RemovePositionFromTypeArgs(QsNullable> tArgs) => diff --git a/src/QsCompiler/Transformations/ClassicallyControlled.cs b/src/QsCompiler/Transformations/ClassicallyControlled.cs index adca6b6c31..fafe70faad 100644 --- a/src/QsCompiler/Transformations/ClassicallyControlled.cs +++ b/src/QsCompiler/Transformations/ClassicallyControlled.cs @@ -72,80 +72,6 @@ private class StatementTransformation : StatementTransformation parent) : base(parent) { } - private static ImmutableDictionary>, ResolvedType> TryCompineTypeResolution - (QsQualifiedName parent, params ImmutableDictionary>, ResolvedType>[] resolutions) - { - var compined = ImmutableDictionary.CreateBuilder>, ResolvedType>(); - if (resolutions.Length == 0) return compined.ToImmutable(); - - //var parent = resolutions[0].Keys.First().Item1; WRONG... - bool EqualsParent(QsQualifiedName origin) => origin.Namespace.Value == parent.Namespace.Value && origin.Name.Value == parent.Name.Value; - static Tuple> AsTypeResolutionKey(QsTypeParameter tp) => Tuple.Create(tp.Origin, tp.TypeName); - - foreach (var resolution in resolutions) - { - // Contains a lookup of all the keys in compined whose value needs to be updated - // if a certain type parameter is resolved by the corrently processed dictionary. - var mayBeReplaced = compined - .Where(kv => kv.Value.Resolution.IsTypeParameter) - .ToLookup( - kv => AsTypeResolutionKey((kv.Value.Resolution as ResolvedTypeKind.TypeParameter).Item), - kv => kv.Key); - - foreach (var keyValue in resolution) - { - foreach (var keyInCombined in mayBeReplaced[keyValue.Key]) - { - // Give an error if one of the values is a type parameter from the parent callable, - // but it isn't mapped to itself. - if (keyValue.Value.Resolution is ResolvedTypeKind.TypeParameter tp - && EqualsParent(tp.Item.Origin) - && (keyInCombined.Item2.Value != tp.Item.TypeName.Value || !EqualsParent(keyInCombined.Item1))) - { - throw new Exception("TOO RESTRICTIVE"); - // FIXME: PROPER DIAGNOSTIC - } - compined[keyInCombined] = keyValue.Value; - } - - if (EqualsParent(keyValue.Key.Item1)) - { - compined[keyValue.Key] = keyValue.Value; - } - else if (!mayBeReplaced.Contains(keyValue.Key)) - { - // FIXME: WE MAY NEED TO ALLOW THIS TO AVOID CHECKING EACH NODE IN A CYCLE - throw new Exception("ATTEMPTING TO RESOLVE TYPE PARAMETER THAT DOES NOT BELONG TO THIS CALLABLE"); - } - } - } - - return compined.ToImmutable(); - } - - /// - /// Get the combined type resolutions for a pair of nested resolutions, - /// resolving references in the inner resolutions to the outer resolutions. - /// - //private static TypeArgsResolution GetCombinedTypeResolution(TypeArgsResolution outer, TypeArgsResolution inner) - //{ - // var outerDict = outer.ToDictionary(x => (x.Item1, x.Item2), x => x.Item3); - // return inner.Select(innerRes => - // { - // if (innerRes.Item3.Resolution is ResolvedTypeKind.TypeParameter typeParam && - // outerDict.TryGetValue((typeParam.Item.Origin, typeParam.Item.TypeName), out var outerRes)) - // { - // outerDict.Remove((typeParam.Item.Origin, typeParam.Item.TypeName)); // FIXME: THERE IS A BUG HERE: THIS COULD OCCUR SEVERAL TIMES AS VALUE IN THE INNER DICT - // return Tuple.Create(innerRes.Item1, innerRes.Item2, outerRes); - // } - // else - // { - // return innerRes; - // } - // }) - // .Concat(outerDict.Select(x => Tuple.Create(x.Key.Item1, x.Key.Item2, x.Value))).ToImmutableArray(); - //} - /// /// Checks if the scope is valid for conversion to an operation call from the conditional control API. /// It is valid if there is exactly one statement in it and that statement is a call like expression statement. @@ -175,7 +101,8 @@ newCallIdentifier.Expression is ExpressionKind.Identifier id { // We are dissolving the application of arguments here, so the call's type argument // resolutions have to be moved to the 'identifier' sub expression. - var combinedTypeArguments = TryCompineTypeResolution(global.Item, idTypeArguments, callTypeArguments); + var combined = CallGraph.TryCombineTypeResolutions(global.Item, out var combinedTypeArguments, idTypeArguments, callTypeArguments); + QsCompilerError.Verify(combined, "failed to compine type parameter resolution"); var globalCallable = SharedState.Compilation.Namespaces .Where(ns => ns.Name.Equals(global.Item.Namespace)) From f8e68ca3b4c4180befbb39acba291a2e29d4aaaa Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 30 Mar 2020 16:29:30 -0700 Subject: [PATCH 03/25] doc comment --- .../Transformations/CallGraphWalker.cs | 47 +++++++++++++------ 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index c454dc48ec..31f1140ea0 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -32,8 +32,18 @@ public struct CallGraphNode } /// - /// Get the combined type resolutions for a pair of nested resolutions, - /// resolving references in the inner resolutions to the outer resolutions. + /// Given a sequence of type resolutions specifying e.g. subsequent concretizations as part of a nested expression, + /// or concretizations as part of a cycle in the call graph, constructs a dictionary containing the resolution + /// for the type parameters of the specified parent callable. + /// The given resolutions are expected to be ordered starting with the dictionary containing the initial mapping for the + /// type parameters of the specified parent callable (the "innermost resolutions"). This mapping may potentially be to + /// type parameters of other callables, which are then further concretized by subsequent resolutions. + /// Returns true if the combination of the given resolutions is valid, i.e. if there are no conflicting resolutions + /// and type parameters of the parent callables are uniquely resolved to either a concrete type, + /// a type parameter of another callable, or themselves. + /// Throws an ArgumentNullException if the given parent is null. + /// Throws an ArgumentException if the given resolutions imply that type parameters from multiple callables are + /// simultaneously treated as concrete types. /// public static bool TryCombineTypeResolutions (QsQualifiedName parent, out ImmutableDictionary>, ResolvedType> combined, @@ -49,7 +59,8 @@ public static bool TryCombineTypeResolutions foreach (var resolution in resolutions) { // Contains a lookup of all the keys in the combined resolutions whose value needs to be updated - // if a certain non-native type parameter is resolved by the currently processed dictionary. + // if a certain external type parameter is resolved by the currently processed dictionary. + // External type parameters here means type parameters that do not belong to the parent callable. var mayBeReplaced = combinedBuilder .Select(kv => (kv.Key, kv.Value.Resolution as ResolvedTypeKind.TypeParameter)) .Where(entry => entry.Item2 != null && !EqualsParent(entry.Item2.Item.Origin)) @@ -62,27 +73,35 @@ public static bool TryCombineTypeResolutions var resolutionToTypeParam = entry.Value.Resolution as ResolvedTypeKind.TypeParameter; var isResolutionToNative = resolutionToTypeParam != null && EqualsParent(resolutionToTypeParam.Item.Origin); - // resolution of a non-native type parameter that is currently listed as value in the combined type resolution dictionary - foreach (var keyInCombined in mayBeReplaced[entry.Key]) + // resolution of an external type parameter that is currently listed as value in the combined type resolution dictionary + if (mayBeReplaced.Contains(entry.Key)) { - // If one of the values is a type parameter from the parent callable, - // but it isn't mapped to itself then the combined resolution is invalid. - var nontrivialResolutionToNative = isResolutionToNative && keyInCombined.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; - success = success && !nontrivialResolutionToNative; - combinedBuilder[keyInCombined] = entry.Value; + foreach (var keyInCombined in mayBeReplaced[entry.Key]) + { + // If one of the values is a type parameter from the parent callable, + // but it isn't mapped to itself then the combined resolution is invalid. + var nontrivialResolutionToNative = isResolutionToNative && keyInCombined.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; + success = success && !nontrivialResolutionToNative; + combinedBuilder[keyInCombined] = entry.Value; + } } - // resolution of a native type parameter, i.e. a type parameter that belongs to the parent callable - if (EqualsParent(entry.Key.Item1)) + // resolution of a type parameter that belongs to the parent callable and has not already been processed + else if (EqualsParent(entry.Key.Item1)) { // A native type parameter cannot be resolved to another native type parameter, since this would constrain them. var nontrivialResolutionToNative = isResolutionToNative && entry.Key.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; success = success && !nontrivialResolutionToNative; - // TODO: ALSO CHECK IF THERE IS An EXISTING AND CONFLICTING NON-TRIVIAL RESOLUTION FOR A NATIVE ONE + // Check that there is no conflicting resolution already defined. + var conflictingResolutionExists = combinedBuilder.TryGetValue(entry.Key, out var current) && !current.Equals(entry.Value); + success = success && !conflictingResolutionExists; combinedBuilder[entry.Key] = entry.Value; } - else if (!mayBeReplaced.Contains(entry.Key)) + + else { + // It does not make sense to support this case, since there is no valid context in which type parameters + // belonging to multiple callables can/should be treated as concrete types simultaneously. throw new ArgumentException("attempting to define resolution for type parameter that does not belong to parent callable"); } From 1554db0e1eb163a80ca2ca6104733256b298bdfc Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 30 Mar 2020 18:35:33 -0700 Subject: [PATCH 04/25] add a stump for cycle verification --- .../Transformations/CallGraphWalker.cs | 26 ++++++++++++++----- .../Transformations/ClassicallyControlled.cs | 6 ++--- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index 31f1140ea0..00bb194f98 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -31,6 +31,21 @@ public struct CallGraphNode public QsNullable> TypeArgs; } + // TODO: + // This is the method that should be invoked to verify cycles of interest, + // i.e. where each callable in the cycle is type parametrized. + // It should probably generate diagnostics; I'll add the doc comment once its use is fully defined. + private static bool VerifyCycle(CallGraphNode rootNode, params CallGraphEdge[] edges) + { + var parent = rootNode.CallableName; + bool EqualsParent(QsQualifiedName origin) => origin.Namespace.Value == parent.Namespace.Value && origin.Name.Value == parent.Name.Value; + var validResolution = TryCombineTypeResolutions(parent, out var combined, edges.Select(edge => edge.ParamResolutions).ToArray()); + var resolvedToConcrete = combined.Values.All(res => !(res.Resolution is ResolvedTypeKind.TypeParameter tp) || EqualsParent(tp.Item.Origin)); + return validResolution && resolvedToConcrete; + //var isClosedCycle = validCycle && combined.Values.Any(res => res.Resolution is ResolvedTypeKind.TypeParameter tp && EqualsParent(tp.Item.Origin)); + // TODO: check that monomorphization correctly processes closed cycles - meaning add a test... + } + /// /// Given a sequence of type resolutions specifying e.g. subsequent concretizations as part of a nested expression, /// or concretizations as part of a cycle in the call graph, constructs a dictionary containing the resolution @@ -38,12 +53,13 @@ public struct CallGraphNode /// The given resolutions are expected to be ordered starting with the dictionary containing the initial mapping for the /// type parameters of the specified parent callable (the "innermost resolutions"). This mapping may potentially be to /// type parameters of other callables, which are then further concretized by subsequent resolutions. - /// Returns true if the combination of the given resolutions is valid, i.e. if there are no conflicting resolutions - /// and type parameters of the parent callables are uniquely resolved to either a concrete type, - /// a type parameter of another callable, or themselves. + /// Returns the constructed dictionary as out parameter. Returns true if the combination of the given resolutions is valid, + /// i.e. if there are no conflicting resolutions and type parameters of the parent callables are uniquely resolved + /// to either a concrete type, a type parameter of another callable, or themselves. /// Throws an ArgumentNullException if the given parent is null. /// Throws an ArgumentException if the given resolutions imply that type parameters from multiple callables are /// simultaneously treated as concrete types. + /// NOTE: This routine prioritizes the verifications to ensure the correctness of the resolution over performance. /// public static bool TryCombineTypeResolutions (QsQualifiedName parent, out ImmutableDictionary>, ResolvedType> combined, @@ -108,10 +124,8 @@ public static bool TryCombineTypeResolutions } } - QsCompilerError.Verify( - combinedBuilder.Keys.All(key => EqualsParent(key.Item1)), - "for type parameter that does not belong to parent callable"); combined = combinedBuilder.ToImmutable(); + QsCompilerError.Verify(combined.Keys.All(key => EqualsParent(key.Item1)), "for type parameter that does not belong to parent callable"); return success; } diff --git a/src/QsCompiler/Transformations/ClassicallyControlled.cs b/src/QsCompiler/Transformations/ClassicallyControlled.cs index fafe70faad..42f2c2607d 100644 --- a/src/QsCompiler/Transformations/ClassicallyControlled.cs +++ b/src/QsCompiler/Transformations/ClassicallyControlled.cs @@ -102,14 +102,14 @@ newCallIdentifier.Expression is ExpressionKind.Identifier id // We are dissolving the application of arguments here, so the call's type argument // resolutions have to be moved to the 'identifier' sub expression. var combined = CallGraph.TryCombineTypeResolutions(global.Item, out var combinedTypeArguments, idTypeArguments, callTypeArguments); - QsCompilerError.Verify(combined, "failed to compine type parameter resolution"); + QsCompilerError.Verify(combined, "failed to combine type parameter resolution"); var globalCallable = SharedState.Compilation.Namespaces .Where(ns => ns.Name.Equals(global.Item.Namespace)) .Callables() .FirstOrDefault(c => c.FullName.Name.Equals(global.Item.Name)); - QsCompilerError.Verify(globalCallable != null, $"Could not find the global reference {global.Item.Namespace.Value + "." + global.Item.Name.Value}"); + QsCompilerError.Verify(globalCallable != null, $"Could not find the global reference {global.Item.ToString()}"); var callableTypeParameters = globalCallable.Signature.TypeParameters .Select(x => x as QsLocalSymbol.ValidName); @@ -121,7 +121,7 @@ newCallIdentifier.Expression is ExpressionKind.Identifier id id.Item1, QsNullable>.NewValue( callableTypeParameters - .Select(x => combinedTypeArguments.First(y => y.Key.Item2.Equals(x.Item)).Value).ToImmutableArray())), // FIXME: THIS IS WITHOUT ANY CHECK OF THE PARENT... + .Select(x => combinedTypeArguments[Tuple.Create(global.Item, x.Item)]).ToImmutableArray())), TypedExpression.AsTypeArguments(combinedTypeArguments), call.Item1.ResolvedType, call.Item1.InferredInformation, From 398e304d130353505a077654cb4b06862c288dad Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Tue, 31 Mar 2020 06:45:22 -0700 Subject: [PATCH 05/25] setting up some tests --- .../Tests.Compiler/CallGraphTests.fs | 65 +++++++++++++++++++ .../Tests.Compiler/Tests.Compiler.fsproj | 5 +- .../Transformations/CallGraphWalker.cs | 2 +- 3 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 src/QsCompiler/Tests.Compiler/CallGraphTests.fs diff --git a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs new file mode 100644 index 0000000000..9a40b88687 --- /dev/null +++ b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs @@ -0,0 +1,65 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +namespace Microsoft.Quantum.QsCompiler.Testing + +open System.Collections.Generic +open System.Collections.Immutable +open Microsoft.Quantum.QsCompiler.DataTypes +open Microsoft.Quantum.QsCompiler.SyntaxExtensions +open Microsoft.Quantum.QsCompiler.SyntaxTokens +open Microsoft.Quantum.QsCompiler.SyntaxTree +open Microsoft.Quantum.QsCompiler.Transformations +open Xunit +open Xunit.Abstractions + + +type CallGraphTests (output:ITestOutputHelper) = + + let qualifiedName name = + ("NS" |> NonNullable.New, name |> NonNullable.New) |> QsQualifiedName.New + + let typeParameter (id : string) = + let pieces = id.Split(".") + Assert.True(pieces.Length = 2) + let parent = qualifiedName pieces.[0] + let name = pieces.[1] |> NonNullable.New + QsTypeParameter.New (parent, name, Null) + + let resolution (res : (QsTypeParameter * QsTypeKind<_,_,_,_>) list) = + res.ToImmutableDictionary((fun (tp,_) -> tp.Origin, tp.TypeName), snd >> ResolvedType.New) + + let compareDict (d1 : ImmutableDictionary<_,_>) (d2 : ImmutableDictionary<_,_>) = + let keysMismatch = new HashSet<_>(d1.Keys) + keysMismatch.SymmetricExceptWith d2.Keys + keysMismatch.Count = 0 && d1 |> Seq.exists (fun kv -> d2.[kv.Key] <> kv.Value) |> not + + let Foo = qualifiedName "Foo" + let Bar = qualifiedName "Bar" + + let FooA = typeParameter "Foo.A" + let FooB = typeParameter "Foo.B" + let BarA = typeParameter "Bar.A" + let BarC = typeParameter "Bar.C" + let BazA = typeParameter "Baz.A" + + + [] + member this.``Type resolution`` () = + + let res1 = resolution [ + (FooA, BarA |> TypeParameter) + (FooB, Int) + ] + let res2 = resolution [ + (BarA, Int) + ] + let expected = resolution [ + (FooA, Int) + (FooB, Int) + ] + + let mutable combined = ImmutableDictionary.Empty + let success = CallGraph.TryCombineTypeResolutions(Foo, &combined, res1, res2) + Assert.True(success) + Assert.True(compareDict expected combined) diff --git a/src/QsCompiler/Tests.Compiler/Tests.Compiler.fsproj b/src/QsCompiler/Tests.Compiler/Tests.Compiler.fsproj index 879f089a5d..897df1a5ab 100644 --- a/src/QsCompiler/Tests.Compiler/Tests.Compiler.fsproj +++ b/src/QsCompiler/Tests.Compiler/Tests.Compiler.fsproj @@ -143,6 +143,7 @@ + @@ -179,9 +180,7 @@ $(MSBuildThisFileDirectory)..\TestTargets\Simulation\Example\bin\$(Configuration)\netcoreapp3.1\Example.dll $(MSBuildThisFileDirectory)..\TestTargets\Libraries\Library1\bin\$(Configuration)\netcoreapp3.1\Library1.dll - + diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index 00bb194f98..a74ff8d7e4 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -35,7 +35,7 @@ public struct CallGraphNode // This is the method that should be invoked to verify cycles of interest, // i.e. where each callable in the cycle is type parametrized. // It should probably generate diagnostics; I'll add the doc comment once its use is fully defined. - private static bool VerifyCycle(CallGraphNode rootNode, params CallGraphEdge[] edges) + internal static bool VerifyCycle(CallGraphNode rootNode, params CallGraphEdge[] edges) { var parent = rootNode.CallableName; bool EqualsParent(QsQualifiedName origin) => origin.Namespace.Value == parent.Namespace.Value && origin.Name.Value == parent.Name.Value; From 991c0cb8711030ea57013753607c2f0be58d46b9 Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Wed, 1 Apr 2020 08:33:16 -0700 Subject: [PATCH 06/25] found and fixed a bug --- .../Tests.Compiler/CallGraphTests.fs | 73 ++++++++++++++++--- .../Transformations/CallGraphWalker.cs | 5 +- 2 files changed, 68 insertions(+), 10 deletions(-) diff --git a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs index 9a40b88687..a2428d0892 100644 --- a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs +++ b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs @@ -3,6 +3,7 @@ namespace Microsoft.Quantum.QsCompiler.Testing +open System open System.Collections.Generic open System.Collections.Immutable open Microsoft.Quantum.QsCompiler.DataTypes @@ -29,11 +30,6 @@ type CallGraphTests (output:ITestOutputHelper) = let resolution (res : (QsTypeParameter * QsTypeKind<_,_,_,_>) list) = res.ToImmutableDictionary((fun (tp,_) -> tp.Origin, tp.TypeName), snd >> ResolvedType.New) - let compareDict (d1 : ImmutableDictionary<_,_>) (d2 : ImmutableDictionary<_,_>) = - let keysMismatch = new HashSet<_>(d1.Keys) - keysMismatch.SymmetricExceptWith d2.Keys - keysMismatch.Count = 0 && d1 |> Seq.exists (fun kv -> d2.[kv.Key] <> kv.Value) |> not - let Foo = qualifiedName "Foo" let Bar = qualifiedName "Bar" @@ -43,6 +39,24 @@ type CallGraphTests (output:ITestOutputHelper) = let BarC = typeParameter "Bar.C" let BazA = typeParameter "Baz.A" + static member CheckResolution (parent, expected, [] resolutions) = + let compareDict (d1 : ImmutableDictionary<_,_>) (d2 : ImmutableDictionary<_,_>) = + let keysMismatch = new HashSet<_>(d1.Keys) + keysMismatch.SymmetricExceptWith d2.Keys + keysMismatch.Count = 0 && d1 |> Seq.exists (fun kv -> d2.[kv.Key] <> kv.Value) |> not + let mutable combined = ImmutableDictionary.Empty + let success = CallGraph.TryCombineTypeResolutions(parent, &combined, resolutions) + Assert.True(compareDict expected combined) + success + + static member AssertResolution (parent, expected, [] resolutions) = + let success = CallGraphTests.CheckResolution (parent, expected, resolutions) + Assert.True(success) + + static member AssertResolutionFailure (parent, expected, [] resolutions) = + let success = CallGraphTests.CheckResolution (parent, expected, resolutions) + Assert.False(success) + [] member this.``Type resolution`` () = @@ -58,8 +72,49 @@ type CallGraphTests (output:ITestOutputHelper) = (FooA, Int) (FooB, Int) ] + CallGraphTests.AssertResolution(Foo, expected, res1, res2) - let mutable combined = ImmutableDictionary.Empty - let success = CallGraph.TryCombineTypeResolutions(Foo, &combined, res1, res2) - Assert.True(success) - Assert.True(compareDict expected combined) + let res1 = resolution [ + (FooA, BarA |> TypeParameter) + ] + let res2 = resolution [ + (BarA, Int) + (FooB, Int) + ] + let expected = resolution [ + (FooA, Int) + (FooB, Int) + ] + CallGraphTests.AssertResolution(Foo, expected, res1, res2) + + let res1 = resolution [ + (FooA, BarA |> TypeParameter) + (FooB, BarA |> TypeParameter) + ] + let res2 = resolution [ + (BarA, Int) + ] + let expected = resolution [ + (FooA, Int) + (FooB, Int) + ] + CallGraphTests.AssertResolution(Foo, expected, res1, res2) + + let res1 = resolution [ + (FooA, FooA |> TypeParameter) + ] + let expected = resolution [ + (FooA, FooA |> TypeParameter) + ] + CallGraphTests.AssertResolution(Foo, expected, res1) + + let res1 = resolution [ + (FooA, FooA |> TypeParameter) + ] + let res2 = resolution [ + (FooA, Int) + ] + let expected = resolution [ + (FooA, Int) + ] + CallGraphTests.AssertResolution(Foo, expected, res1, res2) diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index a74ff8d7e4..3b3e990913 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -71,6 +71,8 @@ public static bool TryCombineTypeResolutions bool EqualsParent(QsQualifiedName origin) => origin.Namespace.Value == parent.Namespace.Value && origin.Name.Value == parent.Name.Value; static Tuple> AsTypeResolutionKey(QsTypeParameter tp) => Tuple.Create(tp.Origin, tp.TypeName); + static bool ResolutionToTypeParameter(Tuple> typeParam, ResolvedType res) => + res.Resolution is ResolvedTypeKind.TypeParameter tp && tp.Item.Origin.Equals(typeParam.Item1) && tp.Item.TypeName.Equals(typeParam.Item2); foreach (var resolution in resolutions) { @@ -109,7 +111,8 @@ public static bool TryCombineTypeResolutions var nontrivialResolutionToNative = isResolutionToNative && entry.Key.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; success = success && !nontrivialResolutionToNative; // Check that there is no conflicting resolution already defined. - var conflictingResolutionExists = combinedBuilder.TryGetValue(entry.Key, out var current) && !current.Equals(entry.Value); + var conflictingResolutionExists = combinedBuilder.TryGetValue(entry.Key, out var current) + && !current.Equals(entry.Value) && !ResolutionToTypeParameter(entry.Key, current); success = success && !conflictingResolutionExists; combinedBuilder[entry.Key] = entry.Value; } From 4cdfc53f6ded9a22eda757f947385cdab41e339f Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Wed, 1 Apr 2020 09:56:06 -0700 Subject: [PATCH 07/25] found and fixed another edge case --- .../Tests.Compiler/CallGraphTests.fs | 38 +++++++++++++++++-- .../Transformations/CallGraphWalker.cs | 5 ++- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs index a2428d0892..02570c5fec 100644 --- a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs +++ b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs @@ -46,16 +46,16 @@ type CallGraphTests (output:ITestOutputHelper) = keysMismatch.Count = 0 && d1 |> Seq.exists (fun kv -> d2.[kv.Key] <> kv.Value) |> not let mutable combined = ImmutableDictionary.Empty let success = CallGraph.TryCombineTypeResolutions(parent, &combined, resolutions) - Assert.True(compareDict expected combined) + Assert.True(compareDict expected combined, "combined resolutions did not match the expected ones") success static member AssertResolution (parent, expected, [] resolutions) = let success = CallGraphTests.CheckResolution (parent, expected, resolutions) - Assert.True(success) + Assert.True(success, "overall status indicated as not successful") static member AssertResolutionFailure (parent, expected, [] resolutions) = let success = CallGraphTests.CheckResolution (parent, expected, resolutions) - Assert.False(success) + Assert.False(success, "overall status indicated as success") [] @@ -118,3 +118,35 @@ type CallGraphTests (output:ITestOutputHelper) = (FooA, Int) ] CallGraphTests.AssertResolution(Foo, expected, res1, res2) + + let res1 = resolution [ + (FooA, FooB |> TypeParameter) + ] + let expected = resolution [ + (FooA, FooB |> TypeParameter) + ] + CallGraphTests.AssertResolutionFailure(Foo, expected, res1) + + let res1 = resolution [ + (FooA, BarA |> TypeParameter) + ] + let res2 = resolution [ + (BarA, Int) + (FooA, Int) + ] + let expected = resolution [ + (FooA, Int) + ] + CallGraphTests.AssertResolution(Foo, expected, res1, res2) + + let res1 = resolution [ + (FooA, BarA |> TypeParameter) + ] + let res2 = resolution [ + (BarA, Int) + (FooA, Double) + ] + let expected = resolution [ + (FooA, Double) + ] + CallGraphTests.AssertResolutionFailure(Foo, expected, res1, res2) diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index 3b3e990913..8d3a846952 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -86,7 +86,10 @@ static bool ResolutionToTypeParameter(Tuple entry => AsTypeResolutionKey(entry.Item2.Item), entry => entry.Key); - foreach (var entry in resolution) + // We need to ensure that the mappings for extenal type parameters are processed first, + // to cover an edge case that would otherwise be indicated as a conflicting resolution. + var entriesToProcess = resolution.OrderByDescending(entry => mayBeReplaced.Contains(entry.Key)); + foreach (var entry in entriesToProcess) { var resolutionToTypeParam = entry.Value.Resolution as ResolvedTypeKind.TypeParameter; var isResolutionToNative = resolutionToTypeParam != null && EqualsParent(resolutionToTypeParam.Item.Origin); From f64578691feb0be258de0a083fed0de8d1a1e5ba Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Wed, 1 Apr 2020 11:07:45 -0700 Subject: [PATCH 08/25] more tests - the current setup would require checking for each node in the cycle... --- .../Tests.Compiler/CallGraphTests.fs | 68 +++++++++++++++++++ .../Transformations/CallGraphWalker.cs | 38 +++++------ 2 files changed, 86 insertions(+), 20 deletions(-) diff --git a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs index 02570c5fec..45147e6d3d 100644 --- a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs +++ b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs @@ -100,6 +100,21 @@ type CallGraphTests (output:ITestOutputHelper) = ] CallGraphTests.AssertResolution(Foo, expected, res1, res2) + let res1 = resolution [ + (FooA, BarA |> TypeParameter) + ] + let res2 = resolution [ + (FooB, BarA |> TypeParameter) + ] + let res3 = resolution [ + (BarA, Int) + ] + let expected = resolution [ + (FooA, Int) + (FooB, Int) + ] + CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) + let res1 = resolution [ (FooA, FooA |> TypeParameter) ] @@ -150,3 +165,56 @@ type CallGraphTests (output:ITestOutputHelper) = (FooA, Double) ] CallGraphTests.AssertResolutionFailure(Foo, expected, res1, res2) + + let res1 = resolution [ + (FooA, BarA |> TypeParameter) + ] + let res2 = resolution [ + (BarA, FooB |> TypeParameter) + ] + let expected = resolution [ + (FooA, FooB |> TypeParameter) + ] + CallGraphTests.AssertResolutionFailure(Foo, expected, res1, res2) + + let res1 = resolution [ + (FooA, BarA |> TypeParameter) + ] + let res2 = resolution [ + (BarA, BazA |> TypeParameter) + ] + let res3 = resolution [ + (BazA, FooB |> TypeParameter) + ] + let expected = resolution [ + (FooA, FooB |> TypeParameter) + ] + CallGraphTests.AssertResolutionFailure(Foo, expected, res1, res2, res3) + + let res1 = resolution [ + (FooA, BarA |> TypeParameter) + ] + let res2 = resolution [ + (BarA, BazA |> TypeParameter) + ] + let res3 = resolution [ + (BazA, FooA |> TypeParameter) + ] + let expected = resolution [ + (FooA, FooA |> TypeParameter) + ] + CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) + + let res1 = resolution [ + (FooA, BarA |> TypeParameter) + ] + let res2 = resolution [ + (BarA, BazA |> TypeParameter) + ] + let res3 = resolution [ + (BazA, BarC |> TypeParameter) // FIXME: THIS SHOULD NOT BE OK + ] + let expected = resolution [ + (FooA, BarC |> TypeParameter) + ] + CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) \ No newline at end of file diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index 8d3a846952..2334542657 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -77,25 +77,36 @@ static bool ResolutionToTypeParameter(Tuple foreach (var resolution in resolutions) { // Contains a lookup of all the keys in the combined resolutions whose value needs to be updated - // if a certain external type parameter is resolved by the currently processed dictionary. - // External type parameters here means type parameters that do not belong to the parent callable. + // if a certain type parameter is resolved by the currently processed dictionary. var mayBeReplaced = combinedBuilder - .Select(kv => (kv.Key, kv.Value.Resolution as ResolvedTypeKind.TypeParameter)) - .Where(entry => entry.Item2 != null && !EqualsParent(entry.Item2.Item.Origin)) + .Where(kv => kv.Value.Resolution.IsTypeParameter) .ToLookup( - entry => AsTypeResolutionKey(entry.Item2.Item), + kv => AsTypeResolutionKey(((ResolvedTypeKind.TypeParameter)kv.Value.Resolution).Item), entry => entry.Key); // We need to ensure that the mappings for extenal type parameters are processed first, // to cover an edge case that would otherwise be indicated as a conflicting resolution. - var entriesToProcess = resolution.OrderByDescending(entry => mayBeReplaced.Contains(entry.Key)); + var entriesToProcess = resolution.OrderByDescending(entry => !EqualsParent(entry.Key.Item1)); foreach (var entry in entriesToProcess) { var resolutionToTypeParam = entry.Value.Resolution as ResolvedTypeKind.TypeParameter; var isResolutionToNative = resolutionToTypeParam != null && EqualsParent(resolutionToTypeParam.Item.Origin); + // resolution of a type parameter that belongs to the parent callable + if (EqualsParent(entry.Key.Item1)) + { + // A native type parameter cannot be resolved to another native type parameter, since this would constrain them. + var nontrivialResolutionToNative = isResolutionToNative && entry.Key.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; + success = success && !nontrivialResolutionToNative; + // Check that there is no conflicting resolution already defined. + var conflictingResolutionExists = combinedBuilder.TryGetValue(entry.Key, out var current) + && !current.Equals(entry.Value) && !ResolutionToTypeParameter(entry.Key, current); + success = success && !conflictingResolutionExists; + combinedBuilder[entry.Key] = entry.Value; + } + // resolution of an external type parameter that is currently listed as value in the combined type resolution dictionary - if (mayBeReplaced.Contains(entry.Key)) + else if (mayBeReplaced.Contains(entry.Key)) { foreach (var keyInCombined in mayBeReplaced[entry.Key]) { @@ -107,19 +118,6 @@ static bool ResolutionToTypeParameter(Tuple } } - // resolution of a type parameter that belongs to the parent callable and has not already been processed - else if (EqualsParent(entry.Key.Item1)) - { - // A native type parameter cannot be resolved to another native type parameter, since this would constrain them. - var nontrivialResolutionToNative = isResolutionToNative && entry.Key.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; - success = success && !nontrivialResolutionToNative; - // Check that there is no conflicting resolution already defined. - var conflictingResolutionExists = combinedBuilder.TryGetValue(entry.Key, out var current) - && !current.Equals(entry.Value) && !ResolutionToTypeParameter(entry.Key, current); - success = success && !conflictingResolutionExists; - combinedBuilder[entry.Key] = entry.Value; - } - else { // It does not make sense to support this case, since there is no valid context in which type parameters From 8bc17c9c8049bd09a8122355131df9d9a6202fc7 Mon Sep 17 00:00:00 2001 From: bettinaheim <34236215+bettinaheim@users.noreply.github.com> Date: Mon, 6 Apr 2020 08:02:52 -0700 Subject: [PATCH 09/25] Update src/QsCompiler/Transformations/ClassicallyControlled.cs Co-Authored-By: Scott Carda <55811729+ScottCarda-MS@users.noreply.github.com> --- src/QsCompiler/Transformations/ClassicallyControlled.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/QsCompiler/Transformations/ClassicallyControlled.cs b/src/QsCompiler/Transformations/ClassicallyControlled.cs index 42f2c2607d..763b4ff91b 100644 --- a/src/QsCompiler/Transformations/ClassicallyControlled.cs +++ b/src/QsCompiler/Transformations/ClassicallyControlled.cs @@ -109,7 +109,7 @@ newCallIdentifier.Expression is ExpressionKind.Identifier id .Callables() .FirstOrDefault(c => c.FullName.Name.Equals(global.Item.Name)); - QsCompilerError.Verify(globalCallable != null, $"Could not find the global reference {global.Item.ToString()}"); + QsCompilerError.Verify(globalCallable != null, $"Could not find the global reference {global.Item}."); var callableTypeParameters = globalCallable.Signature.TypeParameters .Select(x => x as QsLocalSymbol.ValidName); From 0f85ec3f012513a038cd6119a51c5753dc99a5c0 Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 6 Apr 2020 08:09:18 -0700 Subject: [PATCH 10/25] review comment --- src/QsCompiler/Transformations/CallGraphWalker.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index 2334542657..e7a3ca22a6 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -69,7 +69,6 @@ public static bool TryCombineTypeResolutions var combinedBuilder = ImmutableDictionary.CreateBuilder>, ResolvedType>(); var success = true; - bool EqualsParent(QsQualifiedName origin) => origin.Namespace.Value == parent.Namespace.Value && origin.Name.Value == parent.Name.Value; static Tuple> AsTypeResolutionKey(QsTypeParameter tp) => Tuple.Create(tp.Origin, tp.TypeName); static bool ResolutionToTypeParameter(Tuple> typeParam, ResolvedType res) => res.Resolution is ResolvedTypeKind.TypeParameter tp && tp.Item.Origin.Equals(typeParam.Item1) && tp.Item.TypeName.Equals(typeParam.Item2); @@ -86,14 +85,14 @@ static bool ResolutionToTypeParameter(Tuple // We need to ensure that the mappings for extenal type parameters are processed first, // to cover an edge case that would otherwise be indicated as a conflicting resolution. - var entriesToProcess = resolution.OrderByDescending(entry => !EqualsParent(entry.Key.Item1)); + var entriesToProcess = resolution.OrderByDescending(entry => !entry.Key.Item1.Equals(parent)); foreach (var entry in entriesToProcess) { var resolutionToTypeParam = entry.Value.Resolution as ResolvedTypeKind.TypeParameter; - var isResolutionToNative = resolutionToTypeParam != null && EqualsParent(resolutionToTypeParam.Item.Origin); + var isResolutionToNative = resolutionToTypeParam != null && resolutionToTypeParam.Item.Origin.Equals(parent); // resolution of a type parameter that belongs to the parent callable - if (EqualsParent(entry.Key.Item1)) + if (entry.Key.Item1.Equals(parent)) { // A native type parameter cannot be resolved to another native type parameter, since this would constrain them. var nontrivialResolutionToNative = isResolutionToNative && entry.Key.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; @@ -129,7 +128,7 @@ static bool ResolutionToTypeParameter(Tuple } combined = combinedBuilder.ToImmutable(); - QsCompilerError.Verify(combined.Keys.All(key => EqualsParent(key.Item1)), "for type parameter that does not belong to parent callable"); + QsCompilerError.Verify(combined.Keys.All(key => key.Item1.Equals(parent)), "for type parameter that does not belong to parent callable"); return success; } From dd53a876c1b7f446f23250142c4c5d71659c0e2f Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 6 Apr 2020 08:15:39 -0700 Subject: [PATCH 11/25] doc comment --- src/QsCompiler/Transformations/CallGraphWalker.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index e7a3ca22a6..62dc0c5690 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -47,9 +47,8 @@ internal static bool VerifyCycle(CallGraphNode rootNode, params CallGraphEdge[] } /// - /// Given a sequence of type resolutions specifying e.g. subsequent concretizations as part of a nested expression, - /// or concretizations as part of a cycle in the call graph, constructs a dictionary containing the resolution - /// for the type parameters of the specified parent callable. + /// Combines subsequent concretions as part of a nested expression, or concretions as part of a cycle in the call graph, + /// into a single dictionary containing the resolution for the type parameters of the specified parent callable. /// The given resolutions are expected to be ordered starting with the dictionary containing the initial mapping for the /// type parameters of the specified parent callable (the "innermost resolutions"). This mapping may potentially be to /// type parameters of other callables, which are then further concretized by subsequent resolutions. From 20dc2139d53fcd91c76a38ca33ba5dedfb161227 Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 6 Apr 2020 08:27:42 -0700 Subject: [PATCH 12/25] more review comments --- src/QsCompiler/Transformations/CallGraphWalker.cs | 3 +-- .../Transformations/ClassicallyControlled.cs | 10 +++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index 62dc0c5690..f5d73fc29b 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -38,9 +38,8 @@ public struct CallGraphNode internal static bool VerifyCycle(CallGraphNode rootNode, params CallGraphEdge[] edges) { var parent = rootNode.CallableName; - bool EqualsParent(QsQualifiedName origin) => origin.Namespace.Value == parent.Namespace.Value && origin.Name.Value == parent.Name.Value; var validResolution = TryCombineTypeResolutions(parent, out var combined, edges.Select(edge => edge.ParamResolutions).ToArray()); - var resolvedToConcrete = combined.Values.All(res => !(res.Resolution is ResolvedTypeKind.TypeParameter tp) || EqualsParent(tp.Item.Origin)); + var resolvedToConcrete = combined.Values.All(res => !(res.Resolution is ResolvedTypeKind.TypeParameter tp) || tp.Item.Origin.Equals(parent)); return validResolution && resolvedToConcrete; //var isClosedCycle = validCycle && combined.Values.Any(res => res.Resolution is ResolvedTypeKind.TypeParameter tp && EqualsParent(tp.Item.Origin)); // TODO: check that monomorphization correctly processes closed cycles - meaning add a test... diff --git a/src/QsCompiler/Transformations/ClassicallyControlled.cs b/src/QsCompiler/Transformations/ClassicallyControlled.cs index 42f2c2607d..1abfffedf7 100644 --- a/src/QsCompiler/Transformations/ClassicallyControlled.cs +++ b/src/QsCompiler/Transformations/ClassicallyControlled.cs @@ -91,17 +91,17 @@ public StatementTransformation(SyntaxTreeTransformation par { var newCallIdentifier = call.Item1; var callTypeArguments = expr.Item.TypeParameterResolutions; - var idTypeArguments = call.Item1.TypeParameterResolutions; // This relies on anything having type parameters must be a global callable. - if ( - newCallIdentifier.Expression is ExpressionKind.Identifier id + if (newCallIdentifier.Expression is ExpressionKind.Identifier id && id.Item1 is Identifier.GlobalCallable global - && (callTypeArguments.Any() || idTypeArguments.Any())) + && (callTypeArguments.Any())) { // We are dissolving the application of arguments here, so the call's type argument // resolutions have to be moved to the 'identifier' sub expression. - var combined = CallGraph.TryCombineTypeResolutions(global.Item, out var combinedTypeArguments, idTypeArguments, callTypeArguments); + var combined = CallGraph.TryCombineTypeResolutions(global.Item, + out var combinedTypeArguments, + newCallIdentifier.TypeParameterResolutions, callTypeArguments); QsCompilerError.Verify(combined, "failed to combine type parameter resolution"); var globalCallable = SharedState.Compilation.Namespaces From a64a849e6859f24cab78a7ab109349b9e566fa89 Mon Sep 17 00:00:00 2001 From: bettinaheim <34236215+bettinaheim@users.noreply.github.com> Date: Mon, 6 Apr 2020 13:44:36 -0700 Subject: [PATCH 13/25] Update src/QsCompiler/Transformations/CallGraphWalker.cs Co-Authored-By: Scott Carda <55811729+ScottCarda-MS@users.noreply.github.com> --- src/QsCompiler/Transformations/CallGraphWalker.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index f5d73fc29b..343d89b0ac 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -81,7 +81,7 @@ static bool ResolutionToTypeParameter(Tuple kv => AsTypeResolutionKey(((ResolvedTypeKind.TypeParameter)kv.Value.Resolution).Item), entry => entry.Key); - // We need to ensure that the mappings for extenal type parameters are processed first, + // We need to ensure that the mappings for external type parameters are processed first, // to cover an edge case that would otherwise be indicated as a conflicting resolution. var entriesToProcess = resolution.OrderByDescending(entry => !entry.Key.Item1.Equals(parent)); foreach (var entry in entriesToProcess) From e90082a755211661c19f38e3762573226e62ed34 Mon Sep 17 00:00:00 2001 From: bettinaheim <34236215+bettinaheim@users.noreply.github.com> Date: Mon, 6 Apr 2020 13:45:39 -0700 Subject: [PATCH 14/25] Update src/QsCompiler/Transformations/CallGraphWalker.cs Co-Authored-By: Scott Carda <55811729+ScottCarda-MS@users.noreply.github.com> --- src/QsCompiler/Transformations/CallGraphWalker.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index 343d89b0ac..2bb6674121 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -83,7 +83,7 @@ static bool ResolutionToTypeParameter(Tuple // We need to ensure that the mappings for external type parameters are processed first, // to cover an edge case that would otherwise be indicated as a conflicting resolution. - var entriesToProcess = resolution.OrderByDescending(entry => !entry.Key.Item1.Equals(parent)); + var entriesToProcess = resolution.OrderBy(entry => entry.Key.Item1.Equals(parent)); foreach (var entry in entriesToProcess) { var resolutionToTypeParam = entry.Value.Resolution as ResolvedTypeKind.TypeParameter; From 5454320b86c3f69488794c83e23e8e406998b03b Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 6 Apr 2020 13:54:53 -0700 Subject: [PATCH 15/25] adapting comment --- src/QsCompiler/Tests.Compiler/CallGraphTests.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs index 45147e6d3d..2f0865a0a7 100644 --- a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs +++ b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs @@ -212,7 +212,7 @@ type CallGraphTests (output:ITestOutputHelper) = (BarA, BazA |> TypeParameter) ] let res3 = resolution [ - (BazA, BarC |> TypeParameter) // FIXME: THIS SHOULD NOT BE OK + (BazA, BarC |> TypeParameter) // TODO: for performance reasons it would be nice to detect this case as well and error here ] let expected = resolution [ (FooA, BarC |> TypeParameter) From 0d84170663a6aa0a1e3c782abf15672d14087311 Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 6 Apr 2020 14:02:31 -0700 Subject: [PATCH 16/25] test --- src/QsCompiler/Tests.Compiler/CallGraphTests.fs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs index 2f0865a0a7..a0c427acbe 100644 --- a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs +++ b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs @@ -39,14 +39,14 @@ type CallGraphTests (output:ITestOutputHelper) = let BarC = typeParameter "Bar.C" let BazA = typeParameter "Baz.A" - static member CheckResolution (parent, expected, [] resolutions) = - let compareDict (d1 : ImmutableDictionary<_,_>) (d2 : ImmutableDictionary<_,_>) = - let keysMismatch = new HashSet<_>(d1.Keys) - keysMismatch.SymmetricExceptWith d2.Keys - keysMismatch.Count = 0 && d1 |> Seq.exists (fun kv -> d2.[kv.Key] <> kv.Value) |> not + static member CheckResolution (parent, expected : IDictionary<_,_>, [] resolutions) = + let expectedKeys = ImmutableHashSet.CreateRange(expected.Keys) + let compareWithExpected (d : ImmutableDictionary<_,_>) = + let keysMismatch = expectedKeys.SymmetricExcept d.Keys + keysMismatch.Count = 0 && expected |> Seq.exists (fun kv -> d.[kv.Key] <> kv.Value) |> not let mutable combined = ImmutableDictionary.Empty let success = CallGraph.TryCombineTypeResolutions(parent, &combined, resolutions) - Assert.True(compareDict expected combined, "combined resolutions did not match the expected ones") + Assert.True(compareWithExpected combined, "combined resolutions did not match the expected ones") success static member AssertResolution (parent, expected, [] resolutions) = From 92fee034de7474fd84e2d8a8f43ee649e3e1200e Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 6 Apr 2020 14:38:41 -0700 Subject: [PATCH 17/25] two more tests --- .../Tests.Compiler/CallGraphTests.fs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs index a0c427acbe..a2a1695ff4 100644 --- a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs +++ b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs @@ -205,6 +205,31 @@ type CallGraphTests (output:ITestOutputHelper) = ] CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) + let res1 = resolution [ + (FooA, BarA |> TypeParameter) + ] + let res2 = resolution [ + (BarA, BazA |> TypeParameter) + ] + let expected = resolution [ + (FooA, BazA |> TypeParameter) + ] + CallGraphTests.AssertResolution(Foo, expected, res1, res2) + + let res1 = resolution [ + (FooA, BarA |> TypeParameter) + ] + let res2 = resolution [ + (BarA, BazA |> TypeParameter) + ] + let res3 = resolution [ + (FooA, BazA |> TypeParameter) + ] + let expected = resolution [ + (FooA, BazA |> TypeParameter) + ] + CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) + let res1 = resolution [ (FooA, BarA |> TypeParameter) ] From 8d68acdfa4b412500d7e8494c3d75ea424be64c6 Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 6 Apr 2020 14:40:54 -0700 Subject: [PATCH 18/25] comments --- src/QsCompiler/Tests.Compiler/CallGraphTests.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs index a2a1695ff4..574403d0cb 100644 --- a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs +++ b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs @@ -51,11 +51,11 @@ type CallGraphTests (output:ITestOutputHelper) = static member AssertResolution (parent, expected, [] resolutions) = let success = CallGraphTests.CheckResolution (parent, expected, resolutions) - Assert.True(success, "overall status indicated as not successful") + Assert.True(success, "Combining type resolutions was not successful.") static member AssertResolutionFailure (parent, expected, [] resolutions) = let success = CallGraphTests.CheckResolution (parent, expected, resolutions) - Assert.False(success, "overall status indicated as success") + Assert.False(success, "Combining type resolutions should have failed.") [] From 9f72a996479876314eeb3aec56997150b3d6b90b Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 6 Apr 2020 17:01:24 -0700 Subject: [PATCH 19/25] renaming --- src/QsCompiler/Transformations/CallGraphWalker.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index 2bb6674121..bafedb9345 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -93,8 +93,8 @@ static bool ResolutionToTypeParameter(Tuple if (entry.Key.Item1.Equals(parent)) { // A native type parameter cannot be resolved to another native type parameter, since this would constrain them. - var nontrivialResolutionToNative = isResolutionToNative && entry.Key.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; - success = success && !nontrivialResolutionToNative; + var inconsistentResolutionToNative = isResolutionToNative && entry.Key.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; + success = success && !inconsistentResolutionToNative; // Check that there is no conflicting resolution already defined. var conflictingResolutionExists = combinedBuilder.TryGetValue(entry.Key, out var current) && !current.Equals(entry.Value) && !ResolutionToTypeParameter(entry.Key, current); @@ -109,8 +109,8 @@ static bool ResolutionToTypeParameter(Tuple { // If one of the values is a type parameter from the parent callable, // but it isn't mapped to itself then the combined resolution is invalid. - var nontrivialResolutionToNative = isResolutionToNative && keyInCombined.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; - success = success && !nontrivialResolutionToNative; + var inconsistentResolutionToNative = isResolutionToNative && keyInCombined.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; + success = success && !inconsistentResolutionToNative; combinedBuilder[keyInCombined] = entry.Value; } } From 3c8a874301da088b98dd5ce649d8d69e7202ffd7 Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 6 Apr 2020 19:56:45 -0700 Subject: [PATCH 20/25] splitting tests into individual tests --- .../Tests.Compiler/CallGraphTests.fs | 172 ++++++++++++++---- 1 file changed, 135 insertions(+), 37 deletions(-) diff --git a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs index 574403d0cb..d105a57eef 100644 --- a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs +++ b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs @@ -59,7 +59,8 @@ type CallGraphTests (output:ITestOutputHelper) = [] - member this.``Type resolution`` () = + [] + member this.``Type resolution: resolution to concrete`` () = let res1 = resolution [ (FooA, BarA |> TypeParameter) @@ -74,6 +75,40 @@ type CallGraphTests (output:ITestOutputHelper) = ] CallGraphTests.AssertResolution(Foo, expected, res1, res2) + [] + [] + member this.``Type resolution: resolution to type parameter`` () = + + let res1 = resolution [ + (FooA, BarA |> TypeParameter) + ] + let res2 = resolution [ + (BarA, BazA |> TypeParameter) + ] + let expected = resolution [ + (FooA, BazA |> TypeParameter) + ] + CallGraphTests.AssertResolution(Foo, expected, res1, res2) + + [] + [] + member this.``Type resolution: resolution via identity mapping`` () = + + let res1 = resolution [ + (FooA, FooA |> TypeParameter) + ] + let res2 = resolution [ + (FooA, Int) + ] + let expected = resolution [ + (FooA, Int) + ] + CallGraphTests.AssertResolution(Foo, expected, res1, res2) + + [] + [] + member this.``Type resolution: multi-stage resolution`` () = + let res1 = resolution [ (FooA, BarA |> TypeParameter) ] @@ -87,6 +122,10 @@ type CallGraphTests (output:ITestOutputHelper) = ] CallGraphTests.AssertResolution(Foo, expected, res1, res2) + [] + [] + member this.``Type resolution: multiple resolutions to concrete`` () = + let res1 = resolution [ (FooA, BarA |> TypeParameter) (FooB, BarA |> TypeParameter) @@ -100,6 +139,30 @@ type CallGraphTests (output:ITestOutputHelper) = ] CallGraphTests.AssertResolution(Foo, expected, res1, res2) + [] + [] + member this.``Type resolution: multiple resolutions to type parameter`` () = + + let res1 = resolution [ + (FooA, BarA |> TypeParameter) + (FooB, BarA |> TypeParameter) + ] + let res2 = resolution [ + (BarA, BazA |> TypeParameter) + ] + let res3 = resolution [ + (BazA, Double) + ] + let expected = resolution [ + (FooA, Double) + (FooB, Double) + ] + CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) + + [] + [] + member this.``Type resolution: multi-stage resolution of multiple resolutions to concrete`` () = + let res1 = resolution [ (FooA, BarA |> TypeParameter) ] @@ -115,18 +178,35 @@ type CallGraphTests (output:ITestOutputHelper) = ] CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) + [] + [] + member this.``Type resolution: multi-stage resolution of multiple resolutions to type parameter`` () = + let res1 = resolution [ - (FooA, FooA |> TypeParameter) + (FooA, BarA |> TypeParameter) + ] + let res2 = resolution [ + (BarA, BazA |> TypeParameter) + (FooB, BarA |> TypeParameter) + ] + let res3 = resolution [ + (BazA, Int) ] let expected = resolution [ - (FooA, FooA |> TypeParameter) + (FooA, Int) + (FooB, Int) ] - CallGraphTests.AssertResolution(Foo, expected, res1) + CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) + + [] + [] + member this.``Type resolution: redundant resolution to concrete`` () = let res1 = resolution [ - (FooA, FooA |> TypeParameter) + (FooA, BarA |> TypeParameter) ] let res2 = resolution [ + (BarA, Int) (FooA, Int) ] let expected = resolution [ @@ -134,25 +214,27 @@ type CallGraphTests (output:ITestOutputHelper) = ] CallGraphTests.AssertResolution(Foo, expected, res1, res2) - let res1 = resolution [ - (FooA, FooB |> TypeParameter) - ] - let expected = resolution [ - (FooA, FooB |> TypeParameter) - ] - CallGraphTests.AssertResolutionFailure(Foo, expected, res1) + [] + [] + member this.``Type resolution: redundant resolution to type parameter`` () = let res1 = resolution [ (FooA, BarA |> TypeParameter) ] let res2 = resolution [ - (BarA, Int) - (FooA, Int) + (BarA, BazA |> TypeParameter) + ] + let res3 = resolution [ + (FooA, BazA |> TypeParameter) ] let expected = resolution [ - (FooA, Int) + (FooA, BazA |> TypeParameter) ] - CallGraphTests.AssertResolution(Foo, expected, res1, res2) + CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) + + [] + [] + member this.``Type resolution: conflicting resolution to concrete`` () = let res1 = resolution [ (FooA, BarA |> TypeParameter) @@ -166,30 +248,37 @@ type CallGraphTests (output:ITestOutputHelper) = ] CallGraphTests.AssertResolutionFailure(Foo, expected, res1, res2) + [] + [] + member this.``Type resolution: conflicting resolution to type parameter`` () = + let res1 = resolution [ (FooA, BarA |> TypeParameter) ] let res2 = resolution [ - (BarA, FooB |> TypeParameter) + (BarA, BazA |> TypeParameter) + (FooA, BarC |> TypeParameter) ] let expected = resolution [ - (FooA, FooB |> TypeParameter) + (FooA, BarC |> TypeParameter) ] CallGraphTests.AssertResolutionFailure(Foo, expected, res1, res2) + [] + [] + member this.``Type resolution: direct resolution to native`` () = + let res1 = resolution [ - (FooA, BarA |> TypeParameter) - ] - let res2 = resolution [ - (BarA, BazA |> TypeParameter) - ] - let res3 = resolution [ - (BazA, FooB |> TypeParameter) + (FooA, FooA |> TypeParameter) ] let expected = resolution [ - (FooA, FooB |> TypeParameter) + (FooA, FooA |> TypeParameter) ] - CallGraphTests.AssertResolutionFailure(Foo, expected, res1, res2, res3) + CallGraphTests.AssertResolution(Foo, expected, res1) + + [] + [] + member this.``Type resolution: indirect resolution to native`` () = let res1 = resolution [ (FooA, BarA |> TypeParameter) @@ -205,16 +294,21 @@ type CallGraphTests (output:ITestOutputHelper) = ] CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) + [] + [] + member this.``Type resolution: direct resolution constrains native`` () = + let res1 = resolution [ - (FooA, BarA |> TypeParameter) - ] - let res2 = resolution [ - (BarA, BazA |> TypeParameter) + (FooA, FooB |> TypeParameter) ] let expected = resolution [ - (FooA, BazA |> TypeParameter) + (FooA, FooB |> TypeParameter) ] - CallGraphTests.AssertResolution(Foo, expected, res1, res2) + CallGraphTests.AssertResolutionFailure(Foo, expected, res1) + + [] + [] + member this.``Type resolution: indirect resolution constrains native`` () = let res1 = resolution [ (FooA, BarA |> TypeParameter) @@ -223,12 +317,16 @@ type CallGraphTests (output:ITestOutputHelper) = (BarA, BazA |> TypeParameter) ] let res3 = resolution [ - (FooA, BazA |> TypeParameter) + (BazA, FooB |> TypeParameter) ] let expected = resolution [ - (FooA, BazA |> TypeParameter) + (FooA, FooB |> TypeParameter) ] - CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) + CallGraphTests.AssertResolutionFailure(Foo, expected, res1, res2, res3) + + [] + [] + member this.``Type resolution: inner cycle constrains type parameter`` () = let res1 = resolution [ (FooA, BarA |> TypeParameter) @@ -242,4 +340,4 @@ type CallGraphTests (output:ITestOutputHelper) = let expected = resolution [ (FooA, BarC |> TypeParameter) ] - CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) \ No newline at end of file + CallGraphTests.AssertResolutionFailure(Foo, expected, res1, res2, res3) \ No newline at end of file From ea7e68e1190399f3c47cfb964a445b449ff1f2d6 Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 6 Apr 2020 20:34:58 -0700 Subject: [PATCH 21/25] splitting out inconsistent resolution to native check --- .../Transformations/CallGraphWalker.cs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index bafedb9345..7c2778e6c9 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -71,6 +71,16 @@ public static bool TryCombineTypeResolutions static bool ResolutionToTypeParameter(Tuple> typeParam, ResolvedType res) => res.Resolution is ResolvedTypeKind.TypeParameter tp && tp.Item.Origin.Equals(typeParam.Item1) && tp.Item.TypeName.Equals(typeParam.Item2); + bool InconsistentResolutionToNative(Tuple> key, ResolvedType resolution) + { + var resolutionToTypeParam = resolution.Resolution as ResolvedTypeKind.TypeParameter; + var isResolutionToNative = resolutionToTypeParam != null && resolutionToTypeParam.Item.Origin.Equals(parent); + return isResolutionToNative + // We can omit this check as long as combinedBuilder only ever contains native type parameters: + // && key.Item1.Equals(parent) + && key.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; + } + foreach (var resolution in resolutions) { // Contains a lookup of all the keys in the combined resolutions whose value needs to be updated @@ -86,15 +96,12 @@ static bool ResolutionToTypeParameter(Tuple var entriesToProcess = resolution.OrderBy(entry => entry.Key.Item1.Equals(parent)); foreach (var entry in entriesToProcess) { - var resolutionToTypeParam = entry.Value.Resolution as ResolvedTypeKind.TypeParameter; - var isResolutionToNative = resolutionToTypeParam != null && resolutionToTypeParam.Item.Origin.Equals(parent); // resolution of a type parameter that belongs to the parent callable if (entry.Key.Item1.Equals(parent)) { // A native type parameter cannot be resolved to another native type parameter, since this would constrain them. - var inconsistentResolutionToNative = isResolutionToNative && entry.Key.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; - success = success && !inconsistentResolutionToNative; + success = success && !InconsistentResolutionToNative(entry.Key, entry.Value); // Check that there is no conflicting resolution already defined. var conflictingResolutionExists = combinedBuilder.TryGetValue(entry.Key, out var current) && !current.Equals(entry.Value) && !ResolutionToTypeParameter(entry.Key, current); @@ -109,8 +116,7 @@ static bool ResolutionToTypeParameter(Tuple { // If one of the values is a type parameter from the parent callable, // but it isn't mapped to itself then the combined resolution is invalid. - var inconsistentResolutionToNative = isResolutionToNative && keyInCombined.Item2.Value != resolutionToTypeParam.Item.TypeName.Value; - success = success && !inconsistentResolutionToNative; + success = success && !InconsistentResolutionToNative(keyInCombined, entry.Value); combinedBuilder[keyInCombined] = entry.Value; } } From 1a10823659ef27ccb4adbac23142ab27e7cb9f16 Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 6 Apr 2020 20:40:29 -0700 Subject: [PATCH 22/25] helpful comment --- src/QsCompiler/Transformations/CallGraphWalker.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index 7c2778e6c9..61d08dac4a 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -71,6 +71,8 @@ public static bool TryCombineTypeResolutions static bool ResolutionToTypeParameter(Tuple> typeParam, ResolvedType res) => res.Resolution is ResolvedTypeKind.TypeParameter tp && tp.Item.Origin.Equals(typeParam.Item1) && tp.Item.TypeName.Equals(typeParam.Item2); + // Returns true if the given resolution for the given key constrains the type parameter + // by mapping it to a different type parameter belonging to the same callable. bool InconsistentResolutionToNative(Tuple> key, ResolvedType resolution) { var resolutionToTypeParam = resolution.Resolution as ResolvedTypeKind.TypeParameter; @@ -127,7 +129,6 @@ bool InconsistentResolutionToNative(Tuple> // belonging to multiple callables can/should be treated as concrete types simultaneously. throw new ArgumentException("attempting to define resolution for type parameter that does not belong to parent callable"); } - } } From 83d089da14dc604aa79fad9ded8ad47aaea95f1a Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 6 Apr 2020 20:50:35 -0700 Subject: [PATCH 23/25] separate loops --- .../Transformations/CallGraphWalker.cs | 53 +++++++++---------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/src/QsCompiler/Transformations/CallGraphWalker.cs b/src/QsCompiler/Transformations/CallGraphWalker.cs index 61d08dac4a..a44ab7d809 100644 --- a/src/QsCompiler/Transformations/CallGraphWalker.cs +++ b/src/QsCompiler/Transformations/CallGraphWalker.cs @@ -95,40 +95,35 @@ bool InconsistentResolutionToNative(Tuple> // We need to ensure that the mappings for external type parameters are processed first, // to cover an edge case that would otherwise be indicated as a conflicting resolution. - var entriesToProcess = resolution.OrderBy(entry => entry.Key.Item1.Equals(parent)); - foreach (var entry in entriesToProcess) + foreach (var entry in resolution.Where(entry => mayBeReplaced.Contains(entry.Key))) { - - // resolution of a type parameter that belongs to the parent callable - if (entry.Key.Item1.Equals(parent)) - { - // A native type parameter cannot be resolved to another native type parameter, since this would constrain them. - success = success && !InconsistentResolutionToNative(entry.Key, entry.Value); - // Check that there is no conflicting resolution already defined. - var conflictingResolutionExists = combinedBuilder.TryGetValue(entry.Key, out var current) - && !current.Equals(entry.Value) && !ResolutionToTypeParameter(entry.Key, current); - success = success && !conflictingResolutionExists; - combinedBuilder[entry.Key] = entry.Value; - } - // resolution of an external type parameter that is currently listed as value in the combined type resolution dictionary - else if (mayBeReplaced.Contains(entry.Key)) + foreach (var keyInCombined in mayBeReplaced[entry.Key]) { - foreach (var keyInCombined in mayBeReplaced[entry.Key]) - { - // If one of the values is a type parameter from the parent callable, - // but it isn't mapped to itself then the combined resolution is invalid. - success = success && !InconsistentResolutionToNative(keyInCombined, entry.Value); - combinedBuilder[keyInCombined] = entry.Value; - } + // If one of the values is a type parameter from the parent callable, + // but it isn't mapped to itself then the combined resolution is invalid. + success = success && !InconsistentResolutionToNative(keyInCombined, entry.Value); + combinedBuilder[keyInCombined] = entry.Value; } + } - else - { - // It does not make sense to support this case, since there is no valid context in which type parameters - // belonging to multiple callables can/should be treated as concrete types simultaneously. - throw new ArgumentException("attempting to define resolution for type parameter that does not belong to parent callable"); - } + // resolution of a type parameter that belongs to the parent callable + foreach (var entry in resolution.Where(entry => entry.Key.Item1.Equals(parent))) + { + // A native type parameter cannot be resolved to another native type parameter, since this would constrain them. + success = success && !InconsistentResolutionToNative(entry.Key, entry.Value); + // Check that there is no conflicting resolution already defined. + var conflictingResolutionExists = combinedBuilder.TryGetValue(entry.Key, out var current) + && !current.Equals(entry.Value) && !ResolutionToTypeParameter(entry.Key, current); + success = success && !conflictingResolutionExists; + combinedBuilder[entry.Key] = entry.Value; + } + + if (resolution.Any(entry => !mayBeReplaced.Contains(entry.Key) && !entry.Key.Item1.Equals(parent))) + { + // It does not make sense to support this case, since there is no valid context in which type parameters + // belonging to multiple callables can/should be treated as concrete types simultaneously. + throw new ArgumentException("attempting to define resolution for type parameter that does not belong to parent callable"); } } From 602f4d584724678bfb3ffa44a9c58c0e8ee27537 Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Mon, 6 Apr 2020 20:51:49 -0700 Subject: [PATCH 24/25] disabling test as not yet supported for now --- src/QsCompiler/Tests.Compiler/CallGraphTests.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs index d105a57eef..ba2c71f9d3 100644 --- a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs +++ b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs @@ -178,7 +178,7 @@ type CallGraphTests (output:ITestOutputHelper) = ] CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) - [] + [] [] member this.``Type resolution: multi-stage resolution of multiple resolutions to type parameter`` () = From 01289370a41190cc0229eece08bb0a81622a02e5 Mon Sep 17 00:00:00 2001 From: Bettina Heim Date: Thu, 9 Apr 2020 19:38:42 -0700 Subject: [PATCH 25/25] removing tests for parent independent case --- .../Tests.Compiler/CallGraphTests.fs | 37 ------------------- 1 file changed, 37 deletions(-) diff --git a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs index ba2c71f9d3..b47843038d 100644 --- a/src/QsCompiler/Tests.Compiler/CallGraphTests.fs +++ b/src/QsCompiler/Tests.Compiler/CallGraphTests.fs @@ -178,26 +178,6 @@ type CallGraphTests (output:ITestOutputHelper) = ] CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) - [] - [] - member this.``Type resolution: multi-stage resolution of multiple resolutions to type parameter`` () = - - let res1 = resolution [ - (FooA, BarA |> TypeParameter) - ] - let res2 = resolution [ - (BarA, BazA |> TypeParameter) - (FooB, BarA |> TypeParameter) - ] - let res3 = resolution [ - (BazA, Int) - ] - let expected = resolution [ - (FooA, Int) - (FooB, Int) - ] - CallGraphTests.AssertResolution(Foo, expected, res1, res2, res3) - [] [] member this.``Type resolution: redundant resolution to concrete`` () = @@ -324,20 +304,3 @@ type CallGraphTests (output:ITestOutputHelper) = ] CallGraphTests.AssertResolutionFailure(Foo, expected, res1, res2, res3) - [] - [] - member this.``Type resolution: inner cycle constrains type parameter`` () = - - let res1 = resolution [ - (FooA, BarA |> TypeParameter) - ] - let res2 = resolution [ - (BarA, BazA |> TypeParameter) - ] - let res3 = resolution [ - (BazA, BarC |> TypeParameter) // TODO: for performance reasons it would be nice to detect this case as well and error here - ] - let expected = resolution [ - (FooA, BarC |> TypeParameter) - ] - CallGraphTests.AssertResolutionFailure(Foo, expected, res1, res2, res3) \ No newline at end of file