From 9ee21d8c6ada1bbf68dfb1219209f3102c190785 Mon Sep 17 00:00:00 2001 From: Sarah Marshall Date: Mon, 16 Mar 2020 11:35:52 -0700 Subject: [PATCH 1/6] Re-enable multiple internal references test --- src/QsCompiler/Tests.Compiler/LinkingTests.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/QsCompiler/Tests.Compiler/LinkingTests.fs b/src/QsCompiler/Tests.Compiler/LinkingTests.fs index e812561507..773088983c 100644 --- a/src/QsCompiler/Tests.Compiler/LinkingTests.fs +++ b/src/QsCompiler/Tests.Compiler/LinkingTests.fs @@ -36,7 +36,7 @@ type LinkingTests (output:ITestOutputHelper) = Offset = DiagnosticTools.AsTuple (Position (0, 0)) Range = QsCompilerDiagnostic.DefaultRange } - + let qualifiedName ns name = { Namespace = NonNullable<_>.New ns Name = NonNullable<_>.New name @@ -356,7 +356,7 @@ type LinkingTests (output:ITestOutputHelper) = [qualifiedName Signatures.InternalRenamingNs "Foo"] [qualifiedName Signatures.InternalRenamingNs "Bar"] - [] + [] member this.``Group internal specializations by source file`` () = let chunks = LinkingTests.ReadAndChunkSourceFile "InternalRenaming.qs" let sourceCompilation = this.BuildContent chunks.[7] From d11d89593350838d73d2b0a2bad97c0162c721a6 Mon Sep 17 00:00:00 2001 From: Sarah Marshall Date: Mon, 16 Mar 2020 13:55:39 -0700 Subject: [PATCH 2/6] Update symbol table --- src/QsCompiler/Core/SymbolResolution.fs | 8 +- src/QsCompiler/Core/SymbolTable.fs | 143 +++++++++++++----------- 2 files changed, 83 insertions(+), 68 deletions(-) diff --git a/src/QsCompiler/Core/SymbolResolution.fs b/src/QsCompiler/Core/SymbolResolution.fs index 80db1f071e..d2e65eff98 100644 --- a/src/QsCompiler/Core/SymbolResolution.fs +++ b/src/QsCompiler/Core/SymbolResolution.fs @@ -133,7 +133,7 @@ module internal ResolutionResult = /// Returns a Found result if there is only one in the sequence. If there is more than one, returns an Ambiguous /// result containing the namespaces of all Found results given by applying nsGetter to each value. Otherwise, /// returns the same value as TryFirstBest. - let internal TryExactlyOne<'T> (nsGetter : 'T -> NonNullable) + let internal TryAtMostOne<'T> (nsGetter : 'T -> NonNullable) (results : seq>) : ResolutionResult<'T> = let found = results |> Seq.filter (function | Found _ -> true | _ -> false) if Seq.length found > 1 @@ -146,9 +146,9 @@ module internal ResolutionResult = |> Option.defaultWith (fun () -> TryFirstBest results) /// Returns a Found result if there is only one in the sequence. If there is more than one, raises an exception. - /// Otherwise, returns the same value as ResolutionResult.tryFirst. - let internal ExactlyOne<'T> : seq> -> ResolutionResult<'T> = - TryExactlyOne (fun _ -> NonNullable<_>.New "") >> function + /// Otherwise, returns the same value as ResolutionResult.TryFirstBest. + let internal AtMostOne<'T> : seq> -> ResolutionResult<'T> = + TryAtMostOne (fun _ -> NonNullable<_>.New "") >> function | Ambiguous _ -> QsCompilerError.Raise "Resolution is ambiguous" Exception () |> raise | result -> result diff --git a/src/QsCompiler/Core/SymbolTable.fs b/src/QsCompiler/Core/SymbolTable.fs index 95b5e727c1..5b25b3f2bf 100644 --- a/src/QsCompiler/Core/SymbolTable.fs +++ b/src/QsCompiler/Core/SymbolTable.fs @@ -240,10 +240,12 @@ type private PartialNamespace private /// Access modifiers are taken into consideration when resolving symbols. Some methods bypass this (e.g., when returning /// a list of all declarations). Individual methods will mention if they adhere to symbol accessibility. and Namespace private - (name, parts : IEnumerable,PartialNamespace>>, - CallablesInReferences : ImmutableDictionary, CallableDeclarationHeader>, - SpecializationsInReferences : ILookup, SpecializationDeclarationHeader * SpecializationImplementation>, - TypesInReferences : ImmutableDictionary, TypeDeclarationHeader>) = + (name, + parts : IEnumerable,PartialNamespace>>, + CallablesInReferences : ILookup, CallableDeclarationHeader>, + SpecializationsInReferences : ILookup, + SpecializationDeclarationHeader * SpecializationImplementation>, + TypesInReferences : ILookup, TypeDeclarationHeader>) = /// dictionary containing a PartialNamespaces for each source file which implements a part of this namespace - /// the key is the source file where each part of the namespace is defined @@ -253,16 +255,20 @@ and Namespace private /// Returns true if the name is available for use in a new declaration. let isNameAvailable name = - let isAvailableWith declarationGetter accessibilityGetter sameAssembly = - match declarationGetter name with - | true, value -> not <| Namespace.IsDeclarationAccessible (sameAssembly, accessibilityGetter value) - | false, _ -> true + let isAvailableWith declarationsGetter accessibilityGetter sameAssembly = + declarationsGetter name + |> Seq.exists (fun name -> Namespace.IsDeclarationAccessible (sameAssembly, accessibilityGetter name)) + |> not - isAvailableWith CallablesInReferences.TryGetValue (fun c -> c.Modifiers.Access) false && - isAvailableWith TypesInReferences.TryGetValue (fun t -> t.Modifiers.Access) false && + let tryToList = function + | true, value -> [value] + | false, _ -> [] + + isAvailableWith (fun name -> CallablesInReferences.[name]) (fun c -> c.Modifiers.Access) false && + isAvailableWith (fun name -> TypesInReferences.[name]) (fun t -> t.Modifiers.Access) false && Parts.Values.All (fun partial -> - isAvailableWith partial.TryGetCallable (fun c -> (snd c).Modifiers.Access) true && - isAvailableWith partial.TryGetType (fun t -> t.Modifiers.Access) true) + isAvailableWith (partial.TryGetCallable >> tryToList) (fun c -> (snd c).Modifiers.Access) true && + isAvailableWith (partial.TryGetType >> tryToList) (fun t -> t.Modifiers.Access) true) /// Returns whether a declaration is accessible from the calling location, given whether the calling location is in /// the same assembly as the declaration, and the declaration's access modifier. @@ -286,24 +292,33 @@ and Namespace private /// constructor taking the name of the namespace as well the name of the files in which (part of) it is declared in as arguments, /// as well as the information about all types and callables declared in referenced assemblies that belong to this namespace - internal new (name, sources, callablesInRefs : IEnumerable<_>, specializationsInRefs : IEnumerable<_>, typesInRefs : IEnumerable<_>) = + internal new (name, sources, callablesInRefs : IEnumerable<_>, specializationsInRefs : IEnumerable<_>, typesInRefs : IEnumerable<_>) = let initialSources = sources |> Seq.distinct |> Seq.map (fun source -> new KeyValuePair<_,_>(source, new PartialNamespace(name, source))) let typesInRefs = typesInRefs.Where (fun (header : TypeDeclarationHeader) -> header.QualifiedName.Namespace = name) let callablesInRefs = callablesInRefs.Where(fun (header : CallableDeclarationHeader) -> header.QualifiedName.Namespace = name) let specializationsInRefs = specializationsInRefs.Where(fun (header : SpecializationDeclarationHeader, _) -> header.Parent.Namespace = name) - // ignore ambiguous/clashing references - let FilterUnique (g : IGrouping<_,_>) = - if g.Count() > 1 then None - else g.Single() |> Some - let typesInRefs = typesInRefs.GroupBy(fun t -> t.QualifiedName.Name) |> Seq.choose FilterUnique - let callablesInRefs = callablesInRefs.GroupBy(fun c -> c.QualifiedName.Name) |> Seq.choose FilterUnique - - let types = typesInRefs.ToImmutableDictionary(fun t -> t.QualifiedName.Name) - let callables = callablesInRefs.ToImmutableDictionary(fun c -> c.QualifiedName.Name) - let specializations = specializationsInRefs.Where(fun (s, _) -> callables.ContainsKey s.Parent.Name).ToLookup(fun (s, _) -> s.Parent.Name) - new Namespace(name, initialSources, callables, specializations, types) - + let discardConflicts getAccess (_, nameGroup) = + // Only one externally accessible declaration with the same name is allowed. + let isAccessible header = Namespace.IsDeclarationAccessible (false, getAccess header) + if nameGroup |> Seq.filter isAccessible |> Seq.length > 1 + then nameGroup |> Seq.filter (not << isAccessible) + else nameGroup + + let createLookup getName getAccess headers = + headers + |> Seq.groupBy getName + |> Seq.map (discardConflicts getAccess) + |> Seq.concat + |> fun headers -> headers.ToLookup (Func<_, _> getName) + + let types = typesInRefs |> createLookup (fun t -> t.QualifiedName.Name) (fun t -> t.Modifiers.Access) + let callables = callablesInRefs |> createLookup (fun c -> c.QualifiedName.Name) (fun c -> c.Modifiers.Access) + let specializations = + specializationsInRefs + .Where(fun (s, _) -> callables.[s.Parent.Name].Any()) + .ToLookup(fun (s, _) -> s.Parent.Name) + Namespace (name, initialSources, callables, specializations, types) /// returns true if the namespace currently contains no source files or referenced content member this.IsEmpty = @@ -370,12 +385,16 @@ and Namespace private resolution.Resolved.ValueOrApply missingResolutionException |> fst |> Some | _ -> None | false, _ -> - match TypesInReferences.TryGetValue attName with - | true, qsType -> + let referenceType = + TypesInReferences.[attName] + |> Seq.filter (fun qsType -> qsType.SourceFile = source) + |> Seq.tryExactlyOne + match referenceType with + | Some qsType -> if Seq.exists marksAttribute qsType.Attributes then Some qsType.Type else None - | false, _ -> ArgumentException "Given source file is not part of the namespace" |> raise + | None -> ArgumentException "given source file is not part of the namespace" |> raise /// Returns the type with the given name defined in the given source file within this namespace. /// Note that files contained in referenced assemblies are *not* considered to be source files for the namespace! @@ -462,15 +481,12 @@ and Namespace private defaultArg checkDeprecation (fun qual -> String.IsNullOrWhiteSpace qual || qual = BuiltIn.Deprecated.Namespace.Value) - let fromReferences = - match TypesInReferences.TryGetValue tName with - | true, qsType -> - if Namespace.IsDeclarationAccessible (false, qsType.Modifiers.Access) - then Found (qsType.SourceFile, - SymbolResolution.TryFindRedirect qsType.Attributes, - qsType.Modifiers.Access) - else Inaccessible - | false, _ -> NotFound + let resolveReferenceType (typeHeader : TypeDeclarationHeader) = + if Namespace.IsDeclarationAccessible (false, typeHeader.Modifiers.Access) + then Found (typeHeader.SourceFile, + SymbolResolution.TryFindRedirect typeHeader.Attributes, + typeHeader.Modifiers.Access) + else Inaccessible let findInPartial (partial : PartialNamespace) = match partial.TryGetType tName with @@ -482,8 +498,8 @@ and Namespace private else Inaccessible | false, _ -> NotFound - seq { yield fromReferences - yield Seq.map findInPartial Parts.Values |> ResolutionResult.ExactlyOne } + seq { yield Seq.map resolveReferenceType TypesInReferences.[tName] |> ResolutionResult.AtMostOne + yield Seq.map findInPartial Parts.Values |> ResolutionResult.AtMostOne } |> ResolutionResult.TryFirstBest /// Returns a resolution result for the callable with the given name containing the name of the source file or @@ -503,13 +519,10 @@ and Namespace private defaultArg checkDeprecation (fun qual -> String.IsNullOrWhiteSpace qual || qual = BuiltIn.Deprecated.Namespace.Value) - let fromReferences = - match CallablesInReferences.TryGetValue cName with - | true, callable -> - if Namespace.IsDeclarationAccessible (false, callable.Modifiers.Access) - then Found (callable.SourceFile, SymbolResolution.TryFindRedirect callable.Attributes) - else Inaccessible - | false, _ -> NotFound + let resolveReferenceCallable (callable : CallableDeclarationHeader) = + if Namespace.IsDeclarationAccessible (false, callable.Modifiers.Access) + then Found (callable.SourceFile, SymbolResolution.TryFindRedirect callable.Attributes) + else Inaccessible let findInPartial (partial : PartialNamespace) = match partial.TryGetCallable cName with @@ -520,8 +533,8 @@ and Namespace private else Inaccessible | false, _ -> NotFound - seq { yield fromReferences - yield Seq.map findInPartial Parts.Values |> ResolutionResult.ExactlyOne } + seq { yield Seq.map resolveReferenceCallable CallablesInReferences.[cName] |> ResolutionResult.AtMostOne + yield Seq.map findInPartial Parts.Values |> ResolutionResult.AtMostOne } |> ResolutionResult.TryFirstBest /// Sets the resolution for the type with the given name in the given source file to the given type, @@ -668,7 +681,7 @@ and Namespace private let unitReturn = cDecl.Defined.ReturnType |> unitOrInvalid (fun (t : QsType) -> t.Type) unitReturn, cDecl.Defined.TypeParameters.Length | false, _ -> - let cDecl = CallablesInReferences.[cName] + let cDecl = CallablesInReferences.[cName] |> Seq.filter (fun c -> c.SourceFile = source) |> Seq.exactlyOne let unitReturn = cDecl.Signature.ReturnType |> unitOrInvalid (fun (t : ResolvedType) -> t.Resolution) unitReturn, cDecl.Signature.TypeParameters.Length @@ -780,7 +793,7 @@ and NamespaceManager resolver ns |> ResolutionResult.Map (fun value -> (ns.Name, value)) let currentNs, importedNs = OpenNamespaces (nsName, source) seq { yield resolveWithNsName currentNs - yield Seq.map resolveWithNsName importedNs |> ResolutionResult.TryExactlyOne fst } + yield Seq.map resolveWithNsName importedNs |> ResolutionResult.TryAtMostOne fst } |> ResolutionResult.TryFirstBest /// Given a qualifier for a symbol name, returns the corresponding namespace as Some @@ -1260,8 +1273,9 @@ and NamespaceManager member this.ImportedCallables () = // TODO: this needs to be adapted if we support external specializations syncRoot.EnterReadLock() - try let imported = Namespaces.Values |> Seq.collect (fun ns -> ns.CallablesInReferencedAssemblies.Values) - imported.ToImmutableArray() + try Namespaces.Values + |> Seq.collect (fun ns -> ns.CallablesInReferencedAssemblies.SelectMany (fun g -> g.AsEnumerable ())) + |> fun callables -> callables.ToImmutableArray () finally syncRoot.ExitReadLock() /// Returns the declaration headers for all callables defined in source files, regardless of accessibility. @@ -1307,8 +1321,9 @@ and NamespaceManager /// of accessibility. member this.ImportedTypes() = syncRoot.EnterReadLock() - try let imported = Namespaces.Values |> Seq.collect (fun ns -> ns.TypesInReferencedAssemblies.Values) - imported.ToImmutableArray() + try Namespaces.Values + |> Seq.collect (fun ns -> ns.TypesInReferencedAssemblies.SelectMany (fun g -> g.AsEnumerable ())) + |> fun types -> types.ToImmutableArray () finally syncRoot.ExitReadLock() /// Returns the declaration headers for all types defined in source files, regardless of accessibility. @@ -1451,12 +1466,12 @@ and NamespaceManager } let findInReferences (ns : Namespace) = - match ns.CallablesInReferencedAssemblies.TryGetValue callableName.Name with - | true, callable -> + ns.CallablesInReferencedAssemblies.[callableName.Name] + |> Seq.map (fun callable -> if Namespace.IsDeclarationAccessible (false, callable.Modifiers.Access) then Found callable - else Inaccessible - | false, _ -> NotFound + else Inaccessible) + |> ResolutionResult.AtMostOne let findInSources (ns : Namespace) = function | Some source -> @@ -1536,12 +1551,12 @@ and NamespaceManager } let findInReferences (ns : Namespace) = - match ns.TypesInReferencedAssemblies.TryGetValue typeName.Name with - | true, qsType -> - if Namespace.IsDeclarationAccessible (false, qsType.Modifiers.Access) - then Found qsType - else Inaccessible - | false, _ -> NotFound + ns.TypesInReferencedAssemblies.[typeName.Name] + |> Seq.map (fun typeHeader -> + if Namespace.IsDeclarationAccessible (false, typeHeader.Modifiers.Access) + then Found typeHeader + else Inaccessible) + |> ResolutionResult.AtMostOne let findInSources (ns : Namespace) = function | Some source -> From 7e3a6564a324a0c7af4df541844c5d6395bee98c Mon Sep 17 00:00:00 2001 From: Sarah Marshall Date: Mon, 16 Mar 2020 14:08:25 -0700 Subject: [PATCH 3/6] Group specializations by source file --- .../CompilationManager/CompilationUnit.cs | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/QsCompiler/CompilationManager/CompilationUnit.cs b/src/QsCompiler/CompilationManager/CompilationUnit.cs index ae2ce5937d..8e9f77a820 100644 --- a/src/QsCompiler/CompilationManager/CompilationUnit.cs +++ b/src/QsCompiler/CompilationManager/CompilationUnit.cs @@ -483,25 +483,29 @@ private QsCallable GetImportedCallable(CallableDeclarationHeader header) { // TODO: this needs to be adapted if we want to support external specializations if (header == null) throw new ArgumentNullException(nameof(header)); - var importedSpecs = this.GlobalSymbols.ImportedSpecializations(header.QualifiedName); if (Namespace.IsDeclarationAccessible(false, header.Modifiers.Access)) { - var definedSpecs = this.GlobalSymbols.DefinedSpecializations(header.QualifiedName); + var definedSpecs = GlobalSymbols.DefinedSpecializations(header.QualifiedName); QsCompilerError.Verify(definedSpecs.Length == 0, "external specializations are currently not supported"); } - var specializations = importedSpecs.Select(imported => - { - var (specHeader, implementation) = imported; - var specSignature = specHeader.Kind.IsQsControlled || specHeader.Kind.IsQsControlledAdjoint - ? SyntaxGenerator.BuildControlled(header.Signature) - : header.Signature; - return new QsSpecialization(specHeader.Kind, header.QualifiedName, specHeader.Attributes, - specHeader.SourceFile, specHeader.Location, specHeader.TypeArguments, specSignature, - implementation, specHeader.Documentation, QsComments.Empty); - }) - .ToImmutableArray(); + var specializations = + GlobalSymbols + .ImportedSpecializations(header.QualifiedName) + .Where(specialization => specialization.Item1.SourceFile.Equals(header.SourceFile)) + .Select(specialization => + { + var (specHeader, implementation) = specialization; + var specSignature = specHeader.Kind.IsQsControlled || specHeader.Kind.IsQsControlledAdjoint + ? SyntaxGenerator.BuildControlled(header.Signature) + : header.Signature; + return new QsSpecialization( + specHeader.Kind, header.QualifiedName, specHeader.Attributes, specHeader.SourceFile, + specHeader.Location, specHeader.TypeArguments, specSignature, implementation, + specHeader.Documentation, QsComments.Empty); + }) + .ToImmutableArray(); return new QsCallable( header.Kind, header.QualifiedName, From 4a15dcd73b9e80152c7344b3f576148e20725028 Mon Sep 17 00:00:00 2001 From: Sarah Marshall Date: Mon, 16 Mar 2020 15:05:18 -0700 Subject: [PATCH 4/6] Fix alignment --- src/QsCompiler/Core/SymbolResolution.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/QsCompiler/Core/SymbolResolution.fs b/src/QsCompiler/Core/SymbolResolution.fs index d2e65eff98..0d3a0620cd 100644 --- a/src/QsCompiler/Core/SymbolResolution.fs +++ b/src/QsCompiler/Core/SymbolResolution.fs @@ -133,8 +133,8 @@ module internal ResolutionResult = /// Returns a Found result if there is only one in the sequence. If there is more than one, returns an Ambiguous /// result containing the namespaces of all Found results given by applying nsGetter to each value. Otherwise, /// returns the same value as TryFirstBest. - let internal TryAtMostOne<'T> (nsGetter : 'T -> NonNullable) - (results : seq>) : ResolutionResult<'T> = + let internal TryAtMostOne<'T> (nsGetter : 'T -> NonNullable) (results : seq>) + : ResolutionResult<'T> = let found = results |> Seq.filter (function | Found _ -> true | _ -> false) if Seq.length found > 1 then found From ec84fa13a666deb5ceef72eac913a5f78e27b066 Mon Sep 17 00:00:00 2001 From: Sarah Marshall Date: Mon, 16 Mar 2020 17:19:17 -0700 Subject: [PATCH 5/6] Only group specializations by source if callable is internal --- src/QsCompiler/CompilationManager/CompilationUnit.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/QsCompiler/CompilationManager/CompilationUnit.cs b/src/QsCompiler/CompilationManager/CompilationUnit.cs index 8e9f77a820..3cb0980f27 100644 --- a/src/QsCompiler/CompilationManager/CompilationUnit.cs +++ b/src/QsCompiler/CompilationManager/CompilationUnit.cs @@ -493,7 +493,11 @@ private QsCallable GetImportedCallable(CallableDeclarationHeader header) var specializations = GlobalSymbols .ImportedSpecializations(header.QualifiedName) - .Where(specialization => specialization.Item1.SourceFile.Equals(header.SourceFile)) + .Where(specialization => + // Either the callable is externally accessible, or all of its specializations must be defined in + // the same reference as the callable. + Namespace.IsDeclarationAccessible(false, header.Modifiers.Access) || + specialization.Item1.SourceFile.Equals(header.SourceFile)) .Select(specialization => { var (specHeader, implementation) = specialization; From 75e464811b164aac6896866ce56fc2c864fdfe1e Mon Sep 17 00:00:00 2001 From: Sarah Marshall Date: Mon, 16 Mar 2020 17:47:59 -0700 Subject: [PATCH 6/6] Re-add explicit this --- src/QsCompiler/CompilationManager/CompilationUnit.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/QsCompiler/CompilationManager/CompilationUnit.cs b/src/QsCompiler/CompilationManager/CompilationUnit.cs index 3cb0980f27..9c824bb4c6 100644 --- a/src/QsCompiler/CompilationManager/CompilationUnit.cs +++ b/src/QsCompiler/CompilationManager/CompilationUnit.cs @@ -485,13 +485,13 @@ private QsCallable GetImportedCallable(CallableDeclarationHeader header) if (header == null) throw new ArgumentNullException(nameof(header)); if (Namespace.IsDeclarationAccessible(false, header.Modifiers.Access)) { - var definedSpecs = GlobalSymbols.DefinedSpecializations(header.QualifiedName); + var definedSpecs = this.GlobalSymbols.DefinedSpecializations(header.QualifiedName); QsCompilerError.Verify(definedSpecs.Length == 0, "external specializations are currently not supported"); } var specializations = - GlobalSymbols + this.GlobalSymbols .ImportedSpecializations(header.QualifiedName) .Where(specialization => // Either the callable is externally accessible, or all of its specializations must be defined in