Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

warning fix #9681

Merged
merged 1 commit into from
Nov 19, 2020
Merged

warning fix #9681

merged 1 commit into from
Nov 19, 2020

Conversation

huangminghuang
Copy link
Contributor

@huangminghuang huangminghuang commented Nov 17, 2020

Change Description

  1. add "-Wall -Wsign-compare -Wrange-loop-analysis" compiler flag to clang build
  2. fix compiler warnings.

Change Type

Select ONE

  • Documentation
  • Stability bug fix
  • Other
  • Other - special case

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

@@ -45,6 +45,7 @@ elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang" OR "${CMAKE_CXX_COMPILER_ID}
if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 5.0)
message(FATAL_ERROR "Clang version must be at least 5.0!")
endif()
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wsign-compare -Wrange-loop-analysis")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you mean to add -Wall? (wasn't in PR desc)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After cleaning up these warnings, what would be the thoughts on turning any new warnings into errors -Werror?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-Werror by default is very user hostile, I'd hope it'd be an option defaulted to off

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(and potentially enabled in CI)

@@ -98,6 +99,7 @@ else()
set(no_whole_archive_flag "--no-whole-archive")
endif()

set(Boost_USE_MULTITHREADED ON)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have both single/multithread boost libraries installed, it would choose the multithread ones. I got a lot cmake warning messages without it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value of Boost_USE_MULTITHREADED is documented as ON.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah okay, Boost_USE_MULTITHREADED is default ON when using cmake's module, but

The default is to use either.

for Boost's module.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants