Add C++11/14 support utility file #10868

kjbracey · 2019-06-19T13:52:51Z

Description

As we start trying to use new facilities, we're likely to need some more helpers.

In particular, ARM C 5 has no C++11 support in its library at all, so to avoid totally breaking it we need some backup.

For the other toolchains, we can add a few C++17/C++20/TS extensions into namespace mbed to make life a little easier.

For ARM C 5: C++14 type_traits subset, std::move, std::forward, std::array, std::initializer_list, std::begin, std::end, std::align, std::maxalign_t, std::aligned_storage, alignof + alignas macro replacements.
For ARM C 5: MBED_CONSTEXPR_FN_14 and MBED_CONSTEXPR_OBJ_14 to mark things that can only be constexpr in C++14 or later.
For other compilers: mbed::void_t, mbed::type_identity, mbed::conjunction, mbed::disjunction, mbed::negation, mbed::experimental::nonesuch, mbed::experimental::is_detected family, mbed::as_const, mbed::remove_cvref.

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[X] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@pan-, @bulislaw

Release Notes

Various modern C++ forward-compatibility helpers have been added in mbed_cxxsupport.h to assist with C++11 and later code, including supporting ARM C 5 as much as possible.

ciarmcom · 2019-06-19T17:00:32Z

@kjbracey-arm, thank you for your changes.
@ARMmbed/mbed-os-maintainers please review.

kjbracey · 2019-06-20T07:00:40Z

The majority of content in this file is a crutch to fill in the limited C++11 support in ARM C 5, so there is at least some chance of someone being able to build master with ARM C 5 once later PRs start using C++11/14.

I could leave it out and greatly simplify it, but it would mean ARM C 5 builds will soon just not work at all.

pan- · 2019-06-20T08:46:57Z

Last commit looks great! And maybe it would deserve its own PR.

Few things that can be added:

std::begin and std::end that can be used in range for loop.
More type traits; a lot are implemented by the compiler: http://www.keil.com/support/man/docs/armcc/armcc_chr1359124947145.htm
std::max_align_t and std::align, std:: alignment_of and std::aligned_storage
result_of and invoke_result.

kjbracey · 2019-06-20T08:51:09Z

Last commit looks great! And maybe it would deserve its own PR.

I've actually dropped it from this PR for now, and agree that it would. I believe there are some glitches with backporting it to C++14 - in particular stricter enum initialization rules, so it might not quite fly.

(I'm not 100% sold on std::byte in practice yet, arguably due to lack of experience, so I'm not desperate to push it).

Few things that can be added:

* `std::begin` and `std::end` that can be used in range for loop.

Good point.

* More type traits; a lot are implemented by the compiler: http://www.keil.com/support/man/docs/armcc/armcc_chr1359124947145.htm

Indeed, thanks for pointing that out!

Just spotted that myself an hour ago. (Not by reading the documentation - by doing strings armcc | grep is_convertible, finding __is_convertible_to then Googling it and hitting the Keil manual). That will help immensely.

* `std::max_align_t` and `std::align`, `std:: alignment_of` and `std::aligned_storage`

Sorry, no. ARM C 5 does not support alignof/alignas.

* `result_of` and `invoke_result`.

Will look at those.

kjbracey · 2019-06-20T08:54:00Z

BTW, ARM C 5 cannot do expression SFINAE, so the Span.h is_convertible is not SFINAE usable in ARM C 5. Which is why I was groping around checking for hidden intrinsics.

pan- · 2019-06-20T09:58:18Z

Sorry, no. ARM C 5 does not support alignof/alignas.

That's one more reason to implement std::aligned_storage with __attribute__((aligned)) then.

kjbracey · 2019-06-20T13:00:02Z

Okay, added most of your suggestions.

It seems there is enough to get all the alignment things apparently working, except alignas(Type) won't work, you'll have to use alignas(alignof(Type)).

I've baulked at common_type, result_of and invoke_result - getting pretty complex.

bulislaw

It's not really a functionality change, more of a "feature" in a sense we enable something new in the old compiler.

adbridge · 2019-06-21T10:53:02Z

@pan- are you happy with the updates ?

pan-

Very good!
It would be nice to have some tests to static_assert results of traits and make sure compilation passes on all target; today that file isn't included anywhere and won't disrupt the CI even if there's error inside. I can help with that if you need a hand.

Note: I haven't reviewed array and what's bellow yet.

platform/mbed_cxxsupport.h

pan- · 2019-06-21T10:41:45Z

platform/mbed_cxxsupport.h

+} // namespace impl
+
+template <typename T>
+struct is_member_pointer : impl::is_unqualified_member_pointer<remove_cv_t<T>> { };


