Remove from ABI, but keep in API #32
Conversation
mpi.h
Outdated
| #define MPI_Status_set_elements_x MPI_Status_set_elements_c | ||
| #define MPI_Type_get_extent_x MPI_Type_get_extent_c | ||
| #define MPI_Type_get_true_extent_x MPI_Type_get_true_extent_c | ||
| #define MPI_Type_size_x MPI_Type_size_c |
There was a problem hiding this comment.
I am against this. Either you want to include them, then declare them explicitly and treat them as regular abi symbols; or remove them completely and do not support legacy code.
There was a problem hiding this comment.
- These symbols cannot be removed in 4.2, they have been just deprecated so far. Moreover, a full removal will do nothing more than harm users.
- But this stuff is deprecated, and we would like to somehow start to get rid of it or discourage use, right? If we keep this stuff, we will probably have to keep it FOREVER. In the future, we are not going to break the ABI compatibility of the library just for removing silly deprecated routines added 30 years ago or workarounds no longer needed like the
_xroutines. As I said in the meeting, MPI 3.0 was the only version to actually remove things and break APIs and ABIs. MPICH did not went along with it. Open MPI did, and that unravel some minor level of misery on end users. I do remember PETSc folks rushing for a workaround to fix the creation of a struct datatype.
This change is a very good compromise between removal, and keeping support forever.
- To use the ABI, applications have to be recompiled anyway.
- Upon recompilation, most code will not notice in practice that a bunch of long deprecated stuff are now preprocessor aliases.
- User binaries will reference the new replacement symbols.
- MPI implementations will not have to care about implementing and supporting this deprecated stuff, at least under the ABI (not every implementation generates bindings with scripts).
There was a problem hiding this comment.
You are effectively removing deprecated functions. The aliases are just one of the workaround for patching code. I don't have an opinion one way or another. This should be up to the forum to decide.
There was a problem hiding this comment.
You are effectively removing deprecated functions.
Yes, I'm removing them from the C ABI, but not quite from the C API, in the sense that user code compiles unmodified to avoid any pain and grief. The produced binaries will not have any references to the old stuff.
This should be up to the forum to decide.
Definitely. I'm just submitting PRs to open up the discussion, and to make the point with actual code that user applications (mpi4py so far, and only for the _x functions) can happily compile.
dalcinl
force-pushed
the
rm-deprecated
branch
6 times, most recently
from
0e6ea26
to
309285d
Compare
* Add backward API compatibility #define's. * MPI_Info_get[_valuelen]() may also be doable.
What? |
Sorry, bad copy & paste, I meant #define MPI_T_ERR_INVALID_ITEM MPI_T_ERR_INVALID_INDEX |
This one is going to be controversial... but isn't it beautiful 😇 ?
Even if it is ultimately rejected, I believe it is worth to put into condideration.
Implemented so far:
#define's for the removals.static inlinefunctions.For further consideration:
MPI_Info_get[_valuelen]()are implemented with static inline functions.inlineis not in C89, but in that case most compilers support the__inlinekeyword as an extension.#define MPI_T_ERR_INVALID_ITEM MPI_T_ERR_INVALID_INDEXto get rid of the deprecated enum value?MPI_HOST? Maybe#define MPI_HOST MPI_KEYVAL_INVALIDto defer failure to runtime?