Syntax Tree Walkers #233
Syntax Tree Walkers #233
Conversation
…s based on the code, which will require levels to be defined.
…ess of expressions in `if` statements.
…en state (everything is wrapped in the top-level transformation). Still tests to add, as well as a few TODOs left in the code.
…ing walker. The leveler could be a transform on top and a walker below; not clear how much this would gain.
…tation of specializations, rather than transforming them, avoiding a bunch of copying and allocation.
src/QsCompiler/Core/Dependencies.fs
Outdated
| } | ||
|
|
||
| static member LevelAttribute = { | ||
| Name = "Level" |> NonNullable<string>.New |
There was a problem hiding this comment.
I might suggest something more verbose, especially since it in in the Core namespace, which is automatically opened if imported. I am open to what name exactly.
| Name = "Level" |> NonNullable<string>.New | |
| Name = "TargetCapabilityLevel" |> NonNullable<string>.New |
| Parent : QsQualifiedName | ||
| /// contains all attributes associated with the specialization | ||
| Attributes : ImmutableArray<QsDeclarationAttribute> | ||
| /// contains the minimum target capability level for this specialization's code, not including anything it calls |
There was a problem hiding this comment.
| /// contains the minimum target capability level for this specialization's code, not including anything it calls | |
| /// contains the minimum target capability level required for executing this specialization's code, not including anything it calls |
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.VisualStudio.LanguageServer.Protocol" Version="16.0.2264" /> |
There was a problem hiding this comment.
What is the package needed for? Diagnostics?
| next |> Seq.fold (fun (a : HashSet<SpecializationKey>) k -> if a.Add(k) then WalkDependencyTree k a else a) accum | ||
| | false, _ -> accum | ||
|
|
||
| member this.GetSpecializationLevel(spec) = |
There was a problem hiding this comment.
Shouldn't this function have migrated to CapabilityLeveler.fs?
It shouldn't have anything to do with the callgraph building I think.
| | true, level -> level | ||
| | false, _ -> CapabilityLevel.Unset | ||
|
|
||
| member this.SetSpecializationLevel(spec, level) = |
| /// assigned to a variable or passed as an argument to another callable, | ||
| // which means it could get a functor applied at some later time. | ||
| // We're conservative and add all 4 possible kinds. | ||
| graph.AddDependency(spec, name, QsBody, typeArgs) |
There was a problem hiding this comment.
There is a finer point here unfortunately, that I think I should actually should fix on the expression resolution side: right now these type arguments contain only what is explicitly given as type arguments in the source code, not what the type parameters are that are determined e.g. by the argument of the call. There is also a bit more detail to that that I should think about carefully related to how one would have to set it up to support treating type parameterized callables as first class objects. Let me get back to you on that.
| @@ -0,0 +1,243 @@ | |||
| // Copyright (c) Microsoft Corporation. All rights reserved. | |||
There was a problem hiding this comment.
There seem to be a couple of todo's left here. I might hence be tempted to suggest to split the dependency walker and the capability leveler (and tests) into a separate PR, with the capability leveler possibly going against a feature branch. What do you think?
| | Intrinsic -> CapabilityLevel.Minimal | ||
| | External -> CapabilityLevel.Unset | ||
| | Generated _ -> CapabilityLevel.Unset | ||
| // TODO: For generated specializations, we need to find the appropriate "body" declaration |
There was a problem hiding this comment.
You know, this specifically makes me wonder if we might instead want to track it as inferred information. Let's maybe quickly discuss that option.
ScottCarda-MS
changed the title
bettinaheim
left a comment
bettinaheim
left a comment
There was a problem hiding this comment.
Happy to take this in, since the transformation vs walker distinction makes sense from a perf perspective. I'll make it a weekend challenge to sit down and think about clever ways of unifying the two - or officially admit defeat. ;)
Syntax tree walkers parallel syntax tree transformations, but effectively do
iterinstead offold. In some cases they can be a bit more performant.