Open jsharpe opened 3 years ago
I imagine that cases where we need to know that this flag even exists (which I did not know) are much less with the current default?
I would need to check how intrusive this is locally. The logic that it breaks clang makes sense especially if our conly
flags are wrong for the pip packages in this repo.
Okay, we agree that this default makes more sense. But I think the name conly
does not make sense if it defaults to True
. Instead, lets rename to enable_cxx
and default it to False
. If you do this I will merge this.
@jhance I've updated it to use enable_cxx
however I've left the cffi interface using conly
as due to the use of kwargs
this would probably silently break consumers of this macro. Feel free to change this if you prefer to switch this use too.
I don't agree with your diagnosis of cffi
silently breaking due to kwargs. Its passing the kwargs to something that doesn't accept the conly
parameter as far as I can tell, so things passing conly=True
would still error. That said, easy change to make on my end when I import this.
This will take me longer to import since it is a breaking change for us internally.
Ok, I didn't follow through the logic of where kwargs was being used.
@jhance any updates on this PR?
Ping @jhance
@jhance did you ever manage to land this change internally?
This is a suggestion that the default of using C++ configuration is the wrong choice. Most python extensions are written in C and the behaviour of passing a c++ only flag e.g.
-std=c++17
to a compiler in C mode is generally tolerated but not by all (e.g. clang 13 specifies this as an error).Surely it is better for the few packages that use C++ to require explicitly opting in to C++ flags? I found that flipping this default didn't break the build of any of the pip packages I am importing.