Skip to content

Use unoptimized lookup if source is not available#647

Merged
aviatesk merged 5 commits intoJuliaDebug:masterfrom
serenity4:fix-infinite-recursion
Jun 24, 2025
Merged

Use unoptimized lookup if source is not available#647
aviatesk merged 5 commits intoJuliaDebug:masterfrom
serenity4:fix-infinite-recursion

Conversation

@serenity4
Copy link
Collaborator

Fixes #642.

If we hit a CodeInstance that doesn't have inferred code, we now look up its unoptimized (but inferred) source. I think that's the behavior we want, because then we can look into why it wasn't kept (in the case of #642, we can see that lots of constprops were disabled due to LimitedAccuracy, for example). @topolarity feel free to @descend into #642 (comment) and confirm that inspection provides what you would expect to see.

The DCE path for preprocess_ci!(ci::CodeInfo, ...) was apparently unused, I made a few fixes but I believe CallInfos are almost guaranteed to get out of sync which would trigger an assertion error afterwards. Perhaps we should return the IR instead so we can then extract ir.stmts.info? Anyways, I ended up not requiring it and disabling optimization if we have to fall back to an unoptimized lookup, so that seems to remain unused at the moment.

I believe that testing that would require re-enabling the terminal tests, as we'd have to descend into a specific call. I suggest to put that off for now.

@serenity4 serenity4 force-pushed the fix-infinite-recursion branch from 887764b to b00f001 Compare June 7, 2025 11:51
@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2025

Codecov Report

Attention: Patch coverage is 0% with 13 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (2c33b9a) to head (d70e698).
Report is 63 commits behind head on master.

Files with missing lines Patch % Lines
src/CthulhuBase.jl 0.00% 13 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##           master    #647   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files           9      11    +2     
  Lines        1556    1572   +16     
======================================
- Misses       1556    1572   +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vchuravy
Copy link
Member

vchuravy commented Jun 9, 2025

I think we can also just remove preprocess_ci by now, and make Cthulhu a bit more faithful.

@serenity4
Copy link
Collaborator Author

Done in 12e17cc.

What was the motivation to introduce it in the first place? It would seem natural that we'd always want something as faithful as possible for a debugging/introspection tool.

@serenity4
Copy link
Collaborator Author

Ready for final review & merge.

Copy link
Collaborator

@topolarity topolarity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! It's a bit confusing to see un-optimized IR all of a sudden, but that's pretty easy to notice and the [constprop] Disabled by rettype heuristic (limited accuracy) remarks are very useful.

Looks good to me, thanks @serenity4 !

@topolarity
Copy link
Collaborator

(I'm not familiar with Cthulhu internals, so this would still benefit from a review from someone who is)

@serenity4
Copy link
Collaborator Author

@aviatesk a review would be highly appreciated :)

Copy link
Member

@aviatesk aviatesk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing the issue!

@aviatesk
Copy link
Member

TypedSyntax seems to be broken on both nightly and v1.12... We should merge this regardless though.

@aviatesk aviatesk merged commit b4e999c into JuliaDebug:master Jun 24, 2025
3 of 7 checks passed
@serenity4 serenity4 deleted the fix-infinite-recursion branch June 25, 2025 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

StackOverflow upon descending into (dynamic) invoke

5 participants

Comments