Adjust to JuliaLang/julia#53219 #547
Conversation
TypedSyntax/src/node.jl
Outdated
| if hasfield(typeof(src), :parent) | ||
| mi = src.parent | ||
| else | ||
| mi = Base.method_instance(f, t) | ||
| end |
There was a problem hiding this comment.
I'm a bit uncertain about using a separate method_instance lookup than the one implicit in code_typed. That sounds like an opportunity for desynchronization.
However, I don't think we have a MI-based code_typed entry point yet so I don't believe there's another alternative.
There was a problem hiding this comment.
However, I don't think we have a MI-based code_typed entry point yet
I think it would be reasonable to add one. I agree, it would be better to look up the method instance first and use that to query the source.
There was a problem hiding this comment.
Implemented the refactoring at 707a181.
|
Nightly still reports ~7 failing tests for me locally with the latest changes, but those seem to match the last CI: https://github.com/JuliaDebug/Cthulhu.jl/actions/runs/7857995905/job/21442465198 (from the last run before #541 started affecting us) |
TypedSyntax/src/node.jl
Outdated
| else | ||
| mi = Base.method_instance(f, t) | ||
| end | ||
| return src, rt, mi |
There was a problem hiding this comment.
Also, it seems to be confusing that methods with the same name return different type objects. I would like to extract MethodInstance lookup logic outside of this function rather than including it here.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #547 +/- ##
======================================
Coverage 0.00% 0.00%
======================================
Files 9 9
Lines 1532 1541 +9
======================================
- Misses 1532 1541 +9 ☔ View full report in Codecov by Sentry. |
aviatesk
force-pushed
the
kf/53219
branch
2 times, most recently
from
fb5c98b
to
18e22f9
Compare
Given that Cthulhu is the only user of TypedSyntax currently, and considering the newer versions of Cthulhu no longer supports older Julia versions like 1.6, I believe there isn't much merit in maintaining support for older Julia versions within TypedSyntax. We anticipate the need for fixes to TypedSyntax's reflection capabilities, like #547, however, continuing to provide support for these features across older versions proves to be a significant hassle. Therefore, I'm inclined to match TypedSyntax.jl's supported Julia version with that of Cthulhu. Additionally, we've found that if Cthulhu's tests fail, TypedSyntax.jl's tests don't get executed, leading to unnoticed regressions in the latter. So I've opted to separate and individually run the tests for both Cthulhu and TypedSyntax.
Given that Cthulhu is the only user of TypedSyntax currently, and considering the newer versions of Cthulhu no longer supports older Julia versions like 1.6, I believe there isn't much merit in maintaining support for older Julia versions within TypedSyntax. We anticipate the need for fixes to TypedSyntax's reflection capabilities, like #547, however, continuing to provide support for these features across older versions proves to be a significant hassle. Therefore, I'm inclined to match TypedSyntax.jl's supported Julia version with that of Cthulhu. Additionally, we've found that if Cthulhu's tests fail, TypedSyntax.jl's tests don't get executed, leading to unnoticed regressions in the latter. So I've opted to separate and individually run the tests for both Cthulhu and TypedSyntax.
These are the changes I was using locally when developing that branch to have basic Cthulhu functionality. It's likely additional changes are required to have Cthulhu fully functional.
I'm not 100% sure about the lookup here, but this at least seems to get the nightly tests back into the state they were in before the upstream CodeInstance change.
|
The nightly failures are all #541. Going to merge. |
These are the changes I was using locally when developing that branch to have basic Cthulhu functionality. It's likely additional changes are required to have Cthulhu fully functional.
type CodeInfo has no field rettypeon Julia's master branch #552