On Thu, Feb 23, 2017 at 8:14 AM, Petri Savolainen
<[email protected]> wrote:
> Added ODP_DEPRECATED macro which is visible through API and
> is used to control if deprecated API definitions are enabled
> or disabled.
>
> Deprecated definitions are first moved behind this macro and
> then removed completely in a later API version. Deprecated APIs
> are disabled by default.
>
> Added configuration option --enable-deprecated to control the
> macro value.
>
> Signed-off-by: Petri Savolainen <[email protected]>
> ---
> configure.ac | 19 +++++++++--
> include/odp/api/spec/.gitignore | 1 +
> include/odp/api/spec/deprecated.h.in | 38
> ++++++++++++++++++++++
> include/odp_api.h | 1 +
> platform/Makefile.inc | 1 +
> platform/linux-generic/Makefile.am | 1 +
> .../linux-generic/include/odp/api/deprecated.h | 26 +++++++++++++++
> 7 files changed, 85 insertions(+), 2 deletions(-)
> create mode 100644 include/odp/api/spec/deprecated.h.in
> create mode 100644 platform/linux-generic/include/odp/api/deprecated.h
>
> diff --git a/configure.ac b/configure.ac
> index e4bd3a7..9927916 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -16,7 +16,8 @@ ODP_VERSION_API_MAJOR=odpapi_major_version
> AC_SUBST(ODP_VERSION_API_MAJOR)
> ODP_VERSION_API_MINOR=odpapi_minor_version
> AC_SUBST(ODP_VERSION_API_MINOR)
> -AC_CONFIG_FILES([include/odp/api/spec/version.h])
> +AC_CONFIG_FILES([include/odp/api/spec/version.h
> + include/odp/api/spec/deprecated.h])
>
> AM_INIT_AUTOMAKE([1.9 tar-pax subdir-objects])
> AC_CONFIG_SRCDIR([helper/config.h.in])
> @@ -284,7 +285,7 @@ ODP_CFLAGS="$ODP_CFLAGS -DODP_DEBUG=$ODP_DEBUG"
> ODP_ABI_COMPAT=1
> abi_compat=yes
> AC_ARG_ENABLE([abi-compat],
> - [ --disable-abi-compat disables ABI compatible mode, enables inline
> code in header files],
> + [ --disable-abi-compat disables ABI compatible mode, enables inline
> code in header files],
> [if test "x$enableval" = "xno"; then
> ODP_ABI_COMPAT=0
> abi_compat=no
> @@ -294,6 +295,19 @@ AC_ARG_ENABLE([abi-compat],
> AC_SUBST(ODP_ABI_COMPAT)
>
> ##########################################################################
> +# Enable/disable deprecated API definitions
> +##########################################################################
> +ODP_DEPRECATED=0
> +deprecated=no
> +AC_ARG_ENABLE([deprecated],
> + [ --enable-deprecated enable deprecated API definitions],
> + [if test "x$enableval" = "xyes"; then
> + ODP_DEPRECATED=1
> + deprecated=yes
> + fi])
> +AC_SUBST(ODP_DEPRECATED)
> +
> +##########################################################################
> # Default warning setup
> ##########################################################################
> ODP_CFLAGS="$ODP_CFLAGS -W -Wall -Werror -Wstrict-prototypes
> -Wmissing-prototypes"
> @@ -379,6 +393,7 @@ AC_MSG_RESULT([
> static libraries: ${enable_static}
> shared libraries: ${enable_shared}
> ABI compatible: ${abi_compat}
> + Deprecated APIs: ${deprecated}
> cunit: ${cunit_support}
> test_vald: ${test_vald}
> test_perf: ${test_perf}
> diff --git a/include/odp/api/spec/.gitignore b/include/odp/api/spec/.gitignore
> index 6702033..df9c87d 100644
> --- a/include/odp/api/spec/.gitignore
> +++ b/include/odp/api/spec/.gitignore
> @@ -1 +1,2 @@
> +deprecated.h
> version.h
> diff --git a/include/odp/api/spec/deprecated.h.in
> b/include/odp/api/spec/deprecated.h.in
> new file mode 100644
> index 0000000..a319064
> --- /dev/null
> +++ b/include/odp/api/spec/deprecated.h.in
> @@ -0,0 +1,38 @@
> +/* Copyright (c) 2017, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +/**
> + * @file
> + *
> + * Macro for deprecated API definitions
> + */
> +
> +#ifndef ODP_API_DEPRECATED_H_
> +#define ODP_API_DEPRECATED_H_
> +#include <odp/visibility_begin.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**
> + * Deprecated API definitions
> + *
> + * Some API definitions may be deprecated by this or a previous API version.
> + * This macro controls if those are enabled (and visible to the application)
> + * or disabled.
> + *
> + * * 0: Deprecated API definitions are disabled
> + * * 1: Deprecated API definitions are enabled
> + */
> +#define ODP_DEPRECATED @ODP_DEPRECATED@
I am confused here because this turns a compiler dependency into a
build system + compiler dependency.
Lots of libraries choose to use Autotools as a build system, but it's unnatural
to require API header files like odp.h to also use Autotools. Anyone can grab
these API header files and: include them in their application to use ODP, to
create stubs for an ODP implementation, and most importantly they should
be usable with any build system (Makefile, Autotools, CMake, ninja...).
The original problem stated here is that the current code for the deprecated
macro requires the use of GCC. The fix, or the way to address portability, can
simply be to conditionally compile different code for different compilers:
#ifdef __GCC__
#define DEPRECATED __attribute__((deprecated))
#elif MSCV
#define DEPRECATED __declspec(deprecated)
#else
// define it to nothing... a nop...
#define DEPRECATED
// or... if an implementation must be required for all compilers:
#error Please add support for your compiler in compiler.h
#endif
It is succinct an easy to maintain compared to embedding these portability
concerns deeper into areas such as the build system and API header files.
The same approach to portability can be used for architecturally
specific C code as well as any other system specific macros provided by
the compiler.
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#include <odp/visibility_end.h>
> +#endif
> diff --git a/include/odp_api.h b/include/odp_api.h
> index 73e5309..962415f 100644
> --- a/include/odp_api.h
> +++ b/include/odp_api.h
> @@ -18,6 +18,7 @@
> extern "C" {
> #endif
>
> +#include <odp/api/deprecated.h>
> #include <odp/api/version.h>
> #include <odp/api/std_types.h>
> #include <odp/api/compiler.h>
> diff --git a/platform/Makefile.inc b/platform/Makefile.inc
> index 874cf88..f282770 100644
> --- a/platform/Makefile.inc
> +++ b/platform/Makefile.inc
> @@ -29,6 +29,7 @@ odpapispecinclude_HEADERS = \
> $(top_srcdir)/include/odp/api/spec/cpumask.h \
> $(top_srcdir)/include/odp/api/spec/crypto.h \
> $(top_srcdir)/include/odp/api/spec/debug.h \
> + $(top_srcdir)/include/odp/api/spec/deprecated.h \
> $(top_srcdir)/include/odp/api/spec/errno.h \
> $(top_srcdir)/include/odp/api/spec/event.h \
> $(top_srcdir)/include/odp/api/spec/hash.h \
> diff --git a/platform/linux-generic/Makefile.am
> b/platform/linux-generic/Makefile.am
> index deab284..242c54a 100644
> --- a/platform/linux-generic/Makefile.am
> +++ b/platform/linux-generic/Makefile.am
> @@ -34,6 +34,7 @@ odpapiinclude_HEADERS = \
> $(srcdir)/include/odp/api/cpumask.h \
> $(srcdir)/include/odp/api/crypto.h \
> $(srcdir)/include/odp/api/debug.h \
> + $(srcdir)/include/odp/api/deprecated.h \
> $(srcdir)/include/odp/api/errno.h \
> $(srcdir)/include/odp/api/event.h \
> $(srcdir)/include/odp/api/hash.h \
> diff --git a/platform/linux-generic/include/odp/api/deprecated.h
> b/platform/linux-generic/include/odp/api/deprecated.h
> new file mode 100644
> index 0000000..82797eb
> --- /dev/null
> +++ b/platform/linux-generic/include/odp/api/deprecated.h
> @@ -0,0 +1,26 @@
> +/* Copyright (c) 2017, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +/**
> + * @file
> + *
> + * Control deprecated API definitions
> + */
> +
> +#ifndef ODP_PLAT_DEPRECATED_H_
> +#define ODP_PLAT_DEPRECATED_H_
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <odp/api/spec/deprecated.h>
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> --
> 2.8.1
>
