[Aws::Crt::Optional] avoid std::aligned_storage

[kedartal]

Description of changes:

Checks if std::optional is available, and if so, alias Aws::Crt::Optional<T> to std::optional<T>.
Also, fix a missing #include <type_traits> when not aliasing, for std::aligned_storage.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[kedartal]

Context: std::aligned_storage is deprecated in c++23 (cplusplus/draft#5282). Instead of awkwardly working around it (ignoring deprecated warnings, setting _LIBCPP_DISABLE_DEPRECATION_WARNINGS...) or re-implementing Aws::Crt::Optional, using the standard implementation seems simpler moving forward.

Please note that std::optional is not functionally identical to the Aws::Crt::Optional implementation in c++20 and above -- constexpr and monadic operations were added. Such functionality can be used conditionally by testing the standard __cpp_lib_optional macro (https://en.cppreference.com/w/cpp/feature_test). I couldn't think of a reason why this would break any existing code, but naturally I might be missing something.

[graebm]

We can't simply alias the two types like this.

We used to do that, but later realized it didn't work, and removed the aliasing here: 402d222

using Optional = std::optional; seems like the smart thing to do if your application is c++17. Except we always compile aws-crt-cpp with c++11, so the library's compiled code uses the ABI of our custom Aws::Crt::Optional class. And if a c++17 application includes that header, it’s like “oh yeah I’m c++17 so the ABI of this thing is std::optional”

Solution is: always define our own custom Optional class, even if user’s application is c++17. They’re just different classes, each with its own ABI, even if they have the same API

[graebm]

Can we get rid of the deprecation warnings by using alignas instead of aligned_storage, as suggested on this stackoverflow post?

Or we might be able to just remove the alignment stuff. I don't believe this library is putting anything in its custom Optional class that actually has alignment requirements

[kedartal]

@graebm I see, though why is aws-crt-cpp always compiled with c++11? Anyway yes, alignas can work, or alternatively std::aligned_alloc (https://en.cppreference.com/w/c/memory/aligned_alloc). Any preferences?
Removing the alignment could break clients that use the class with their own types, but perhaps that's not a priority -- I'd definitely document it :)

[kedartal]

@graebm Something like that (just pushed)?

[graebm]

...alignas can work, or alternatively std::aligned_alloc (https://en.cppreference.com/w/c/memory/aligned_alloc). Any preferences?

std::aligned_alloc would perform an unnecessary allocation. Let's use alignas for in-place storage.

[graebm]

why is aws-crt-cpp always compiled with c++11?

If the library is used by multiple applications on the system (like if aws-crt-cpp ends up in a package manager, and is distributed precompiled), we want to be sure it's compatible with our minimum spec C++11.

[kedartal]

@graebm you're right about std::aligned_alloc -- users could always make "fat" types cheap to move, this shouldn't be the responsibility of the optional container.
I don't think there would be any difference between std::array<char, sizeof(T)> and char[ sizeof(T) ] in this case, let's use whichever you like.
I wasn't aware that aws-crt-cpp is meant to guarantee ABI stability, makes sense; perhaps it's worth mentioning that in the documentation as a requirement, and maybe prepare for allowing ABI versioning (e.g, with linker scripts) since coroutines already landed and senders-receivers are just around the corner :)

[kedartal]

@graebm please let me know what you think, I believe there are two options:

• make an ABI compatible change: in libc++, libstdc++ and msvc std::aligned_storage boils down to a union of unsigned char[] and a type that holds the alignment, so that if we made sure m_storage is large enough we should be ok (let's make it unsigned just for aesthetics)
  https://github.com/llvm/llvm-project/blob/042f01b28928c7acb3ce44e4e6281bf229bebb1c/libcxx/include/__type_traits/aligned_storage.h#L86
  https://gcc.gnu.org/onlinedocs/gcc-4.8.2/libstdc++/api/a01609_source.html#l01693
  https://github.com/microsoft/STL/blob/f8b8ad6dea901d360d8165cc6719de47ca9d9ca2/stl/inc/type_traits#L1035
• don't fix: add an explicit warning conditional on c++23 and above to make it easier for users to diagnose

[kedartal]

@graebm ... for the first option, something like alignas(T) unsigned char m_storage[ std::max( sizeof(T), sizeof(std::max_align_t) ];

[graebm]

I wasn't aware that aws-crt-cpp is meant to guarantee ABI stability

We don't guarantee it right now. But we aspire to someday