Make prefix
more type-stable
#121
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While trying to hunt down type instabilities in TuringLang/DynamicPPL.jl#892 this was one of the things that I first looked at. I thought that the changes in this PR would make a difference. However, simply testing
@inferred AbstractPPL.prefix(vn1, vn2)
with various combinations ofvn1
andvn2
, I can't actually see any difference on Julia 1.11.5, and it didn't help with any upstream type instabilities.It appears to me that the compiler is smart enough to figure out that
getoptic(vn)
returns the second type parameter ofvn
, and thus when it looks at the lineif getoptic(vn) == identity
it can statically determine whether this is true or false, and infer the return type appropriately.I don't know if it's risky to rely on this behaviour, hence why I'm keeping this PR open.