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

Make MA work on Pharo 9/10 and on GT #52

Draft
wants to merge 9 commits into
base: pure-z3
Choose a base branch
from

Conversation

janvrany
Copy link
Collaborator

No description provided.

@janvrany janvrany force-pushed the pr/fix-pharo9-and-later branch 2 times, most recently from aa15ed2 to f9e1888 Compare April 21, 2023 14:23
@shingarov shingarov force-pushed the pure-z3 branch 3 times, most recently from 9c3d9ed to 4af3d3c Compare April 25, 2023 22:28
@janvrany janvrany force-pushed the pr/fix-pharo9-and-later branch 3 times, most recently from b1738ab to b7df27e Compare May 17, 2023 21:59
@janvrany janvrany force-pushed the pr/fix-pharo9-and-later branch 7 times, most recently from 7f70a8f to 5d3fef5 Compare June 1, 2023 22:49
@janvrany
Copy link
Collaborator Author

janvrany commented Jun 1, 2023

FYI @shingarov: with latest version of this PR, MA tests pass on P8, P10 and P11 on both, Linux and Windows.
See https://github.com/shingarov/MachineArithmetic/pull/52/checks

I tried P9 but there's something weird and given that current "stable" Pharo is P11 and P12 already on the way, I won't bother.

@shingarov shingarov force-pushed the pure-z3 branch 2 times, most recently from ecf783a to bb0d874 Compare July 27, 2023 05:30
...if `PHARO_DOWNLOAD_VERSION` is set accordingly. It still
defaults to Pharo 8, though.
This commit creates new packages, one for Pharo8 and another for
Pharo9 an newer that contain overrides for `BlockClosure >> value:...`.

This is necessary since primitive numbers have changed from one Pharo
version to another, causing the VM to crash when using wrong primitive
number.

With this commit, MachineArithmetic loads into Pharo 10.
Compiler API apparently changed between P9 and P10, making this test failing on
P10. There's no real need to go through compiler in this test, so this commit changes
the code to not using it. 

As a side-effect, this makes this test working on Smalltalk/X too.
Pharo 10 removed built-in support for #mustBeBoolean deoptimization
(see [1]). This commit loads an external package [2] to bring this
essential functionality back.

[1]: pharo-project/pharo@94cff3a
[2]: https://github.com/pharo/MustBeBooleanMagic
This commit add a copy of MustBeBooleanMagic ([1], commit c5ae7bc)
to MachineArithmetic. This is to allow for further modifications
to make it work also on Pharo 11 and later.

Another option would be to fork it and then maintain yet another fork,
but for such a small (and likely abandoned) package this seems to too
much maintenance overhead and we already have "repository plaque".
…Context:`

...since the latter was removed from P11. Similarly, use (new)
`#rewriteTempsForMustBeBooleanMagicContext:` instead of `#rewriteTempsForContext:`.
This commit adds some `Context` methods introduced after P8 to 
`CardanoTartagliaContext`. With this new methods, Refinements-Tests
all pass on P10.
In P11, the Opal Compiler changes quite a bit so this commit add more methods
to `CardanoTartagliaContext` to make it work also on P11. This required introducing
another helper class `CardanoTartagliaContextVariable` to make Opal compiler happy.

With these changes, `Refinements-Tests` all pass on P11.
@janvrany janvrany force-pushed the pr/fix-pharo9-and-later branch from 5d3fef5 to 6ddcc1d Compare August 25, 2023 12:52
@shingarov
Copy link
Owner

I am looking at the latest commits and I am not even sure what is worse in terms of maintenance overhead: fixing P8 (possibly compiling our own VM to not break libcurl), or fixing P11. If a Smalltalk-80 dialect has broken #ifTrue:, MachineArithmetic-on-that-dialect will wait until it gets fixed.

@janvrany
Copy link
Collaborator Author

Fine with me, but that effectively means we stay on P8 until we fix it elsewhere : - )

@shingarov
Copy link
Owner

One way or another, that #ifTrue: is NOT part of MA, that much is for sure. So perhaps — if we really REALLY wanted P11 — we could load into a "Usable P11", which could be built including a tag inside our own fork of MustBeBooleanMagic. But to drag that insanity into MA, I just don't see how this is any better than the problems with P8.

@janvrany
Copy link
Collaborator Author

If that's one only concern, I can certainly move 16f92eb and 5146f3c to https://github.com/janvrany/pharo-hacks (Usable ... might not be a good place).

@shingarov
Copy link
Owner

The concern is that the sum of P11 hacks is becoming a bigger mess than whatever maintenance overhead would be required to stay on P8.

@shingarov
Copy link
Owner

In other words: P11 is more broken than P8.

@janvrany
Copy link
Collaborator Author

However, for now IMO the major blocker here is removal of GT inspector, see pharo-project/pharo@0fdf1ae

@shingarov
Copy link
Owner

Yeah that's another one. In summary: P11 is a non-starter.

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.

2 participants