search

oscar-system / Polymake.jl

Interface to Polymake using CxxWrap
Other
25 stars 17 forks source link

avoid closure cfunctions #467

Closed benlorenz closed 5 months ago

benlorenz commented 5 months ago

Maybe fix some errors on M1 with julia nightly. Still unclear why this only happens with nightly.

fingolfin commented 5 months ago

I can reproduce the issue with Julia master on my M1 mac, and also can verify that this PR fixes using Polymake there

laurentbartholdi commented 5 months ago

If _test_rand_fun is made a global function, then the macro @.***_cfunction can construct a cfunction at compile time, and this works well on M1; however, with the current LLVM, julia cannot construct the cfunction at runtime, which is what Polymake tried to do by passing a function.

It's possible the whole problem could have been resolved by tagging set_rand_source(::Function) with @inline; but in all cases we're in hackland here, with a problem that (hopefully) will disappear when LLVM gets fixed.

On Wed, 24 Jan 2024 at 15:03, Benjamin Lorenz @.***> wrote:

@.**** commented on this pull request.

In test/util.jl https://github.com/oscar-system/Polymake.jl/pull/467#discussion_r1464960620 :

@@ -1,3 +1,5 @@ +_test_rand_fun() = return 42

why?

— Reply to this email directly, view it on GitHub https://github.com/oscar-system/Polymake.jl/pull/467#discussion_r1464960620, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARAQUBUSIVZD2QPVOJNQJLYQEICNAVCNFSM6AAAAABCHZRIFWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNBRGQ2TENJXHE . You are receiving this because you commented.Message ID: @.***>

-- Laurent Bartholdi laurent.bartholdigmailcom Fachrichtung Informatik+Mathematik, Universität des Saarlandes Postfach 151150, 66041 Saarbrücken, Germany Tel. +49 681 3023227, Sekr. +49 681 3023430

benlorenz commented 5 months ago

With this PR both helper functions are global and there should be no closures.

Once I have access to an M1 mac myself I can do some testing if this can be solved with a macro or something like @inline as well but for now this PR seems to work and fixes the issue. Even if this gets fixed in llvm in the near future we will still keep supporting older julia versions without that feature for a while.

My why? was only referring to why = 42 is better than = return 42.