andysworkshop / stm32plus

The C++ library for the STM32 F0, F100, F103, F107 and F4 microcontrollers
http://www.andybrown.me.uk
Other
745 stars 224 forks source link

Out of date vendored STL #221

Open mikepurvis opened 4 years ago

mikepurvis commented 4 years ago

There are another round of fixes needed for the out of date STL shipped by stm32plus here: https://github.com/andysworkshop/stm32plus/tree/master/lib/include/stl

gnuarm 7-2018-q2-6 complains, for example:

[ 92%] Building CXX object CMakeFiles/[...]/src/hardware/system_hardware.cpp.obj
In file included from [...]/include/hardware/system_hardware_revision.h:43:0,
                 from [...]/src/hardware/system_hardware_revision.cpp:12:
/usr/local/arm-none-eabi/include/stm32plus-040100/stl/string: In instantiation of 'std::basic_string<_CharT, _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::append(_InputIter, _InputIter) [with _InputIter = const char*; _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]':
/usr/local/arm-none-eabi/include/stm32plus-040100/stl/string:1317:11:   required from 'std::basic_string<_CharT, _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::assign(const _CharT*, const _CharT*) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]'
/usr/local/arm-none-eabi/include/stm32plus-040100/stl/string:366:13:   required from 'std::basic_string<_CharT, _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::operator=(const std::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]'
[...]/src/hardware/system_hardware_revision.cpp:44:25:   required from here
/usr/local/arm-none-eabi/include/stm32plus-040100/stl/string:570:30: error: no matching function for call to 'std::basic_string<char>::_M_append_dispatch(const char*&, const char*&, _Integral)'
     return _M_append_dispatch(__first, __last, _Integral());

However, other than slist (renamed to forward_list), I believe the library mostly works with a modern STL like the one in the libstdc++-arm-none-eabi-newlib package (version 15:7-2018-q2-5+12 in Focal). Would we entertain stripping out the vendored STL in favour of recommending an external one?


Looks like some effort was made to support external STL implementations with be510ee330ee3a3553ebb303c89da3335aa839df, though there are other slist usages (especially in net) which need to be similarly patched.

In all, the changes needed for me (since newlib does indeed ship an ext/slist header) were:

I think there are basically three main paths forward here:

  1. Support newlib only (probably as of 6.3.1, the version in Ubuntu Bionic), excising lib/include/stl from the repo. At some point, port the internal slist uses over to use the standard forward_list.
  2. Improve on the current dual-support story, probably with an internal compatibility header for slist to smooth over the type naming and import difference (and maybe a better ifdef name than EXT_SLIST).
  3. Don't explicitly support newlib, and continue to patch the vendored SGI STL to work with newer compilers. Main downsides are just the ongoing whack-a-mole + potential compat issues in projects that use other libs linking to the standard toolchain.
mikepurvis commented 4 years ago

Here's the diff that corresponds to option 1:

https://github.com/andysworkshop/stm32plus/compare/master...mikepurvis:no-stl

This is what we'll be moving forward with for now on our end, but I'm open to the discussion resolving in whatever way it does.