It turns out that _LIBCPP_HAS_NO_ALIGNED_ALLOCATION can never be defined
before this point (because that is the only place that defines it), so
the conditional definition of _LIBCPP_HAS_NO_ALIGNED_ALLOCATION can be
simplified.
Details
Details
- Reviewers
EricWF
Diff Detail
Diff Detail
- Repository
- rCXX libc++
- Build Status
Buildable 26078 Build 26077: arc lint + arc unit
Event Timeline
Comment Actions
I'm putting up a review for this simple change in case the intent was to allow users to define _LIBCPP_HAS_NO_ALIGNED_ALLOCATION before including a libc++ header. I don't expect that was the intent, but just in case.
If that happens to be true, I think that intent should be documented and a CMake #define should be added. I'd also like to understand why that use case is something we need to support.
Comment Actions
In general, we *do* protect against user-defined defines.
This, and defining stuff as negative (_LIBCPP_HAS_NO_XXX) lets users disable parts of the libc++ functionality by defining the macros.
So I think we should leave this "as is"
Comment Actions
The problem I have with this is that we're hence implicitly supporting this use case, which means we should make sure we don't break it, we should add a tester with this configuration, and we should document that this macro can be used to alter the behaviour of the library. Either we should do this if we have a reason to, or we should remove this accidental API by not checking whether the user defines the macro. Does this make sense?
Comment Actions
90 seconds of grepping found the following extant macros in <__config>: _LIBCPP_HAS_NO_ALIGNED_ALLOCATION, _LIBCPP_HAS_NO_ASAN, _LIBCPP_HAS_NO_ATOMIC_HEADER, _LIBCPP_HAS_NO_AUTO_TYPE, _LIBCPP_HAS_NO_BUILTIN_ADDRESSOF, _LIBCPP_HAS_NO_BUILTIN_OPERATOR_NEW_DELETE, _LIBCPP_HAS_NO_CONSTEXPR, _LIBCPP_HAS_NO_COROUTINES, _LIBCPP_HAS_NO_CXX14_CONSTEXPR, _LIBCPP_HAS_NO_CXX20_CHRONO_LITERALS, _LIBCPP_HAS_NO_DECLTYPE, _LIBCPP_HAS_NO_DEDUCTION_GUIDES, _LIBCPP_HAS_NO_GENERALIZED_INITIALIZERS, _LIBCPP_HAS_NO_GLOBAL_FILESYSTEM_NAMESPACE, _LIBCPP_HAS_NO_INT128, _LIBCPP_HAS_NO_IS_AGGREGATE, _LIBCPP_HAS_NO_LAMBDAS, _LIBCPP_HAS_NO_LIBRARY_ALIGNED_ALLOCATION, _LIBCPP_HAS_NO_LONG_LONG, _LIBCPP_HAS_NO_MONOTONIC_CLOCK, _LIBCPP_HAS_NO_NOEXCEPT, _LIBCPP_HAS_NO_NULLPTR, _LIBCPP_HAS_NO_OFF_T_FUNCTIONS, _LIBCPP_HAS_NO_PRAGMA_PUSH_POP_MACRO, _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER, _LIBCPP_HAS_NO_RVALUE_REFERENCES, _LIBCPP_HAS_NO_SPACESHIP_OPERATOR, _LIBCPP_HAS_NO_STDIN, _LIBCPP_HAS_NO_STDOUT, _LIBCPP_HAS_NO_STRONG_ENUMS, _LIBCPP_HAS_NO_THREAD_UNSAFE_C_FUNCTIONS, _LIBCPP_HAS_NO_THREADS, _LIBCPP_HAS_NO_UNICODE_CHARS, _LIBCPP_HAS_NO_VARIABLE_TEMPLATES, _LIBCPP_HAS_NO_VARIADICS, _LIBCPP_HAS_NO_VECTOR_EXTENSION.
As far as I know, we don't have testers for all of these.
Comment Actions
What I'm saying is that this is a pattern we use throughout libc++, and I'd rather follow that pattern than explain why _LIBCPP_HAS_NO_ALIGNED_ALLOCATION is different from all these others.