<antocuni>
i.e., before #including hpy.h, you have to define one of HPY_{CPYTHON,HYBRID,UNIVERSAL}_ABI macros
<antocuni>
(our distutils integration does it automatically)
<antocuni>
my problem is that inside our own code, we often have #ifdef HPY_UNIVERSAL_ABI, which worked so far but now it's imprecise
<antocuni>
because most of the times, we want "#ifdef "universal or hybrid" ABI'
<antocuni>
the current solution which I came with is to say that HPY_HYBRID_ABI implies HPY_UNIVERSAL_ABI, (so you can still do #ifdef HPY_UNIVERSAL_ABI)
<antocuni>
and at the same time, I also define HPY_UNIVERSAL_STRICT_ABI for the non-hybrid case
<antocuni>
but I don't really like this solution because it's too confusing
<antocuni>
so basically the question is: what is a good name for "universal or hybrid"?
<antocuni>
I could use HPY_UNIVERSAL_OR_HYBRID_ABI but it looks weird
<antocuni>
_HPY_HAS_CONTEXT_ABI?
<antocuni>
also, I wonder whether we should rename them. E.g.: HPY_ABI_CPYTHON, or even HPyABI_CPYTHON?
<phlebas>
should we just not use `#ifdef` and instead `#if HPY_UNIVERSAL_ABI()`, `#if HPY_HYBRID_ABI()` and `#if HPY_CPYTHON_ABI()` and helper macros to set those to avoid the confusion?
<phlebas>
then you have to do `HPY_USE_HYBRID_ABI()\n#include "hpy.h"`
<antocuni>
I don't understand what you mean
<antocuni>
also, I don't think you can do "#if MACRO()"
<antocuni>
but again, maybe a !HPY_CPYTHON_ABI could be "more semantically correct"?
<fangerer>
> but the github workflow is not running
<fangerer>
I think this is just because you have a merge conflict and CI usually uses the merge commit.
<antocuni>
ah ok
<antocuni>
makes sense
<antocuni>
any opinion on the naming? :)
<antocuni>
fangerer: indeed, I fixed the conflicts and now the checks are running, thank you!
<fangerer>
concerning naming: the fact that hybrid implies universal makes sense to me. I don't find it confusing but I'm probably biased. Instead of `HPY_UNIVERSAL_STRICT_ABI` I would suggest to use `HPY_UNIVERSAL_PURE_ABI` since we already use the term `pure` in conjunction with types and that's related.
<fangerer>
> also, I wonder whether we should rename them. E.g.: HPY_ABI_CPYTHON, or even HPyABI_CPYTHON?
<fangerer>
Yes, that's a good idea. I think `HPY_ABI_*` is more common in C.
<antocuni>
what I find confusing of the current state is that from the outside you specify -DHPY_UNIVERSAL_ABI
<antocuni>
and then "#ifdef HPY_UNIVERSAL_ABI" tests a slightly different thing
<antocuni>
so basically currently #ifdef HPY_UNIVERSAL_STRICT_ABI tests whether you passed -DHPY_UNIVERSAL_ABI
<antocuni>
and #ifdef HPY_UNIVERSAL_ABI tests whether you passed either UNIVERSAL or HYBRID
<antocuni>
we could say that you need to pass -DHPY_UNIVERSAL_PURE_ABI, but then to be consistent we should also use setup.py --hpy-abi=universal-pure
<antocuni>
also from some point of view, "universal-pure" seems conceptually wrong: if it's not pure, it's not "universal" :)
<fangerer>
I see. If we make `HPY_UNIVERSAL_ABI` to be _strict_ then you cannot use legacy API, right? So far, we didn't prevent that.
<fangerer>
I agree, that this would be simpler. Your only concern was the usage of the macros in our code. But we can easily change that.
<fangerer>
I would not like to have `--hpy-abi=universal-pure`, so let's make `HPY_UNIVERSAL_ABI` strict.
<antocuni>
yes, that's what I'm doing in the branch
<antocuni>
when you set HPY_UNIVERSAL_ABI, you will not be able to use legacy features
<antocuni>
ok, we agree on this
<antocuni>
but then we are back to square 1
<antocuni>
what is the best name for "hybrid or universal"?
<antocuni>
maybe it's just HYBRID_OR_UNIVERSAL 😅
<antocuni>
but I am wondering whether there is anything which matches it more closely, like "HAS_CONTEXT" (which however sounds wrong in other ways)
<fangerer>
yes, because we still have a context in all ABIs (because of the context handles)
<antocuni>
exactly
<antocuni>
I think I'll just use !CPYTHON 😅
<fangerer>
I think we cannot use `#ifndef HPY_CPYTHON_ABI` because I have your HPy ABI picture in mind where implementations may add custom ABIs, right?
<antocuni>
ah, that's true
<antocuni>
but also, I think that for now we use it only in our internal code
<fangerer>
what if you just use `#if defined(UNIVERSAL) or defined(HYBRID)`
<antocuni>
yes, that's what I wanted to avoid because it seems too easy to make a mistake, forget a "defined(HYBRID)" somewhere and then have subtle bugs which manifests only with universal or only with hybrid
<fangerer>
> but also, I think that for now we use it only in our internal code
<fangerer>
Right, it wouldn't hurt HPy. Implementations will need to patch the code if they import it from vanilla HPy.
<fangerer>
Okay, then let's use `#ifndef HPY_CPYTHON_ABI` but we need to add some docs about it.
<antocuni>
+1
<antocuni>
thanks for the help
<antocuni>
and sure, I'm also going to write docs
<antocuni>
other question
<antocuni>
do we want to run all tests with both hybrid and universal ABIs?
<antocuni>
somehow I think we should, because we want to ensure that things works
<antocuni>
at the same time, for the vast majority universal and hybrid is the same exact test
<fangerer>
I would rather like to avoid that because it would significantly raise the time we need to run all tests. On the other hand, manually _tagging_ is error prone.
<fangerer>
Since the most time consuming part of the tests is building the generated sources, I would suggest following:
<antocuni>
(I don't think that it's the most time consuming part. I think the most time consuming part is the compilation of the [cpython] case because in that case we compile it together with many .c helpers)
<fangerer>
okay, then I finally need to finish what I started there (having a static lib shipped for hte helpers)
<fangerer>
we could also save the time for building the tests in debug mode since we could just use the universal mode binaries and run in debug mode
<fangerer>
i.e. build once with `--hpy-abi=universal` and run with `universal context` and `debug context `
<antocuni>
yes
<antocuni>
I never though it was worth the extra complexity
<antocuni>
personally I'm using ccache on my machine and it works very well
<antocuni>
like, running non-cpython tests is very fast
<fangerer>
do you think we should enable ccache in the CI ?`
<antocuni>
test_00_basic.py -k 'not cpython' runs in 14s on my machine (~0.3 secs/test)
<antocuni>
ah I might have said nonsense
<antocuni>
I don't think it's ccache which helps because every test generates a different .c file, so ccache cannot cache it
<antocuni>
I think that a statically linked library with the helpers would help much more
<antocuni>
another thing which we can investigate is using precompiled headers (if we don't already -- I don't remember)
<antocuni>
I think they help a lot
<antocuni>
and in most tests, the majority of code seen by the compiler is the header, because the .c files are very slim
<antocuni>
for comparison, test_00_basic.py -k cpython takes 43.38 seconds (0.9 secs/test). So [cpython] tests are 3x slower than [universal] and [debug]
<fangerer>
okay, the static lib for helpers will also fix a different problem; I'll work on that next