Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix show_src_expr on 32 bit Julia #525

Merged
merged 1 commit into from
Jan 7, 2024
Merged

Conversation

Zentrik
Copy link
Collaborator

@Zentrik Zentrik commented Jan 2, 2024

Fixes #524.

_lastidx is a UInt32, so the result of catchup ends up a UInt32. This then gets assigned to position (line 80) which is expected to be an Int. I didn't convert _lastidx to an Int because I thought converting the return value might minimise the chances of an InexactError.

`_lastidx` is a `UInt32`, so the result of `catchup` ends up a `UInt32`. This then gets assigned to `position` (line 80) which is expected to be an `Int`.
I didn't convert `_lastidx` to an `Int` because I thought converting the return value might minimise the chances of an InexactError.
@Zentrik Zentrik changed the title Fix show_src_expr on 32 bit Julia Fix show_src_expr on 32 bit Julia Jan 2, 2024
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6c96853) 0.00% compared to head (13b5375) 0.00%.

Additional details and impacted files
@@          Coverage Diff           @@
##           master    #525   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files           9       9           
  Lines        1551    1551           
======================================
  Misses       1551    1551           

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

@Zentrik
Copy link
Collaborator Author

Zentrik commented Jan 2, 2024

Perhaps we should add some x86 CI.

@ThatcherC
Copy link

Were you able to get this to run on 1.9~x86? I still get the same error when running those locally:

(test-x86) pkg> rm Cthulhu

(test-x86) pkg> precompile

(test-x86) pkg> add https://github.com/Zentrik/Cthulhu.jl#fix-32-bit
    Updating git-repo `https://github.com/Zentrik/Cthulhu.jl`
   Resolving package versions...
    Updating `~/Programs/test-x86/Project.toml`
  [f68482b8] + Cthulhu v2.10.0 `https://github.com/Zentrik/Cthulhu.jl#fix-32-bit`
    Updating `~/Programs/test-x86/Manifest.toml`
  [1520ce14] + AbstractTrees v0.4.4

[...]

  [4ec0a83e] + Unicode
Precompiling project...
  ✗ Cthulhu
  0 dependencies successfully precompiled in 13 seconds. 8 already precompiled.
  1 dependency errored. To see a full report either run `import Pkg; Pkg.precompile()` or load the package

(test-x86) pkg> precompile
Precompiling project...
  ✗ Cthulhu
  0 dependencies successfully precompiled in 13 seconds. 8 already precompiled.
[...]

@Zentrik
Copy link
Collaborator Author

Zentrik commented Jan 5, 2024

Odd I thought it was fixed locally, but I do see the same error though subtly different.

@Zentrik
Copy link
Collaborator Author

Zentrik commented Jan 5, 2024

You need to fetch the version of TypedSyntax from this fork as well by doing add https://github.com/Zentrik/Cthulhu.jl#fix-32-bit:TypedSyntax. I'll merge this in once I figure out if the IntegrationTest failure is legitimate or not.

@ThatcherC
Copy link

Ah nice, that did the trick, thanks! I'm able to use @descend now in my 32-bit Julia. Very cool!

@Zentrik
Copy link
Collaborator Author

Zentrik commented Jan 7, 2024

I see the same CI errors on master and on a new pr, so going to merge this in.

@Zentrik Zentrik merged commit 03c0c85 into JuliaDebug:master Jan 7, 2024
9 of 11 checks passed
@Zentrik Zentrik deleted the fix-32-bit branch January 7, 2024 01:04
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.

Precompilation fails on 32-bit Julia 1.9
3 participants