search

ReactiveX / RxCpp

Reactive Extensions for C++
Apache License 2.0
3.03k stars 390 forks source link

Include headers directory as SYSTEM path #580

Closed wirew0rm closed 2 years ago

wirew0rm commented 2 years ago

When using the rxcpp target from a cmake subproject (using fetch_content or add_subdirectory) it is not possible to use -Werror or stricter warnings in general due to warnings from the rxcpp headers being reported. This change sets the SYSTEM flag only if the variable RXCPP_INCLUDE_AS_SYSTEM_HEADERS is set to true. In the end this passes the include directories to gcc using the -i flag instead of the -I flag and warnings from the rxcpp headers are not reported in projects including it. SYSTEM has to be set conditonally because that would also suppress valid warnings for the rxcpp tests and examples.

See https://www.foonathan.net/2018/10/cmake-warnings/ for a nice writeup of handling warings for header only cmake projects.

wirew0rm commented 2 years ago

After some discussion on similar issues in other projects and with some collegues, I came to the conclusion that the "-Isystem" flag has other side effects, like changing the search order and also possibly might hide warnings caused by wrong use of the library. Instead I now wrap the rxcpp include to selectively disable warnings:

#pragma gcc diagnostic push
#pragma GCC diagnostic ignored "-Wshadow"
#pragma GCC diagnostic ignored "-Wuseless-cast"
#pragma GCC diagnostic ignored "-Wold-style-cast"
#pragma GCC diagnostic ignored "-Wsign-conversion"
#include <rxcpp/rx.hpp>
#pragma GCC diagnostic pop

Ideally the cause of the warnings could be fixed in rxcpp, or if they are false positives ignored locally. I'll try to get around to providing a PR for that if I get to it and close this PR.