Skip to content

Conversation

@oscardssmith
Copy link
Contributor

@oscardssmith oscardssmith commented Aug 6, 2025

#637 but with updated tests to reflect the changes in parsing of JuliaSyntax1.0 and without the AI slop of #637 #659. @aviatesk I've looked at the parse trees, and the changes look correct, but it would be good to get a review from someone who understands this better.

The good news is I've talked to Em, and it sounds like TypedSyntax should be replaceable by JuliaLowering in the relatively short term, which will be a massive de-jankification.

@fingolfin
Copy link
Contributor

Sorry, my previous post was nonsense, what I should have said (I think?) is that this only update the JuliaSyntax compat in TypedSyntax/Project.toml but not in Project.toml and hence tests fail early.

@oscardssmith
Copy link
Contributor Author

good catch, fixed.

@fingolfin
Copy link
Contributor

fingolfin commented Sep 12, 2025

Thanks. src/CthulhuBase.jlhasusing JuliaSyntax: SyntaxNode, AbstractSyntaxNode, child, childrenbutchild` is gone so at least that has to be changed as well...

@fingolfin
Copy link
Contributor

(Luckily it seems one can just remove child from that line, it isn't actually used as far as I can tell)

@oscardssmith
Copy link
Contributor Author

if you want, I have no objection to you pushing to this PR directly if you have possible fixes.

@fingolfin
Copy link
Contributor

Next error I see is one during precompilation of Cthulhu. (This is with Julia 1.12.0-rc2 BTW). Actually also Revise 3.9.0 complains but that is hopefully unrelated:

Precompiling packages finished.
  19 dependencies successfully precompiled in 40 seconds. 20 already precompiled.
  2 dependencies had output during precompilation:
┌ Revise
│  ┌ Warning: precompile directive
│  │     precompile(Tuple{mbody.sig.parameters[1], Symbol, Bool, Bool, Iterators.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:skip_include,), Tuple{Bool}}}, typeof(methods_by_execution!), Compiled, MI, Module, Expr})
│  │ failed. Please report an issue in Revise (after checking for duplicates) or remove this directive.
│  └ @ Revise ~/.julia/packages/Revise/PSzoa/src/precompile.jl:65
└
┌ Cthulhu
│  ┌ Error: Errorred while running the precompile workload, the package may or may not work but latency will be long
│  │   exeption =
│  │    TaskFailedException
│  │    Stacktrace:
│  │      [1] #wait#582
│  │        @ ./task.jl:363 [inlined]
│  │      [2] wait(t::Task)
│  │        @ Base ./task.jl:360
│  │      [3] macro expansion
│  │        @ ~/.julia/dev/Cthulhu/src/Cthulhu.jl:203 [inlined]
│  │      [4] macro expansion
│  │        @ ~/.julia/packages/PrecompileTools/gn08A/src/workloads.jl:73 [inlined]
│  │      [5] macro expansion
│  │        @ ~/.julia/dev/Cthulhu/src/Cthulhu.jl:199 [inlined]
│  │      [6] macro expansion
│  │        @ ~/.julia/packages/PrecompileTools/gn08A/src/workloads.jl:121 [inlined]
│  │      [7] top-level scope
│  │        @ ~/.julia/dev/Cthulhu/src/Cthulhu.jl:197
│  │      [8] include(mod::Module, _path::String)
│  │        @ Base ./Base.jl:305
│  │      [9] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt128}}, source::Nothing)

But it actually somehow gets past that (?) and then running test Cthulhu, this one shows up:

Callsites: Error During Test at /Users/mhorn/.julia/dev/Cthulhu/test/test_Cthulhu.jl:41
  Got exception outside of a @test
  FieldError: type JuliaSyntax.GreenNode has no field `args`, available fields: `head`, `span`, `children`
  Stacktrace:
    [1] getproperty
      @ ./Base_compiler.jl:54 [inlined]
    [2] truncate_if_defaultargs!(tsn::JuliaSyntax.TreeNode{TypedSyntax.TypedSyntaxData}, mappings::Vector{Vector{Union{JuliaSyntax.TreeNode{TypedSyntax.TypedSyntaxData}, JuliaSyntax.SyntaxNode}}}, meth::Method)
      @ Cthulhu ~/.julia/dev/Cthulhu/src/reflection.jl:350

@fingolfin
Copy link
Contributor

Thanks, but I don't think I have permissions to push here?

Anyway, the child one was easy, but now I am stumped I am afraid, as I don't really know anything about JuliaSyntax and how it changed from 0.4 to 1.0 :-/

JuliaSyntax now emits `op=` nodes,
with functions as `Identifier` nodes,
which are no longer detectable with
`is_operator` for e.g. `+` in `a += b`
@codecov-commenter
Copy link

codecov-commenter commented Sep 18, 2025

Codecov Report

❌ Patch coverage is 0% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (2c33b9a) to head (e06b3c0).
⚠️ Report is 79 commits behind head on master.

Files with missing lines Patch % Lines
src/reflection.jl 0.00% 11 Missing ⚠️
src/CthulhuBase.jl 0.00% 2 Missing ⚠️
src/codeview.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##           master    #661   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files           9      12    +3     
  Lines        1556    1600   +44     
======================================
- Misses       1556    1600   +44     

☔ 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.

@serenity4
Copy link
Collaborator

All tests now pass and AFAIK the behavior remains unchanged compared to JuliaSyntax v0.4.

@aviatesk @oscardssmith @fingolfin, feel free to review the changes. In any case, I think it's in a mergeable state and I plan to merge in the coming days then release a new version for Cthulhu and TypedSyntax so we can stop requiring an outdated JuliaSyntax version.

@oscardssmith
Copy link
Contributor Author

Thanks for finishing this up!

@oscardssmith
Copy link
Contributor Author

Looks like the SnoopCompile issue is real. We possibly should have a patch there...

@serenity4
Copy link
Collaborator

Yes, for some reason the dependencies are way off (JET stuck at 0.9.X), I'll have a look.

@serenity4
Copy link
Collaborator

The issue is that SnoopCompile yet has to support JET v0.10 and at the same time JET v0.9 fails to load on 1.12.0-rc2.

See JuliaDebug/SnoopCompile.jl#422.

@oscardssmith oscardssmith reopened this Sep 22, 2025
@serenity4 serenity4 merged commit e12c683 into JuliaDebug:master Sep 22, 2025
13 of 14 checks passed
@oscardssmith oscardssmith deleted the os/JuliaSyntax@1.0 branch September 23, 2025 02:54
@aviatesk
Copy link
Member

Thank you sooooo much for completing this work!!!

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.

5 participants