brevzin / cpp_proposals

My WG21 proposals
35 stars 22 forks source link

P2287 Designated-initializers for Base Classes #81

Closed brevzin closed 1 month ago

brevzin commented 1 year ago

P2287R1 Designated-initializers for Base Classes

https://github.com/cplusplus/papers/issues/978

h-vetinari commented 1 year ago

I don't see it mentioned in R2, so I just thought I'd mention that this touches upon C-compatibility; I'd be surprised if you didn't know, but perhaps this could strengthen the motivation?

Background: Currently in CPython-land, this is causing situations where the C99 syntax is not supported in C++, and non-designated initializers lead to clashes because apparently mixing designated & non-designated initializers in the codebase is forbidden.

This kinda damned-if-you-do-damned-if-you-don't (unless you wholesale force your entire C codebase to ignore a useful feature) is really unfortunate IMO.

brevzin commented 1 year ago

@h-vetinari I don't follow. C doesn't have base classes, so this has nothing to do with C at all. What's the CPython issue? The linked comment claims that C++ doesn't support designated initializers, but it does.

h-vetinari commented 1 year ago

Hey, thanks for the quick response! I took the statements of the python devs at face value (admittedly, the initializer situation in all its facets is confusing to me), and remembered I had seen a designated initializer paper from you. I apologise for the noise.

In case you're nevertheless interested in some more details/closure on this: As it turns out, there was another comment that appeared while I was looking for your papers and ended up here, which explains the situation in more detail, and is representative of the difficulties in this space, especially for a project that tries to be compatible with C/C++.

Alright, I figured it out. It's a problem with C++ and more specifically std:c++20 which some builds use as an option. It has this clause where we cannot mix designated-initializers with non-designated-initializers.

The problem is that with the replacement of _PyObject_IMMORTAL_INIT with PyObject_HEAD_INIT we now introduced this issue where places like Include/internal/pycore_runtime_init.h are using designated initializers, whereas some other places in our codebase are not (like PC/_wmimodule.cpp or Lib/test/_testcppext.cpp).

To fix the "mix of designated and non-designated initializers" error in Include/internal/pycore_runtime_init.h, we now need to add designated initializers to PyObject_HEAD_INIT. However, if we do that, now Lib/test/_testcppext.cpp will have a "mix of designated and non-designated initializers" error. Now, the problem is that if we fix that and add designated initializers to these extensions, they now fail because C++ does not support designated initializers in non C++20.

I think the solution here is to either remove the designated initializers in Include/internal/pycore_runtime_init.h or, perhaps just create a _PyObject_HEAD_INIT that has designated in Include/internal/pycore_runtime_init.h. I'm inclining for the latter (and just include a comment about the C++20 errors), let me try it out and see how it goes.

The solution then was to eschew designated initializers entirely, which is a net negative for a C-heavy codebase IMO, even in the name of C++ compatibility. However, IIUC, the key point in this case is that the required C++ support already exists from C++20 onwards, so this problem will be solved eventually when the baseline C++ version can be lifted that high (which is obviously not the case for CPython yet).

brevzin commented 1 year ago

Sorry I still don't... at all understand the issue you're describing. CPython was already using designated initializers before that PR, so why would C++ suddenly fail by adding more of them? Designated initialization is a C++20 feature, but gcc and clang have supported it forever regardless. MSVC refuses to before C++20, but if you're using Windows, then this never would've worked on C++ anyway.

This sentence, for instance:

The problem is that with the replacement of _PyObject_IMMORTAL_INIT with PyObject_HEAD_INIT we now introduced this issue where places like Include/internal/pycore_runtime_init.h are using designated initializers, whereas some other places in our codebase are not

This sounds like you're saying it's a problem that somewhere you're writing Point{.x=1, .y=2} and elsewhere you're writing Point{3, 4}. But that's fine. The issue (in C++) is that you can't write Point{1, .y=2} or Point{.x=3, 4}. Or, even weirder stuff that C allows, like Point{.x=4, 5, .y=6} (where the 5 is just totally ignored).

And those can be fixed by just making all of these cases designated, since all of these members do have names. The one case where it can't is base classes, since those don't have names, hence this proposal of mine.

h-vetinari commented 1 year ago

Sorry I still don't... at all understand the issue you're describing.

That's on me for picking up something I only encountered second-hand. Apologies. If I manage to document better what problems were encountered (or reproduce them), I'll comment here again. Thank you for your time.