CHARLS_NO_DISCARD vs CHARLS_CHECK_RETURN
malaterre opened this issue · 6 comments
It would be super nice to clarify the need for CHARLS_CHECK_RETURN (now that we have [[nodiscard]]) in CharLS code base.
CHARLS_NO_DISCARD and CHARLS_CHECK_RETURN have the same intend. CHARLS_NO_DISCARD is only active when consuming the CharLS headers in C++17 mode. CHARLS_CHECK_RETURN is active for C and C++14, but only for the MSVC compiler ([[nodiscard]] requires C++17 or newer).
It would be probably possible to express CHARLS_NO_DISCARD in CHARLS_CHECK_RETURN and use only CHARLS_NO_DISCARD in the public API.
It also seems that I missed an opportunity:
RETURN_TYPE_SUCCESS_(return == 0)
enum class jpegls_errc
{
// ...
}
can be defined as
RETURN_TYPE_SUCCESS_(return == 0)
enum class CHARLS_NO_DISCARD jpegls_errc
{
// ...
}
If you ever keep CHARLS_CHECK_RETURN , pay attention that you have equivalent syntax for GCC/Clang:
#define CHARLS_CHECK_RETURN __attribute__ ((warn_unused_result))
[Mostly for discussion]
Reading the attributes page of GCC makes me think that one could (almost) have an equivalent to IN_ and OUT_ with:
__attribute__ ((access (read_only, 1)))
and
__attribute__ ((access (write_only, 1)))
Something like this:
#ifdef _MSC_VER
#define IN_ _In_
#define IN1_ IN_
#define IN2_ IN_
#else
#define IN1_ __attribute__ ((access (read_only, 1)))
#define IN2_ __attribute__ ((access (read_only, 2)))
...
The MSVC SAL attributes need to be in front of the parameters, the GCC attribute needs to be for or after the function.
Most functions that use pointers using IN_ or OUT_ also have already the CHARLS_ATTRIBUTE((nonnull)) for GCC/Clang.
Will experiment with the access attribute.
It seems I can't read documentation properly 😕. Indeed the 'access' macros would need to be seriously updated (take an argument)...