search

potassco / clingcon

⛓️ Extension of clingo to handle constraints over integers
https://potassco.org/
MIT License
25 stars 4 forks source link

catch.hpp: fix for aarch64 and powerpc #100

Closed barracuda156 closed 1 year ago

barracuda156 commented 1 year ago

aarch64 code from: https://github.com/catchorg/Catch2/commit/a25c1a24af8bffd35727a888a307ff0280cf9387 ppc code from: https://github.com/osmcode/libosmium/blob/b293e96dbe2e3a9afe400768a990e107f8af3f02/test/catch/catch.hpp#L2128 https://gitlab.labos.polytechnique.fr/massot-team/mutationpp/-/blob/fd5188804fadbce5029d3a83c89320d201181a61/thirdparty/catch/include/internal/catch_debugger.h#L26

rkaminsk commented 1 year ago

Could this also be fixed by using an up-to-date catch version like for example clingo is doing?

barracuda156 commented 1 year ago

Could this also be fixed by using an up-to-date catch version like for example clingo is doing?

@rkaminsk aarch64 has been fixed in the commit I referenced to. For PPC fix I have opened a PR there: https://github.com/catchorg/Catch2/pull/2619

rkaminsk commented 1 year ago

The wip branch already includes catch as a submodule. So an updated catch should be part of the next release. For now it is probably best to include your patch when packaging for macos.

barracuda156 commented 1 year ago

Strangely, while it is fixed now in Macports for Big Sur aarch64, it still fails on Monterey aarch64:

OK: https://build.macports.org/builders/ports-11_arm64-builder/builds/81062/steps/install-port/logs/stdio Not OK: https://build.macports.org/builders/ports-12_arm64-builder/builds/79710/steps/install-port/logs/stdio

Apparently upstream fix did not fix aarch64 fully.

rkaminsk commented 1 year ago

Apparently upstream fix did not fix aarch64 fully.

Since the whole CATCH_TRAP thing will never be used in this setting, you could modify your patch to unconditionally use the do nothing case.

rkaminsk commented 1 year ago

I can do a new clingcon release with an updated catch version. This has to wait until catch v3.2.2 is available. Note that the next release can also use a system wide installation of catch using option -DCLINGCON_USE_LOCAL_CATCH=OFF.

barracuda156 commented 1 year ago

@rkaminsk Great, thank you. I will close this then.