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

depends: patch boost to ignore -Wnonnull new gcc 11 warnings #607

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

DeckerSU
Copy link

this patch fixes errors like:

include/boost/concept/detail/general.hpp:39:47: warning: 'this' pointer null [-Wnonnull]
   39 |     static void failed() { ((Model*)0)->~Model(); }

during build with gcc 11.x, more details can be found here:

this patch fixes errors like:

```
include/boost/concept/detail/general.hpp:39:47: warning: 'this' pointer null [-Wnonnull]
   39 |     static void failed() { ((Model*)0)->~Model(); }
```

during build with gcc 11.x, more details can be found here:

- boostorg/concept_check#27
- boostorg/concept_check#28

# Conflicts:
#	depends/packages/boost.mk
@DeckerSU DeckerSU requested review from Alrighttt and ca333 November 16, 2023 02:12
@TheComputerGenie
Copy link

TheComputerGenie commented Nov 16, 2023

Wouldn't adding CPPFLAGS='-g -Wno-nonnull' to the configure line have the same effect? One could go a step further and add -Wno-deprecated also (if just removing the prints is the goal).

@DeckerSU
Copy link
Author

Wouldn't adding CPPFLAGS='-g -Wno-nonnull' to the configure line have the same effect? One could go a step further and add -Wno-deprecated also (if just removing the prints is the goal).

No, the effect will not be the same. There is a difference in the scope in which -Wno-nonnull is applied. If you place it in CPPFLAGS, it will apply to all compilation units in the project, not just the boost libraries. This is not the behavior we want. We only want to apply this argument to the boost compilation and specifically to concept_check. Additionally, the developers of boost have included this fix in their own repository, so there is no reason not to use it in the bundled version of boost in the Komodo repository.

@TheComputerGenie
Copy link

a) BOOST_CPPFLAGS is a thing, so it could be added there if the blanket method is "too broad" given that there seems to be no desire to fix non-false-positives
b) if the argument is "well boost devs did x" then the next logical step to that argument is "Why not update to a more current boost (i.e.,1.83.0) that has many more patches applied and is 'better' in many aspects?"

Obviously, the counter to b) is that some actual edits are needed, but the counter to the counter is that eventually the current will be unsupported and will need updating anyway.

@DeckerSU
Copy link
Author

@TheComputerGenie You're correct, using BOOST_CPPFLAGS could also be a solution in this instance. However, we must consider the variety of compilers and versions. The patch we've applied specifically targets gcc > 11.x only, whereas using BOOST_CPPFLAGS in build.sh would affect all compilers and versions.

As for the version of Boost we're using, we've found that 1.72.0 is thoroughly tested and suitable for our project. We haven't encountered any unexpected issues or compatibility problems with our supported platforms. However, using a newer version could potentially lead to such issues. For instance, I've heard that the newer Boost decimal float libraries can cause problems on VRSC, among other things.

Regardless, we appreciate your input. For your own builds, you're certainly welcome to use BOOST_CPPFLAGS and experiment with newer versions of Boost. However, for our production build, the applied patch seems to be the preferred method at this time.

@ca333 ca333 merged commit b5976fe into dev Nov 17, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants