Call Graph from Entry Points #454
Conversation
…in the compilation.
…e are edges between dependent nodes, not that there are no edges.
| // Type arguments need to be resolved for the whole expression to be accurate | ||
| // ToDo: this needs adaption if we want to support type specializations | ||
| var typeArgs = tArgs; | ||
| var typeArgs = QsNullable<ImmutableArray<ResolvedType>>.Null; |
There was a problem hiding this comment.
Hm, I think we have a couple of loose ends here. The additional argument to track which type arguments are associated with a particular call graph node is more generally applicable then just if we had type specializations. Rather than the caller and called type arguments only capturing the type specialization (if we were to support them), they should in fact contain the resolutions for all type parameters. Right now the built graph works perfectly fine for libraries (with some spare handles that are never used); let me see if I can also populate the spare handles when we have an executable (assuming it doesn't contain any call graph cycles that it shouldn't). The handles should either be removed or be used, but I think we can use them.
There was a problem hiding this comment.
Here would be an implementation for populating the type argument handles that are currently unused: #456
The idea would be that the cycle check would be run for each library and the source code separately, and then the full call graph containing the full type information can be build and stored with the compilation (just watch out to keep what is stored with the compilation immutable). As discussed previously, a way to invalidate the call graph if needed would be to just use a guid that is set upon construction.
There was a problem hiding this comment.
I don't understand what this comment is addressing or what the linked implementation does. What is the issue you are trying to address here?
There was a problem hiding this comment.
The todo's in the code; the handles for type arguments are not only applicable if we have type specializations, but make sense to populate in any case if the compilation has entry point(s).
There was a problem hiding this comment.
I'm going to wait to make changes on this topic until it can be worked into the design doc for the call graph. I will create a separate task for this and its implementation will be a separate PR. For now, this PR needs to be merged in so that work can continue on the feature.
You are not providing a hash function for the CallGraphEdge anymore, but I presume the position info will still be in the way of Refers to: src/QsCompiler/Transformations/CallGraph.cs:144 in 4a7c6bb. [](commit_id = 4a7c6bb, deletion_comment = False) |
From the summary comment I'd name this method something like Oh! I think I'm getting it! The goal is to select the subset of type resolutions on the path that actually reach the target. Correct? #Closed Refers to: src/QsCompiler/Transformations/CallGraph.cs:206 in 4a7c6bb. [](commit_id = 4a7c6bb, deletion_comment = False) |
…CombinePathIntoSingleEdge`.
…two separate functions.
…ge and CallGraphNode classes.
bamarsha
left a comment
bamarsha
left a comment
There was a problem hiding this comment.
I haven't had a chance to look at the tests yet, but here are a couple minor comments on the other code.
bamarsha
left a comment
bamarsha
left a comment
There was a problem hiding this comment.
I don't know the context for @bettinaheim's comment about type arguments in the graph, but other than that it looks good.
When Entry Points are present in the compilation, the Call Graph Walker should build a call graph solely off of those entry points and their dependencies, recursively. This will result in smaller, more manageable call graphs that contain info on only the callables that are used.
Other changes:
Copy constructors are added for the node and edge classes.