tier4 / AutowareArchitectureProposal.proj

This is the source code of the feasibility study for Autoware architecture proposal.
Apache License 2.0
652 stars 238 forks source link

Move to C++17 #109

Closed esteve closed 2 years ago

esteve commented 3 years ago

As noted in https://github.com/tier4/scenario_runner.iv.universe/pull/7#discussion_r517464547, requiring C++17 may help us get rid of some dependencies (namely Boost). Let's evaluate the feasibility of using C++17 after the ROS 2 port is done.

kenji-miyake commented 3 years ago

I know C++17 can remove a lot of boost dependencies and I'd love to use it, but I have one concern. Can Autoware.Auto accept C++17? If not, I'm worried about we'll need to re-replace C++17 with boost when we contribute to Autoware.Auto.

mitsudome-r commented 3 years ago

@dejanpan Do you have any comments?

dejanpan commented 3 years ago

@esteve @kenji-miyake @mitsudome-r I provided a slightly longer answer here: https://github.com/eclipse/iceoryx/issues/220#issuecomment-724952814.

Since Adaptive Autosar Guidelines are not used in Autoware.Auto, I think that we could switch to C++17.

There are however 2 things that I am still uncertain about:

  1. https://stackoverflow.com/a/49119902:
    1. Where you have problems is if you link together objects compiled with different versions of GCC and you have used unstable features from a new C++ standard before GCC's support for that standard is complete.
  2. and the other question is - do all the static code analysers and linter tools that we use from ROS 2 understand C++17?

I'd appreciate your comments.

mitsudome-r commented 3 years ago

@esteve Could you comment about the two concerns that @dejanpan pointed out?

esteve commented 3 years ago

@dejanpan @mitsudome-r if we agree to allow C++17 in our code, then we should work on https://github.com/tier4/AutowareArchitectureProposal/issues/106 to make sure that we don't use any unstable feature to avoid any ABI issues. It shouldn't be a problem, because we ourselves don't use anything from std::experimental anyway. However, any dependency that in fact uses an experimental feature may be affected, but just need to ensure that we're using the same compiler as the one used for building the ROS 2 binary packages.

I looked into the tools we use:

dejanpan commented 3 years ago

@esteve thanks and works for me.

Now I do want to make also 1 thing clear: we agreed with T4 to port https://github.com/tier4/AutowareArchitectureProposal to ROS 2 as fast as possible and to leave the code as identical as possible as it was in ROS 1 (so no tests, design docs, documentation, refactoring). However as soon as we will finish the simulation packages, we will stop the work here and move to a proper porting in Autoware.Auto. That is also where I'd switch to C++17 and not in https://github.com/tier4/AutowareArchitectureProposal.

@kfunaoka @mitsudome-r