mbed::disjunction< is_member_function_pointer<T>, is_member_object_pointer<T>> ?

Logically, that's what it is, and it's shorter to write, but I shuddered at the expanded complexity. Each of those is a very simple template specialization, leading to the very complex is_function.

Seemed weird to effectively write if ((is_member && is_function) || (is_member && !is_function)) when is_function is so complex and is_member so simple.

But I have no good feel on how to optimise for compilation speed. I guess the disjunction form less work when not instantiated, as it's a single template? But if instantiated it's dozens of times more complex.

Created a new version passing a filter template to the is member detector. Not sure if that's better.

platform/mbed_cxxsupport.h

pan- · 2019-06-21T11:00:15Z

platform/mbed_cxxsupport.h

+
+/* forward */
+template <typename T>
+T &&forward(remove_reference_t<T> &a) noexcept


Same as above, it may be marked as constexpr.
You should also provide the second overload:

template <typename T> T &&forward(remove_reference_t<T> &&a) noexcept

Hmm, lifted that from ARM C 5 manual, which only had the first overload. I wonder if there was any reason for that, or just an editorial error.

kjbracey · 2019-06-24T07:17:48Z

It's not really a functionality change, more of a "feature" in a sense we enable something new in the old compiler.

"feature" isn't on the list template :)

kjbracey · 2019-06-24T07:38:41Z

Very good!
It would be nice to have some tests to static_assert results of traits and make sure compilation passes on all target; today that file isn't included anywhere and won't disrupt the CI even if there's error inside. I can help with that if you need a hand.

Thanks for scanning though this.

The CI isn't currently going to catch anything from the bulk of this anyway, as ARM C 5 is not tested. (!)

Nevertheless, some tests would be appreciated. It would be worth you doing them as a second pair of eyes - it's always better if people don't write their own tests, and as this is implementing a clear external spec, we get to cross-check interpretation of that spec.

Actual static asserts should be straightforward to pass - more problematic is SFINAE traits use in enable_if or via void_t. I haven't fully figured out what idioms do and don't work in ARM C 5 - when I've got more info, I'll stick some comments in the file reflecting my findings.

I'm somewhat torn on how much effort to expend on ARM C 5. We're liable to end up doing 90% of the work in this area for a non-supported toolchain...

pan- · 2019-06-24T08:05:54Z

Thanks @kjbracey-arm I will add tests for the traits you added then. On one of the project I'm working on; we're working with ARM C 5 and C++11 enabled. I've already re-implemented some of the traits we needed but a more complete shim is more than welcome.

It feels like a complete move to ARMC 5 was a bit premature as ARMC 6 hasn't caught up yet on density of the code it generates. Size really matters for customers.

kjbracey · 2019-06-24T09:49:11Z

Updated incorporating comments.

I feel bad adding 1000 lines of templates to a header (no doubt more for non ARM C 5 pulling in a real <type_traits>), doing heavens-knows-what to compilation time.

So I'm going to offset by taking 4000 lines out of Callback.h in another PR in a moment.

kjbracey · 2019-06-24T15:33:57Z

The detection idiom is driving me slightly nuts. As far as I can see, void_t works on ARM C 5, as long as we have the fallback form with struct void_helper - it doesn't work as a plain using.

Then any idiom I construct manually with void_t seems to work, eg the first example from N4436:

template< class, class = void_t<> >
struct has_type_member : false_type { };
template< class T >
struct has_type_member<T, void_t<typename T::type>> : true_type { };

Or a has_common_type using common_type_t. (And my common_type_t also uses void_t).

But trying to do the same test via the detection idiom, eg is_detected<common_type_t, X, int>, fails. Maybe a bug in template template expansion? Haven't actually gotten any working test with detector...

kjbracey · 2019-06-26T11:24:26Z

Added common_type, corrected errors in add_pointer.

As we start trying to use new facilities, we're likely to need some more helpers. In particular, ARM C 5 has no C++11 support in its library at all, so to avoid totally breaking it we need some backup. For the other toolchains, we can add a few C++17/C++20/TS extensions into namespace mbed to make life a little easier. * For ARM C 5: C++14 type_traits subset, std::move, std::forward, std::array, std::initializer_list, std::begin, std::end, std::align, std::maxalign_t, std::aligned_storage, alignof + alignas macro replacements. * For ARM C 5: MBED_CONSTEXPR_FN_14 and MBED_CONSTEXPR_OBJ_14 to mark things that can only be constexpr in C++14 or later. * For other compilers: mbed::void_t, mbed::type_identity, mbed::conjunction, mbed::disjunction, mbed::negation, mbed::experimental::nonesuch, mbed::experimental::is_detected family, mbed::remove_cvref, mbed::as_const.

