JuliaDebug / Cthulhu.jl

The slow descent into madness
MIT License
657 stars 41 forks source link

minor follow up on the tagged code instance change #543

Closed aviatesk closed 8 months ago

aviatesk commented 8 months ago

Requires JuliaLang/julia#53300.

codecov-commenter commented 8 months ago

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (284f4c8) 0.00% compared to head (d4fb503) 0.00%. Report is 2 commits behind head on master.

Files Patch % Lines
src/interpreter.jl 0.00% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #543 +/- ## ====================================== Coverage 0.00% 0.00% ====================================== Files 9 9 Lines 1553 1551 -2 ====================================== + Misses 1553 1551 -2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

aviatesk commented 8 months ago

There appears to be remaining issues related to world ages of CthulhuInterpreter. In the past, code instances were simply inserted into a dictionary maintained by CthulhuInterpreter and the cache existence was simply equivalent to the key existence. However, as they are now maintained as what could be considered "proper" CodeInstances, we're encountering complex problems in test cases that include do blocks."

Keno commented 8 months ago

Do we actually want this, semantically? The fact that Cthulhu's cache is not poisoned by prior results is one of the best features about it. Also, I don't really see any benefit here. You're not likely to want to cache Cthulhu results in precompile files, etc.

vchuravy commented 8 months ago

Do we actually want this, semantically?

Yeah I was thinking about this as well and that's why in #540 I kept using the old design. Each instance of the abstract interpreter currently get's a fresh cache.

aviatesk commented 8 months ago

Yeah, I agree that the current design with the external code cache is a good fit for Cthulhu.