Providing `std::` functions, but not related headers can be problematic

[matthijskooijman]

I've been using this library today (rather, the one bundled with MsgPack, but that's the same as far as this issue is concerned) and ran into some issues.

In my sketch, I'm using the ArduinoSTL (uclibc++ and STL) library, which contains implementation of most of C++98 stdlibc++ (which is otherwise missing on AVR), making everything available under the standard names (e.g. <algorithm>). When I add the MsgPack.h include, this starts producing conflicts because ArxTypeTraits defines some classes and functions, which are also defined by ArduinoSTL. e.g.:

.../ArduinoSTL/src/algorithm:380:37: error: redefinition of 'template<class T> void std::swap(T&, T&)'
  template<class T> _UCXXEXPORT void swap(T& a, T& b){
.../MsgPack/MsgPack/util/ArxTypeTraits/ArxTypeTraits/type_traits.h:382:10: note: 'template<class T> void std::swap(T&, T&)' previously declared here
     void swap(T& a, T& b)

I already tried two things to work around this, without luck so far:

• I could remove all standard includes (e.g. #include <algorithm> from my code and rely on ArxTypeTraits to provide the needed declarations and implementations. However, this makes the code less portable, and more importantly does not work when using third party code (e.g. I'm also using ArduinoJson now, which also does #include <string> (which indirectly includes <algorithm> as well).
• I tried modifying ArxTypeTraits to not define anything (i.e. by removing #define ARX_TYPE_TRAITS_DISABLED) and instead letting it include standard header files (which would then include ArduinoSTL), but there I ran into the problem that uclibc++ does not offer <tuple>, since that was added in C++11.

One way to fix this seems to be to let ArxTypeTraits offer the functionality under the proper header names, e.g. let it actually have a file called algorithm. This does have some downsides:

• If it does not implement everything that that header should implement, then any other users of the same header might end up missing things.
• This can be somewhat worked around by using e.g. ArduinoSTL instead, and putting it earlier in the include path (by including it before ArxTypeTraits in the .ino file), but then you'd again end up missing C++11 bits (for e.g. <tuple>, this would work because ArduinoSTL does not offer the file at all, so the include would fall through to ArxTypeTraits, but for other stuff, like std::enable_if_t, that was added to existing header files, this would not work.
• This is also a bit tricky to combine with the approach of including ArxTypeTraits in other libraries that need it, since it would end up in the util subdirectory and not on the include path (but given it is a header only library and uses #ifndef header guards, it could maybe work in this case to just include the library twice: Once bundled in e.g. MsgPack, and once as a standalone library, as long as the libraries define the same things, that should be ok).

Another way I can imagine might work, is to tell ArxTypeTraits about what parts of libstdc++ are (not) available. Right now, it assumes that on a few Arduino architectures, libstdc++ is completely unavailable, and defines parts of it itself, and everywhere else it assumes that libstdc++ (C++11 always, newer versions depending on the __cplusplus macro). I'm ignoring the handling of new.h now, which I think might need fixing separately.

It could help if we could just use ArduinoSTL and then tell ArxTypeTraits (e.g. through a macro or so) that libstdc++98 is available, letting it just include the standard header files, and defining only the things that were added in c++11 and later. This might still end up being somewhat problematic (since I think ArduinoSTL currently defines some things from c++11 but far from everything), but it might be a good place to start.

For now, this is not blocking me since I can test on other platforms than AVR for a while, but I might end up trying the latter approach at some point.

[matthijskooijman]

On additional datapoint (which is essentially a separate issue, but might need to be kept in account while fixing this one): In some cases, gcc's libstdc++ can provide functions from C++ versions newer than what __cplusplus defines, causing conflicts with ArxTypeTraits. I noticed a conflict on e.g. std::void_t (defined in C++14) when compiling with -std=gnu++11. Switching to -std=c++11 fixed this, since these "extra" definitions seem to be conditional on !__STRICT_ANSI__, see https://github.com/gcc-mirror/gcc/blob/b0c990f2661a2979c68c840781847efe27a0779b/libstdc%2B%2B-v3/include/std/type_traits#L2575-L2579

It does seem to define the standard version for each such symbol (e.g. #define __cpp_lib_void_t 201411), so that could possibly be checked against as well.

Note that I noticed this compiling outside of Arduino, but I would expect this to happen inside Arduino as well (given that at least the official cores tend to run with -std=gnu++something).

[hideakitai]

Hmm... these are difficult problems...

If it does not implement everything that that header should implement, then any other users of the same header might end up missing things.

Yes, this is the reason why I haven't named Arx** series to the same as standard libraries. I won't (and can't) conver all of the functions of standard libraries, so I've named libraries as Arx***. But in that case, it's definitely correct that this library should be used with ulibc++ (ArduinoSTL) which is intended to cover all of them (far from it, though...).

I think the best way is a selective compile as you imagined, like excluding existing functions using #ifdef ARDUINOSTL_INCLUDED. It is also good for std::void_t problem on -std=gnu++11. (Sorry, I didn't know that...thanks!)

If I have time (I can't say when is the estimated date...), maybe it can be achived to make this library works with ArduinoSTL. Working with -std=gnu++11 will be earlier maybe.

How do you think about it?

[matthijskooijman]

I think the best way is a selective compile as you imagined, like excluding existing functions using #ifdef ARDUINOSTL_INCLUDED.

Yeah, I think this is the right approach. I'm already testing some things to maybe implement that, I'll get back to you (hopefully with a PR). I'll also tackle the operator new stuff, which I think is not implemented entirely correctly on e.g. SAMD (but I'm not entirely sure what would be correct).

Note that ARDUINOSTL_INCLUDED in the sense of #include "ArduinoSTL.h" has previously occurred is not the right check, since it might also be included after ArxTypeTraits of course. More appropriate would be ARDUINO_STL_AVAILABLE, but I'd like to see if I can maybe generalize this a bit more using e.g. #define ARX_HAVE_CPLUSPLUS 201104 or so. We'll see if that can be made to work.

[matthijskooijman]

I tried some more fine-grained definitions, with some success, but I'm not sure what the best approach is yet. Below some of my findings and thoughts, partly for review, and partly to help me organize the subject matter and evaluate options :-)

In my testing, I found some other complicating factors (mostly solvable or solved, just wanted to list them here):

• ArduinoSTL implements mostly C++98, but also has <initializer_list> from C++11.
• The Arduino AVR core has been defining placement new in <new.h> for a while (and recently also in <new>), but it seems the megaAVR core was based on an old version that does not have placement new yet...
• Even though ArxTypeTraits defines some C++98 stuff, it really needs at least C++11, at least for the compiler, so __cplusplus must be at least 2011something.
• On SAM, SAMD, and probably also spresence, all ARM targets, libstdc++ is available normally and should be usable (confirmed on SAMD that std::function and placement new works), so I think ArxTypeTraits should not define the <= C++11 stuff there.
• ArxContainer also defines intializer_list, so there are already some include cards to make sure only one of ArxTypeTraits and ArxContainer defines it. This is a bit of a hack, but it does work.
• I found that OF_VERSION_MAJOR combined with TARGET_WIN32 prevents some stuff from being defined, but I couldn't quite figure out why? Does openframeworks already define these things somehow? Any additional info? These are exceptions that I wouldn't mind removing, or maybe they can be expressed a bit more generically?
• Some APIs in std have been changed rather than being added. For example, numeric_limits::min() become constexpr in C++11. These changes are pretty much impossible to really get right if the old version is defined by the standard library (in case of numeric_limits, this is the case when ArduinoSTL defines the non-constexpr version, then we can't replace that with a constexpr version). Similar things might happen with libstdc++ and C++14/17/2a changes.

Anyway, further analyzing this issue, I've found that there's basically three cases that we'd like to support:

1. The easy case is when gcc's libstdc++ is available. This is pretty much every architecture except AVR. In this case, __cplusplus is a truthful indicator of the std stuff that is available, so most of the stuff can just be included. In addition, some stuff for newer C++ standards needs to be defined by ArxTypeTraits (e.g. when __cplusplus is C++11, the C++14/17/2a stuff should still be defined). There are some exceptions such as void_t which is C++14, but even defined by libstdc++ on e.g. -std=gnu++11 or below, but libstdc++ defines macros for these, so we can check for that (currently only void_t seems to conflict of these).
2. Another easy case is plain AVR or MegaAVR: This has pretty much no libstdc++ at all (except for an incomplete <new.h> and/or <new>). For this, we can just define pretty much everything. In this case, __cplusplus does not really say anything meaningful about what std stuff is available or not.
3. The most tricky case is AVR/MegaAVR combined with ArduinoSTL or some other uclibc++ implementation. This provides most of C++98, plus <initializer_list> (currently, this could change in the future). Also in this case, __cplusplus is rather meaningless.

What I tried now, is to define a ARX_HAVE_LIBSTDCPLUSPLUS to the effective available libstdc++/ArduinoSTL version and check against that instead of __cplusplus in the header files. For case 1, this macro defaults to the value of __cplusplus. For case 2, this defaults to 0 (no std stuff at all). For case 3, this is hardcoded to C++98, with an additional define to indicate that <initializer_list> is available.

To detect case 3, I've used #if __has_include(<ArduinoSTL.h>) which is a non-portable gcc extension, but quite convenient in this case.

I later realized that I could maybe also use __has_include to more generically do some stuff. I.e. instead of setting a HAVE_INITIALIZER_LIST macro in case 3, we can also just do #if __has_include(<initializer_list>), and if it is avaiable, just include it rather than defining it. This same approach could also work for other new C++11 headers such as <tuple> and <type_traits>, except that ArduinoSTL seems to already have a <type_traits> file that does not seem to define any of the contents it should have, so that breaks that approach.

I guess this will really stay a matter of detecting and handling specific cases (e.g. like ArduinoSTL), and adapting this handling when other libraries such as ArduinoSTL change. Really generically and portable defining this stuff is going to prove impossible, I'm afraid.

Looking at this from the perspective of what is being defined, I think that:

• There is a block of C++98 code, that is only needed in case 2.
• There is a block of C++11 code, along with "tuple.h" and "functional.h", which is needed in case 2 and 3.
• There is initializer_list, which is only needed in case 2 (though there might be variants of case 3 using alternative uclibc++ ports that also need it, since initializer_list is an ArduinoSTL-specific addition).
• There are blocks of C++14, C++17 and C++2a code, which might be needed in any of the cases.

So it seems that just defining a ARX_HAVE_LIBSTDCPLUSPLUS as suggested above, and additionally checking for __has_include(<initializer_list>) is a reasonable approach.

For the operator new, we can observe that, as shown above, all of functional.h is only for case 2 and 3, so only on AVR/MegaAVR. On these architectures, we know that <new.h> is available and will have normal new operators and might have placement new (on newer AVR only). So for older AVR and MegaAVR, we'll need to define our own placement new. However, in the future, placement new will likely be added to MegaAVR as well, so checking against ARDUINO_ARCH_MEGAAVR is not ideal. I'm inclined to unconditionally define an inline placement new operator here, which will get used if the Arduino core does not define one, and should be just dropped due to the inline if the Arduino core does define one. However, if the Arduino core would ever start to define this operator inline in new.h, then this would result in a conflict (but I don't think they will, and if someone considers changing new.h, they'll probably ask me first ;-p). Just tested this and it seems like including <new.h> is not even needed, since the non-placement new operators are implicitly declared by the compiler (mandated by the C++ spec).

I've pushed my work in progress for review, see PR at #2.

I also realized there is completely separate and possibly simpler way to fix all this: Instead of defining stuff in the std namespace, just put all of it in the arx (or arx_std or arx::std) namespace instead. Then all of this stuff can just co-exist with any real std stuff, probably preventing most if not all of the problems above.

On platforms where libstdc++ is properly available, you could just include the libstdc++ headers and inside the arx namespace put using namespace std; so the std stuff is available through the arx namespace (so code relying on ArxTypeTraits can just always use the arx namespace).

This approach would make some sense, since any code that uses std stuff will almost certainly also just include the standard libstdc++ headers. To make such code work with ArxTypeTraits, the code must be modified to include different header files, so why not let it use a different namespace as well?

If a sketch or other code is sure that it does not include any std stuff directly and wants to use the std namespace for arx stuff anyway, it can also do the reverse, define a namespace std { using namespace arx; } itself.

How would you feel about something like this? This would of course require a one-off change in all users of the library...

[matthijskooijman]

I found one more problem wrt to my operator new fix: ArduinoSTL defines an inline implemenation in its header, which conflicts with the inline implementation I put in functional.h. So I ended up adding one more fix which includes <new> if it exists, and falls back to an inline implementation otherwise. I also split up the ArduinoSTL fixes in smaller chunks In particular some changes that lead up to using standard libstdc++ headers on SAM/SAMD/spresence could be moved into separate commits, without also fixing the ArduinoSTL issues yet. I'm planning to also try the arx::std namespace trick, but that can be done on top of these separate commits as well.

[hideakitai]

I almost agree with your proposition. Thanks a lot :) __has_include is quite convenient…!

ArxContainer also defines intializer_list, so there are already some include cards to make sure only one of ArxTypeTraits and ArxContainer defines it. This is a bit of a hack, but it does work.

It's not a best way, but I'll add __has_include guard also to ArxContainer now.

I found that OF_VERSION_MAJOR combined with TARGET_WIN32 prevents some stuff from being defined, but I couldn't quite figure out why? Does openframeworks already define these things somehow? Any additional info? These are exceptions that I wouldn't mind removing, or maybe they can be expressed a bit more generically?

A part of C++17 features are included only on openFrameworks in Windows... like void_t by libstdc++. So I'd like to keep these guards.

So it seems that just defining a ARX_HAVE_LIBSTDCPLUSPLUS as suggested above, and additionally checking for __has_include() is a reasonable approach.

Agree :)

I also realized there is completely separate and possibly simpler way to fix all this: Instead of defining stuff in the std namespace, just put all of it in the arx (or arx_std or arx::std) namespace instead.

I wondered which is better to use arx:: and std:: for this library in the beginning, but I've selected std at that time because I haven't consider to use with other STL libraries like ArduinoSTL. But now I think these templates should be included to arx namespace as used in ArxContainer and ArxSmartPtr. It is not so annoying to use namespace std { using namespace arx; } I think. Or make a macro like #USE_ARXTYPETRAITS_AS_STD;)

I also got one more problem about operator new. Thanks a lot!

I have tested your current PR on some libraries which use ArxTypeTraits, so I'll add some comments to your PR. Thanks again!

[matthijskooijman]

A part of C++17 features are included only on openFrameworks in Windows... like void_t by libstdc++. So I'd like to keep these guards.

Ah, I see, makes sense then. There's still one caveat that in this case the guards only work if OF is included before ArxTypeTraits (unlike the libstdc++ void_t, since the ArxTypeTraits itself includes <typetraits>, so it can be sure it's included before the void_t check).

But now I think these templates should be included to arx namespace as used in ArxContainer and ArxSmartPtr.

Yeah, I have this feeling as well. That would make most of my PR irrelevant, though.

I just realized that this approach would also make the conflicts with OF and void_t disappear magically :-)

I gave the arx::std approach a whirl just now, and I think I have gotten that to work. One caveat is initializer_list, which must be defined in std for it to work (since any {1, 2, 3} initializer is typed as std::initializer_list by the compiler). But for that I suspect that just using _has_include(<initializer_list>) is actually reasonable.

I wonder one more thing, though: even when switching to arx::std, that removes conflicts with existing std implementations and/or ArduinoSTL, but still leaves the question: Should we include pre-existing std implementations or not? The easiest way is to never do that, and just always define your own stuff in arx::std (even if it is also available under std). Then you can even remove all of the __cplusplus checks and whatnot, since you'll never conflict. The downside is that the types you define are different from the std:: types, so some interoperability between code that uses arx::std and code that uses std is lost.

Another approach is to use stuff from std when it is available (importing all of std into arx::std seems to work), but then you're back to the original problem of knowing what is (not) available. The problem might have become slightly simpler, though, since now you can maybe just ignore ArduinoSTL etc. and just define everything on AVR and look at __cplusplus for all other architectures (meaning that if ArduinoSTL is used, it would just co-exist besides the ArxTypeTraits implementations). Or ArduinoSTL could be supported after all, with the approach I pushed in #2, but we also move to arx::std. I think the advantage of such a combination approach could be that it's more robust when e.g. ArduinoSTL would add more C++11 or higher stuff, since AFAICS, if you import all of std into arx::std and then define something in the latter that already exists in the former, the former is just overridden. I think this even allows upgrading stuff, like numeric_limits with a constexpr version on top of the non-constexpr ArduinoSTL version.

Let me know what you think, after lunch I'll probably try merging both approaches into a single PR.

[hideakitai]

Thanks! I prefer to use existing items first. The priority is standard library > ArduinoSTL > ArxTypeTraits. ArxTypeTraits is just supplements for the missing part of them, I think :)

[matthijskooijman]

One caveat with __has_include() is that if it returns false, it also prevents a later #include for the same filename from producing an error. So if you include ArxTypeTraits first and then ArduinoSTL, then Arduino doesn't see the missing include error for ArduinoSTL and never adds ArduinoSTL to the include path :-S This seems to be a bug in gcc, but it can be problematic for our use.

A possible workaround could have been to not use _has_include during include detection (maybe just do nothing at all in this case, since none of the includes we do are intended to add extra libraries), but it seems there is no way to detect include detection currently (at least there's nothing meaningful on the compiler commandline for the AVR core). So we could just say people shoudl include ArduinoSTL first. Alternatively, we could check for some standard header like <cstdlib> as an indication of ArduinoSTL (or another third-party standard library) to be present on AVR. This won't cause problems, since Arduino only supports including a .h file to trigger library includes now, so any suppressed errors on e.g. <cstdlib> won't break the build.

For this same reason, things like __has_include(<initializer_list>) won't cause any problems either.

Doing this will lose support for ArduinoSTL on non-AVR cores (since saying "if is available, you'll probably have ArduinoSTL and only support C++98" will certainly be false on architectures that have libstdc++). However, there is the __GLIBCXX__ macro, which tells you if you included libstdc++ (similarly, _LIBCPP_VERSION tells you clang's libc++ is used). uclibc++ even seems to expose __UCLIBCXX_MAJOR__ (though it is not exposed after all includes, it seems).

This might actually allow more generic detection: Just see if some C++ header is available, and if it's libstdc++ or libc++ (just for completeness), assume that __cplusplus indicates the standard library version. If it's uclibc++, assume its C++98. If the header is available but none of the above, raise an error (better than subtle breakage due to a misinformed assumption). If the header is not available at all (e.g. on AVR), set the version macro to 0.

[matthijskooijman]

I just updated the PR with the above refactoring. I think I'm pretty happy with the resulting code as it is now.

The commit history is a bit of a mess still, I can try to clean that up if you agree that the current approach is ok?

One more possible improvement I thought of would be to put the initializer_list code into its own initializer_list.h file, that makes it easier to sync that code between ArxContainer and ArxTypeTraits, and then the ARX_TYPE_TRAITS_INITIALIZER_LIST_DEFINED guard can just become a normal file include guard as well? But maybe that's a separate improvement for later?

There is the question of compatibility now, this change will need all users to update their code. In particular with the bundling of libraries you do, this might become somewhat tricky (i.e. if you update ArxTypeTraits but not MsgPack, or the other way around, things will end up breaking). More generally, whenever two copies of your libraries are included in the same project (e.g. bundled in two libraries, or one directly and one bundled) you will not get duplicate definitions because the include guards for these files prevent only one copy to be loaded, but if the versions of both copies differ, you might end up with broken code (when a user assumes they get version x, but they end up with an older or newer but incompatible version). For this, it might make sense to let e.g. ArxTypeTraits.h define a version number macro (possibly as a value for the ARX_TYPE_TRAITS_H include guard), and when the file is not processed because ARX_TYPE_TRAITS_H is already defined, check to see if the version number is exactly the same as the current file, erroring out if not? This enforces that all bundled version of each library are exactly the same (alternatively, you could allow newer versions, but that probably requires proper major.minor versions and only allowing newer versions with the same major, and that means that swapping the include order breaks the build).

[matthijskooijman]

See also https://github.com/hideakitai/MsgPack/pull/5 for the related MsgPack and ArxContainer changes.

[hideakitai]

Thank you so much! I really appreciate it.

One more possible improvement I thought of would be to put the initializer_list code into its own initializer_list.h file

I agree. I want to do that after checking if it works but before merge PR.

There is the question of compatibility now, this change will need all users to update their code. In particular with the bundling of libraries you do

For me, I will change all of my libraries after merging PR. For other users... please change it by themselves... (But we should note about namespace changes in README)

whenever two copies of your libraries are included in the same project (e.g. bundled in two libraries, or one directly and one bundled) you will not get duplicate definitions because the include guards for these files prevent only one copy to be loaded, but if the versions of both copies differ, you might end up with broken code (when a user assumes they get version x, but they end up with an older or newer but incompatible version). For this, it might make sense to let e.g. ArxTypeTraits.h define a version number macro (possibly as a value for the ARX_TYPE_TRAITS_H include guard), and when the file is not processed because ARX_TYPE_TRAITS_H is already defined, check to see if the version number is exactly the same as the current file, erroring out if not?

Agree. I knew I had to do it, but now it's the time...! I think it is ok not to allow newer version (compilable only when the versions are exactly same).

Then, summarize the rest of tasks...

[hideakitai]

The rest of tasks are like this?

• [x] check if ArxTypeTraits #2 works
• [x] check if hideakitai/MsgPack#5 works
• [x] change all my libraries which depends on ArxTypeTrais and ArxContainer
• [x] check if all of them works (WIP yet)
• [x] separate initializer_list.h (maybe it works if it can be compiled so just check compilation)
• [x] fix support ESP8266
• [ ] merge MsgPack
• [x] merge ArxTypeTraits
• [ ] update and release new version of ArxTypeTraits with updates of README and meta data
• [ ] update all of my libraries depending on ArxTypeTraits again and release new version
• [x] update all Arx series to work like ArxTypeTraits (selective compile and arx::arx_std + re-import to std)
• [ ] add version to include guard (same as release version like 0_2_1 ?)
• [ ] (finally apply performance improvement of string for MsgPack)

[hideakitai]

I've checked the ArxTypeTraits #2 and hideakitai/MsgPack#5.

1. I prefer to use stdlibc++ if it's included, so in such caseI want std::enable_if not arx::std::enable_if.
2. I want to shorten the namespace with using namespace arx but it causes namespace conflict(?) with pre-existing std namespace... (not found enable_if in namespace std)

So how about using just arx not arx::std?

It enables to switched between libstdc++ (or ArduinoSTL) and ArxTypeTraits by switching using namespace std and using namespace arx or use both inside of correct namespace (which has the implementation of other libraries)... maybe?

[matthijskooijman]

The rest of tasks are like this?

Yup, looks good,.

I prefer to use stdlibc++ if it's included, so in such caseI want std::enable_if not arx::std::enable_if.

With the current PR, if stdlibc++ is available, it is included and its stuff is exposed under arx::std, so arx::std::enable_if is just an alias for std::enable_if. Is that what you mean?

I want to shorten the namespace with using namespace arx but it causes namespace conflict(?) with pre-existing std namespace... (not found enable_if in namespace std)

Yeah, as I said before, trying to reference things from arx::std under the std name really only works when you can be sure there is no std namespace from the standard library at all, I think. Though maybe namespace std { using namespace arx::std; } could work, to allow referring to everything in arx::std as std, hopefully (but I think so) then conflicts will not arise because the real std stuff takes precedence over the arx::std stuff.

So how about using just arx not arx::std?

I would not do this, because it muddies the water between the "std::" stuff implemented or imported by ArxTypeTraits, and the stuff that it adds to it. Putting everything in arx makes it less clear what is standard stuff and what is not, which I don't like much. Having said that, if you really want to do this, I guess it could be made to work, though.

It enables to switched between libstdc++ (or ArduinoSTL) and ArxTypeTraits by switching using namespace std and using namespace arx

I think you can now do this already with using namespace std or using namespace arx::std?

or use both inside of correct namespace (which has the implementation of other libraries)... maybe?

Note sure what you mean here?

[matthijskooijman]

I want to shorten the namespace with using namespace arx but it causes namespace conflict(?) with pre-existing std namespace...

Ah, I see what you mean there: If you do using namespace arx, then both std and arx::std are imported in the global namespace, so any std:: references can become ambiguous. One way to solve this is to use ::std, like here: https://github.com/hideakitai/ArxTypeTraits/blob/dbe4018617d3f525a4178a4e2ed4282cf829202d/ArxTypeTraits.h#L35-L39

But having to modify all references to std kind of defeats the purpose of the using declaration.

Also, my previous suggestion of adding namespace std { using namespace arx::std; } does not help here, since that just removes the need for referring to everything by arx::std instead of std. Given that there are also other things in the arx namespace, there is a usecase for using namespace arx; regardless of arx::std.

So if we want the arx-std-stuff in its own namespace, but it cannot be arx::std, then maybe it should be arx_std? Or arx::arx_std? Not sure I like either of these much. Let me ponder this for a bit more.

[hideakitai]

I read codes and comments above again and tested, sorry, my understanding was completely wrong (and sorry for my poor reading/writing of English...).

About 1, yes, this is automatically realized by using arx::std::xxxxx. https://github.com/hideakitai/ArxTypeTraits/blob/dbe4018617d3f525a4178a4e2ed4282cf829202d/ArxTypeTraits.h#L35-L39

About 2, this means exactly what you said in the comment above.

About 2 and using namespace arx problem in global namespace, how about just avoiding such undesired usage by making alias like using stdx = arx::std? (Maybe there is no such person except me but) someone use using namespace arx if namespace arx is exposed to library users.

For example, use only stdx::xxxx not arx::std::xxxx in the library examples and README, but implementation is as is (just add alias using stdx = arx::std.

[matthijskooijman]

I just tried adding this to ArxTypeTraits directly:

namespace std {
    using namespace ::arx::arx_std;
}

(note that I also renamed arx::std to arx::arx_std, more on that below)

As suggested before, this makes sure that everything that is defined by arx is again imported back into the std namespace. This Effectively, this means you can just use std:: everywhere and get the standard lib version if it exists, filling in the missing bits with the arx versions. The only thing that does not work like this is e.g. replacing e.g. the ArduinoSTL numeric_limits with the arx-supplied constexpr version. But if you really need that, you can just use arx::arx_std::numeric_limits.

As for the arx::arx_std name, I'm not quite sure about that yet (maybe just arx_std is better?), but given that you probably would not normally need it at all, it might be ok if it's less than pretty.

using namespace arx problem in global namespace, how about just avoiding such undesired usage by making alias like using stdx = arx::std?

How would that help? You'd still have arx::std that conflicts when doing using namespace arx? If we rename that, you could do using stdx = arx::arx_std or something to make it more convenient. But if we do this unconditionally, we might as well declare everything in stdx in the first place? There is no real need to put it inside arx, other than making things more organized?

(Maybe there is no such person except me but) someone use using namespace arx if namespace arx is exposed to library users.

What are you saying here? That using namespace arx might not need to be supported at all? I'd tend to disagree, since it's a natural thing for users to do, and I'd greatly prefer that it would not break, especially not in hard-to-diagnose ways.

[matthijskooijman]

As suggested before, this makes sure that everything that is defined by arx is again imported back into the std namespace. This Effectively, this means you can just use std:: everywhere and get the standard lib version if it exists, filling in the missing bits with the arx versions.

One thing I do not know is how this would work for e.g. adding specializations or extra overloads, but I suspect that's not currently done in ArxTypeTraits, it just adds names that were (maybe) not defined by the standard library.

[matthijskooijman]

I pushed updates to both PRs just now with the above changes. Because everything is available through std:: after all now, I could drop the changes to MsgPack itself entirely, plus the ::std::initializer_list commit for ArxContainer.

[hideakitai]

Yeah, renaming to arx::arx_std and re-importing to std is much better :).

I previously proposed to use arx instead of arx::std.

namespace arx {
    using ::std;
}

using namespace arx;
enable_if<....>

As for the arx::arx_std name, I'm not quite sure about that yet (maybe just arx_std is better?)

You mean this is same as above? Of course current arx::arx_std is also ok, but it's more pretty.

namespace arx_std {
    using ::std;
}

I just want pretty alias for arx::arx_std :). But now, it's not required because arx::arx_std is re-imoprted to std, right?

[hideakitai]

I've checked and worked perfectly... super cool. Thanks so much! :) I will continue to test it on my libraries.

[matthijskooijman]

Ok, #2 is finished, I think. Looking at the other things on your list (updating other libraries etc.), I think it would make most sense if you did those, also because I'm not really using anything else than MsgPack currently. I'd be happy to review stuff, though.

[hideakitai]

Thank you so much for your awesome contribution! Close this issue with merging #2. :) I'll continue to update other libraries including ArxContainer and MsgPack.