kjbracey · 2019-06-27T11:06:49Z

Couple more minor tweaks - simplify "add_pointer"/"add_reference", then add "referenceable" checks. Flag out the detector idiom until we can figure out how to make it work on ARM C 5.

0xc0170 · 2019-07-04T13:56:41Z

It feels like a complete move to ARMC 5 was a bit premature as ARMC 6 hasn't caught up yet on density of the code it generates. Size really matters for customers.

Hopefully ARMC6 gets there asap 🙏

@pan- Happy with this as it is now?

0xc0170

👍 makes my head to spin hard :-)))))

0xc0170 · 2019-07-04T14:34:09Z

CI started

pan-

LGTM.

mbed-ci · 2019-07-04T17:04:10Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

kjbracey mentioned this pull request Jun 19, 2019

Add Atomic<T> template #10274

Merged

kjbracey force-pushed the cxxsupport branch from f2ce0a2 to 661ece0 Compare June 19, 2019 14:09

ciarmcom requested review from bulislaw, pan- and a team June 19, 2019 15:00

ciarmcom added the needs: review label Jun 19, 2019

kjbracey force-pushed the cxxsupport branch from 661ece0 to f7b74a5 Compare June 20, 2019 06:55

kjbracey force-pushed the cxxsupport branch from f7b74a5 to 5c8a1a7 Compare June 20, 2019 07:07

kjbracey force-pushed the cxxsupport branch from 5c8a1a7 to f026a58 Compare June 20, 2019 12:56

kjbracey force-pushed the cxxsupport branch from f026a58 to 6afda5f Compare June 20, 2019 13:01

bulislaw approved these changes Jun 21, 2019

View reviewed changes

adbridge added the release-version: 5.14.0-rc1 label Jun 21, 2019

pan- requested changes Jun 21, 2019

View reviewed changes

kjbracey force-pushed the cxxsupport branch from 6afda5f to 5c99a24 Compare June 24, 2019 09:45

kjbracey force-pushed the cxxsupport branch from 5c99a24 to c9abd82 Compare June 24, 2019 11:55

kjbracey force-pushed the cxxsupport branch from c9abd82 to b39fb4b Compare June 26, 2019 11:23

kjbracey force-pushed the cxxsupport branch from b39fb4b to 80afc3a Compare June 27, 2019 11:04

0xc0170 approved these changes Jul 4, 2019

View reviewed changes

pan- approved these changes Jul 4, 2019

View reviewed changes

0xc0170 added ready for merge and removed needs: review labels Jul 5, 2019

0xc0170 merged commit efb84f0 into ARMmbed:master Jul 5, 2019

0xc0170 removed the ready for merge label Jul 9, 2019

kjbracey mentioned this pull request Jul 12, 2019

Add <mstd_xxx> C++ headers #11039

Merged

Add C++11/14 support utility file #10868

Add C++11/14 support utility file #10868

Uh oh!

Conversation

kjbracey commented Jun 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Reviewers

Release Notes

Uh oh!

ciarmcom commented Jun 19, 2019

Uh oh!

kjbracey commented Jun 20, 2019

Uh oh!

pan- commented Jun 20, 2019

Uh oh!

kjbracey commented Jun 20, 2019

Uh oh!

kjbracey commented Jun 20, 2019

Uh oh!

pan- commented Jun 20, 2019

Uh oh!

kjbracey commented Jun 20, 2019

Uh oh!

bulislaw left a comment

Choose a reason for hiding this comment

Uh oh!

adbridge commented Jun 21, 2019

Uh oh!

pan- left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pan- Jun 21, 2019

Choose a reason for hiding this comment

Uh oh!

kjbracey Jun 24, 2019

Choose a reason for hiding this comment

Uh oh!

kjbracey Jun 24, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pan- Jun 21, 2019

Choose a reason for hiding this comment

Uh oh!

kjbracey Jun 24, 2019

Choose a reason for hiding this comment

Uh oh!

kjbracey commented Jun 24, 2019

Uh oh!

kjbracey commented Jun 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pan- commented Jun 24, 2019

Uh oh!

kjbracey commented Jun 24, 2019

Uh oh!

kjbracey commented Jun 24, 2019

Uh oh!

kjbracey commented Jun 26, 2019

Uh oh!

kjbracey commented Jun 27, 2019

Uh oh!

0xc0170 commented Jul 4, 2019

Uh oh!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Jul 4, 2019

Uh oh!

pan- left a comment

Choose a reason for hiding this comment

Uh oh!

mbed-ci commented Jul 4, 2019

kjbracey commented Jun 19, 2019 •

edited

Loading

kjbracey commented Jun 24, 2019 •

edited

Loading