Add value and constructor path #1030
Conversation
Signed-off-by: Paul-Elliot <[email protected]>
Signed-off-by: Paul-Elliot <[email protected]>
Signed-off-by: Paul-Elliot <[email protected]>
panglesd
added
the
no changelog
| let () = | ||
| (* Until those are used *) | ||
| ignore value_path; | ||
| ignore constructor_path |
There was a problem hiding this comment.
Would it break things to modify source_info_infos like in #976, or would it allow to check more things in the tests?
There was a problem hiding this comment.
I'm not sure about the question! Initially, #976 was a single PR, but I turned it into three PRs:
- one for the path resolution (this one)
- one for the refactoring Refactor typedtree traverse to use compiler traverse #1031
- Collect occurrences information #976 which will be rebased on top of master when the two other one have been merged.
So no, it won't break things to have source_infos as in #976, on the contrary that is the goal!
There was a problem hiding this comment.
What I meant is it could be done in this PR to avoid ignoring the new functions. And then we end up having a source_info_infos different, but could we ignore the new values when processing the source_infos later down the code or would it break something, in which case we should not modify the source_infos yet?
There was a problem hiding this comment.
Ok I think I understand your point: without test, it's harder to review and be sure the PR is correct...
The problem is that having source_info as in #976 is equivalent to merging the three PRs in one, which is back to the initial situation!
| let () = | ||
| (* Until those are used *) | ||
| ignore value_path; | ||
| ignore constructor_path |
There was a problem hiding this comment.
Same question here, would it break things to update the source_info in unit like in #976?
panglesd
force-pushed
the
value-and-constructor-path
branch
from
448922b to
949f955
Compare
Signed-off-by: Paul-Elliot <[email protected]>
Signed-off-by: Paul-Elliot <[email protected]>
|
I'm not very convinced with all the datatype stuff; I think we've not got the best set of types here. Having said that, we're already in this situation and this PR won't make it more tricky to rip it out, and there are other PRs that depend upon this. Hence, let's figure this out later. |
The ability to resolve those path will be useful in many render source code new features. In particular, this is a prerequisite to #976.