-
Notifications
You must be signed in to change notification settings - Fork 401
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
iox-#2011 Silence some warnings when building with GCC 13. #2296
iox-#2011 Silence some warnings when building with GCC 13. #2296
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release_2.0 #2296 +/- ##
===============================================
+ Coverage 78.46% 78.49% +0.02%
===============================================
Files 370 370
Lines 14217 14217
Branches 2060 2060
===============================================
+ Hits 11156 11160 +4
+ Misses 2377 2372 -5
- Partials 684 685 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@clalancette we need to release v3.0 soonish. All of this is already fixed on the main branch. The clang-tidy check might also bite us here. We might need to ignore it for this branch since the code was not yet fixed and it seems we also did not make it run with all files. Can you add |
@clalancette oh, you already noticed to use #2011 as reference issue. Please adjust your commit message accordingly. |
It looks like it is already there on the release_2.0 branch: https://github.com/eclipse-iceoryx/iceoryx/blob/release_2.0/iceoryx_hoofs/include/iceoryx_hoofs/internal/cxx/variant.inl#L21
My commit message already has "iox-#2011" as the first line of the commit message, but let me know if I need to change it to something else. |
Oh, I confused the files. It seems the
That's weird, it does not show up when I look at the commit. It says |
033ae7e
to
c671fbc
Compare
You are right. I fixed it locally, but I forgot to push it. Pushed now. |
😄 classic Can you add What date would you prefer for a release? |
… 13. GCC 13 (which is in Ubuntu 24.04) introduced a number of false positive warnings when using -Warray-bounds and -Wstring-op; see: * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114758 * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111118 * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110807 * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110498 In my testing, these only show up when building with optimizations, i.e. CMAKE_BUILD_TYPE=RelWithDebInfo. This commit silences those warnings across the two functions that cause the problem, and make the build completely clean. Signed-off-by: Chris Lalancette <[email protected]>
c671fbc
to
434ee45
Compare
Ah, great. I've now updated and pushed that in 434ee45. 🤞
It is not a huge rush for us to get a release with this, but in the next month or so would be great. |
Let's see, maybe until then we find the time for the v3.0 release and also bring it to the ROS world. |
Thank you! |
GCC 13 (which is in Ubuntu 24.04) introduced a number of false positive warnings when using -Warray-bounds and -Wstring-op; see:
In my testing, these only show up when building with optimizations, i.e. CMAKE_BUILD_TYPE=RelWithDebInfo.
This commit silences those warnings across the two functions that cause the problem, and make the build completely clean.
Notes for Reviewer
This should fix the yellow builds in e.g. https://ci.ros2.org/view/packaging/job/packaging_linux/3451/
Note that this targets the
release_2.0
branch, since that is what ROS 2 is currently using. If you'd like me to target another branch and then backport, I'm happy to do that instead.Pre-Review Checklist for the PR Author
iox-123-this-is-a-branch
)iox-#123 commit text
)task-list-completed
)Checklist for the PR Reviewer
iceoryx_hoofs
have been added to./clang-tidy-diff-scans.txt
Post-review Checklist for the PR Author
References