FFY00_ has quit [Read error: Connection reset by peer]
FFY00 has joined #hpy
<antocuni> does anybody know what is the equivalent of __attribute__((error)) on clang and MSVCC?
<antocuni> uhm, I'm confused
<antocuni> in my branch, I have decorated HPy_AsPyObject and HPy_FromPyObject with __attribute__((error)) when you are compiling in universal mode
<antocuni> but on github, mac tests fails, e.g. this: https://github.com/hpyproject/hpy/actions/runs/3289004023/jobs/5420021023
<antocuni> if you look at the error message, it says "atal error: unknown attribute 'error' ignored [-Wunknown-attributes]"
<antocuni> but the clang docs says that __attribute__((error)) is supported: https://clang.llvm.org/docs/AttributeReference.html#error-warning
<antocuni> does anybody have a clue and/or have a mac to try?
<mattip> give me a few minutes to get set up
<antocuni> thanks
<antocuni> I assumed that somewhere on the internet there were a rosetta stone of how to use the various __attribute__ in various compilers but it seems it doesn't exist
<mattip> from this failure
<mattip> it looks like the precompiled code is something like
<mattip> int f(int a) __attribute__((error("Cannot use legacy functions when targeting the HPy Universal ABI")))
<mattip> but that can't be right
<antocuni> why not? Isn't it the correct syntax for function attributes?
<antocuni> it works on gcc
<mattip> error: attributes should be specified before the declarator in a function definition
<antocuni> ah
<antocuni> thanks
<antocuni> the other problem is how to do it with MSVC
<antocuni> because I don't think it has an equivalent ot "error"
<antocuni> I suppose that on windows we can mark it as "deprecated" for now
<mattip> this works on gcc: if you comment out the call to f() in main() it compiles, if you leave it as-is it errors
<mattip> now let me try clang on macos
<antocuni> mattip: I tried and on gcc it works even if you put the __attribute__ in the front
<mattip> on clang it fails
<mattip> with the error in the CI
<mattip> it fails on clang on linux as well
<antocuni> mattip: see my answer on the gist
<mattip> no, still "warning: unknown attribute 'error' ignored [-Wunknown-attributes]"
<antocuni> :(
<antocuni> it works with clang-7 on linux
<mattip> not with version 10
<antocuni> sigh
<antocuni> but the docs say it should work: https://clang.llvm.org/docs/AttributeReference.html#error-warning
<antocuni> does it work if you use "warning" instead of error?
<mattip> no, also unkown
<antocuni> that's very weird
<mattip> the goal is to get a compile error only if the function is used?
<antocuni> yes
<antocuni> well, it's just a plus
<antocuni> I'm fine if we get the error only on gcc on linux for now
<antocuni> the "proper" solution (which is not implemented by this PR) would be have two different contexts
<antocuni> one for pure universal and one for hybrid
<antocuni> and if you try to call HPy_AsPyObject with a pure universal context, you get a clean runtime abort()
<antocuni> and similar if you try to use e.g. HPyModule_Spec.legacy_methods
<antocuni> so from this POV a compile time error is handy, but it's not fundamental
<mattip> well, you could do
<mattip> #ifdef _HPY_LEGACY
<mattip> HPyAPI_FUNC cpy_PyObject *HPy_AsPyObject(HPyContext *ctx, HPy h);
<mattip> #else
<mattip> #endif
<antocuni> yes
<mattip> and then poof the interfaces are gone
<antocuni> but then you have a very obscure error
<antocuni> (and also, it would require a big refactoring of how trampolines are autogenerated and I don't feel like doing it now :))
<mattip> ok, #else #define HPy_AsPyObject #error...
<antocuni> I don't think you can #define Something #error
<mattip> ahh, so a runtime error is fine. Anyhow, in pure universal mode cpy_PyObject should be undefined, right?
<antocuni> kind of
<antocuni> basically, in CPython and Hybrid mode we use the "real" types which comes from Python.h
<antocuni> in universal mode we create fake cpy_* types
<antocuni> we need them because e.g. we need a type for HPyModule_Spec.legacy_methods
<antocuni> but since they are only forward declarations it means that it's impossible to instantiate them, and so in practice it's impossible to use .legacy_methods=...
<antocuni> and btw this is a much better approach than what we have on master; on master, we have "typedef struct _object cpy_PyObject", but this works only as long as CPython calls its struct _object
<antocuni> and I think that we were going to have problems with CPython 3.11 because they changed/removed some of these struct names
<mattip> ok, so what kind of error do you get in universal mode when the compiler tries to instantiate a FORBIDDEN_cpy_PyObject ?
<mattip> maybe that error is good enough?
<antocuni> not really useful
<antocuni> "storage size of 'myvariable' isn't know"
<antocuni> but note that you really have to try hard to arrive at that point
<antocuni> also, allocating a PyObject on the stack doesn't really make sense, you always want a pointer to it
<antocuni> but I think that the most important thing is that if you use PyObject or PyMethodDef you get an error because they are not defined
<antocuni> so then the next thing you would do is to #include <Python.h>
<antocuni> and then you get a nice error which tells you that you cannot use Python.h in universal mode
<mattip> hmm. Like most enhandements, I think it is fine to put in a first version of this PR, and then polish it when others can try it out in their workflows
<mattip> without trying to solve all the problems at once
<antocuni> yes
<mattip> especially if we let it mature before a release
<antocuni> also, all of this is to be really nice to people
<antocuni> but in theory it's enough to say in the docs "if you target universal you cannot include Python.h and if you do, it's your fault"
<mattip> +1
<antocuni> ah, I think I know why clang doesn't work
<antocuni> the documentation which I linked to is for clang 16
<antocuni> but for e.g. clang 10, the attribute error is not there: https://releases.llvm.org/10.0.0/tools/clang/docs/AttributeReference.html
<mattip> well, you could add a __GCC guard if you really really want that feature
<antocuni> it seems that clang has a __has_attribute macro check
<antocuni> I'm experimenting with it
<antocuni> btw, I've discovered the very handy command "gh pr chekcs"
<antocuni> *checks
<antocuni> it shows the github jobs for the current PR directly in the terminal (with a green/yellow/red icon depending on the status), without having to use the browser
<mattip> cool
<antocuni> fangerer: if you have access to windows, could you please help me PR 371? I keep getting windows errors and fixing them blindly is not working
<fangerer> sure, give me a few minutes
<antocuni> ah, in the meantime I might have found the problem
<antocuni> I just pushed yet-another-last-fix
<fangerer> 😄
<antocuni> but if THIS also doesn't work, then I'll definitely need you :)
<mattip> I also can run windows, if you are still stuck
<antocuni> it seems that all tests are green now 🎉
<fangerer> 👍
<antocuni> my branch is ready for review; https://github.com/hpyproject/hpy/pull/371
<fangerer> I'll have a look asap
<antocuni> thanks
FFY00_ has joined #hpy
FFY00 has quit [Ping timeout: 252 seconds]