Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GzSetCompilerFlags: Fix detection of clang-cl #353

Merged
merged 1 commit into from
May 25, 2023

Conversation

traversaro
Copy link
Contributor

@traversaro traversaro commented May 20, 2023

🦟 Bug fix

Fixes compilation of gz-* projects with clang-cl.

Summary

On Windows, there are two drivers available for the clang compiler:

  • clang, that takes in input GCC-style compiler flags
  • clang-cl, that takes in input MSVC-style compiler flags

When clang-cl is used, it does not make sense to pass to it GCC-style compiler flags. Befor this PR, GCC-style compiler flags were passed to clang-cl, that due to some combination with the existing MSVC flags resulted in the wrong C++ standard version be used when compiling the project, creating an endless list of warnings like:

C:\src\gz-math\include\gz/math/Vector2.hh(34,5): warning: inline namespaces are incompatible with C++98 [-Wc++98-compat]
    inline namespace GZ_MATH_VERSION_NAMESPACE {
    ^
C:\src\gz-math\include\gz/math/Vector2.hh(51,15): warning: 'constexpr' specifier is incompatible with C++98 [-Wc++98-compat]
      public: constexpr Vector2()
              ^
C:\src\gz-math\include\gz/math/Vector2.hh(59,15): warning: 'constexpr' specifier is incompatible with C++98 [-Wc++98-compat]
      public: constexpr Vector2(const T &_x, const T &_y)
              ^
C:\src\gz-math\include\gz/math/Vector2.hh(66,47): warning: defaulted function definitions are incompatible with C++98 [-Wc++98-compat]
      public: Vector2(const Vector2<T> &_v) = default;
                                              ^
C:\src\gz-math\include\gz/math/Vector2.hh(69,28): warning: defaulted function definitions are incompatible with C++98 [-Wc++98-compat]
      public: ~Vector2() = default;
                           ^
C:\src\gz-math\include\gz/math/Vector2.hh(227,55): warning: defaulted function definitions are incompatible with C++98 [-Wc++98-compat]
      public: Vector2 &operator=(const Vector2 &_v) = default;
                                                      ^
C:\src\gz-math\include\gz/math/Vector2.hh(52,13): warning: generalized initializer lists are incompatible with C++98 [-Wc++98-compat]
      : data{0, 0}
            ^
C:\src\gz-math\include\gz/math/Vector2.hh(60,13): warning: generalized initializer lists are incompatible with C++98 [-Wc++98-compat]
      : data{_x, _y}

This PR fixes this by only adding GCC-style compiler flags if clang is used, and not adding them if clang-cl is using. Strictly speaking the variable used to check if clang-cl is used is CMAKE_CXX_COMPILER_FRONTEND_VARIANT, that was only introduced in CMake 3.14 while gz-cmake3 supprots CMake 3.2 . However, if that code is evaluted in CMake <= 3.13, the variable CMAKE_CXX_COMPILER_FRONTEND_VARIANT will be empty and the check:

CMAKE_CXX_COMPILER_FRONTEND_VARIANT STREQUAL "MSVC"

will be false.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Silvio Traversaro <silvio@traversaro.it>
@scpeters scpeters merged commit d618dd7 into gazebosim:gz-cmake3 May 25, 2023
@scpeters
Copy link
Member

thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden 🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants