opentelemetry-cpp
opentelemetry-cpp copied to clipboard
Need fine-grained HAVE_CPP_STDLIB
Is your feature request related to a problem?
The std::shared_ptr & std::unique_ptr are well-supported by wide-spread compilers in c++11, while std::string_view & std::variant are unavailable until c++17. We currently use a coarse-grained switch HAVE_CPP_STDLIB to control all these things. As a result, we cannot enable any of them.
Using our private implemented nonstd::shared_ptr would lead to interoperability issue with std::shared_ptr, which is quite annoying.
Describe the solution you'd like
I'd like to split HAVE_CPP_STDLIB into more fine-grained switches:
- OPENTELEMETRY_HAVE_CXX11
- OPENTELEMETRY_HAVE_STD_SHARED_PTR
- OPENTELEMETRY_HAVE_STD_UNIQUE_PTR
- OPENTELEMETRY_HAVE_CXX17
- OPENTELEMETRY_HAVE_STD_STRING_VIEW
- OPENTELEMETRY_HAVE_STD_VARIANT
- OPENTELEMETRY_HAVE_CXX20
- OPENTELEMETRY_HAVE_STD_SPAN
Describe alternatives you've considered
Didn't find a better approach.
I think the switches could be smarter to automatically detect the value from built-in macros & pragmas.
And we provide some switches for user overridings.
- 0: Automatically
- 1: Force STD
- 2: Force Non-STD
- 3: Force Abseil-cpp
- 4: Force GSL
I personally would prefer STD->Abseil/GSL->Non-STD.
I personally would prefer STD->Abseil/GSL->Non-STD.
It's encouraged to always use non-std libraries to maintain ABI compatibility ( https://github.com/open-telemetry/opentelemetry-cpp/blob/main/docs/abi-policy.md#application-binary-interface-abi-policy ). While there are macros switches provided to use other libraries, their use is discouraged. These switches were provided as in the past there were issues reported during compilation with these non-std, but I believe they all should be fixed now.
We currently use a coarse-grained switch HAVE_CPP_STDLIB to control all these things. As a result, we cannot enable any of them.
Do we have a use case why do we want to enable the STL components individually?
- I have to do tedious convertions between
std::string_viewandnonstd::string_view. (actuallyabsl::string_viewin my case, because we are using c++14) nonstd::shared_ptrandnonstd::unique_ptrbreak the strict aliasing assumption, which prevent enable strict aliasing optimization in our code. (https://github.com/open-telemetry/opentelemetry-cpp/blob/3db551f1b0026730c4b5227f1d683e1eb7b75dff/api/include/opentelemetry/nostd/shared_ptr.h#L144)
It's good to maintain ABI compatibility. If we provide such switches, we could still make the default setting ABI compatible.
For the suggested flag like OPENTELEMETRY_HAVE_STD_SHARED_PTR, could they be detected by CMake automatically so no need to expose these flags to developers?
For the suggested flag like
OPENTELEMETRY_HAVE_STD_SHARED_PTR, could they be detected by CMake automatically so no need to expose these flags to developers?
check_cxx_source_compiles(
"
#include <iostream>
#include <string_view>
int main() {
constexpr std::string_view unicode[] {\"▀▄─\", \"▄▀─\", \"▀─▄\", \"▄─▀\"};
for (int y{}, p{}; y != 6; ++y, p = ((p + 1) % 4)) {
for (int x{}; x != 16; ++x)
std::cout << unicode[p];
std::cout << std::endl;
}
return 0;
}"
LIBATFRAME_UTILS_TEST_STL_STRING_VIEW)
We use these cmake scripts to test std::string_view in our project, maybe we can also use it to test std::string_view, std::variant or std::span here?
With C++20 and upper, we can also use #if defined(__cpp_lib_variant) , #if defined(__cpp_lib_string_view) and #if defined(__cpp_lib_span) (https://en.cppreference.com/w/cpp/feature_test) . But I have no idea about how to approach these tests with bazel.
BTW: I think all the environment thaa supported by opentelemetry-cpp has
std::shared_ptrandstd::unique_ptr, but not all the environments havestd::make_span. Is it necessary to teststd::shared_ptrorstd::unique_ptr?
This issue was marked as stale due to lack of activity. It will be closed in 7 days if no furthur activity occurs.
Trying to use opentelemetry-cpp on several platforms, which have varying degrees of STL support, I am facing the same issues.
Currently:
HAVE_CPP_STDLIBis coarse grained, making it impossible to use some C++11/14/17 features without a full C++20 STL.HAVE_CPP_STDLIBpollutes the application namespace, aOPENTELEMETRYprefix is needed.
Proposal for a fix:
In CMake, change the option to:
- WITH_STL=OFF (unchanged). Only setting that guarantees ABI stability.
- WITH_STL=CXX11 (new)
- WITH_STL=CXX14 (new)
- WITH_STL=CXX17 (new)
- WITH_STL=CXX20 (new)
- WITH_STL=ON (unchanged, means WITH_STL=CXX20 as currently)
Use pre processor symbols like:
- OPENTELEMETRY_HAVE_CXX11
- OPENTELEMETRY_HAVE_CXX14
- OPENTELEMETRY_HAVE_CXX17
- OPENTELEMETRY_HAVE_CXX20
When a feature is not available in the STL library, for example when building WITH_STL=CXX17, alternate implementations needed to satisfy nostd can be chosen depending on WITH_GSL, as currently for nostd::span.
For example,
Building with WITH_STL=CXX17 will define the following symbols:
- OPENTELEMETRY_HAVE_CXX11
- OPENTELEMETRY_HAVE_CXX14
- OPENTELEMETRY_HAVE_CXX17
which means that:
- unique_ptr, shared_ptr will use the STL (available since C++11)
- index_sequence will use the STL (available since C++14)
- string_view and variant will use the STL (available since C++17)
but nostd::span will not use the STL (available in C++20 only)
As for individual flags, like OPENTELEMETRY_HAVE_STD_SHARED_PTR, I don't see the need for them.
The assumption is that if a STL implement all C++11 features, there should be no need to pick STD_SHARED_PTR independently from STD_UNIQUE_PTR. Same for any STL flavor.
I had to declare HAVE_CPP_STDLIB top-level for projects using OpenTelemetry, and wished it was prefixed with OPENTELEMETRY_, and then split if needed (suggested above). So I would love to see this coming - OPENTELEMETRY_HAVE_CXXnn - then it'll make sense for us to have these declared top-level somewhere, otherwise it seems unclean (unlikely to break things for us, but who knows if another OSS project usese HAVE_CPP_STDLIB)