Skip to content

Fix: silence backward switch default warning #3203

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.

Sign up for GitHub

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

Jump to bottom
Merged
azeey merged 10 commits into from
Dec 1, 2025
Merged

Fix: silence backward switch default warning #3203

azeey merged 10 commits into from
Dec 1, 2025

Conversation

@miguelgonrod
Copy link

@miguelgonrod miguelgonrod commented Nov 25, 2025

🦟 Bug fix

Fixes #3202

Summary

As the switch are using dwarf_tags to handle the case, I think it could dissable the compilation -Wno-switch-default as it was already evaluating the cases, so no need of a deefault clause

To replicate this behaviour follow this steps:

  1. Go to https://build.osrfoundation.org/job/gz_sim-ci-gz-sim8-jammy-amd64 or https://build.osrfoundation.org/job/gz_sim-ci-gz-sim8-noble-amd64
  2. Build the job
  3. See the gcc warning

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
  • Was GenAI used to generate this PR? If so, make sure to add "Generated-by" to your commits. (See this policy for more info.)

@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Nov 25, 2025
@arjo129
Copy link
Contributor

arjo129 commented Nov 26, 2025 •
edited
Loading

Im wondering if theres some way to tell jenkins to skip gcc warning checks on this directory, since we vendor this from an upstream repository. If you think thats not possible, Im open to approving these changes.

arjo129
arjo129 reviewed Nov 26, 2025
View reviewed changes
test/CMakeLists.txt Outdated
add_subdirectory(plugins)
add_subdirectory(regression)
add_subdirectory(worlds)
add_subdirectory(worlds)
Copy link
Contributor

@arjo129 arjo129 Nov 26, 2025

Choose a reason for hiding this comment

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

Knit: Unrelated change. Please revert.

Copy link
Author

@miguelgonrod miguelgonrod Nov 26, 2025

Choose a reason for hiding this comment

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

Ready, I reverted that fake change. I don't know what happened there 🫤

@miguelgonrod miguelgonrod marked this pull request as ready for review November 26, 2025 15:25
@azeey
Copy link
Contributor

azeey commented Nov 26, 2025

I wonder if we can use INTERFACE_SYSTEM_INCLUDE_DIRECTORIES (https://cmake.org/cmake/help/latest/prop_tgt/INTERFACE_SYSTEM_INCLUDE_DIRECTORIES.html#prop_tgt:INTERFACE_SYSTEM_INCLUDE_DIRECTORIES) to mark the backward directory as a system directory, which suppress warnings from it.

I'd rather avoid making changes to the vendored package if at all possible so that we can easily update it later on from upstream.

@miguelgonrod miguelgonrod marked this pull request as draft November 26, 2025 16:28
@miguelgonrod miguelgonrod marked this pull request as ready for review November 26, 2025 18:15
@miguelgonrod miguelgonrod marked this pull request as draft November 26, 2025 18:38
@miguelgonrod
Copy link
Author

miguelgonrod commented Nov 26, 2025

I'm trying to find the reason, because using SYSTEM worked for every build except gz_sim-ci-pr_any-jammy-amd64

@miguelgonrod miguelgonrod marked this pull request as ready for review November 26, 2025 20:37
@miguelgonrod
Copy link
Author

miguelgonrod commented Nov 26, 2025

Ready, gz_sim-ci-pr_any-jammy-amd64 test failing isn't related to this, it is related to another issue I oppened. This is what you meant?

@github-project-automation github-project-automation bot moved this from Inbox to In review in Core development Nov 27, 2025
@arjo129 arjo129 enabled auto-merge (squash) November 27, 2025 07:31
@miguelgonrod @Crola1702
Fix: silence backward switch default warning
1e49e92 
Signed-off-by: miguelgonrod <[email protected]>
@miguelgonrod @Crola1702
Fix: moved the compilation flag to backward-cpp cmake
9b37e4d 
Signed-off-by: miguelgonrod <[email protected]>
@miguelgonrod @Crola1702
Fix: removed the silence and added a default break
ee52318 
Signed-off-by: miguelgonrod <[email protected]>
@miguelgonrod @Crola1702
Fix: added brackets to avoid jumping to case label
64a467f 
Signed-off-by: miguelgonrod <[email protected]>
@miguelgonrod @Crola1702
Reverted fake change in cmakelist
4c1328e 
Signed-off-by: miguelgonrod <[email protected]>
@miguelgonrod @Crola1702
Fix: reverted switch and added path to system directory
a586b91 
Signed-off-by: miguelgonrod <[email protected]>
@miguelgonrod @Crola1702
Added snnipet to make SYSTEM backward_vendor
3573b5d 
Signed-off-by: miguelgonrod <[email protected]>
@miguelgonrod @Crola1702
fix aliased error for set_target_properties
fb128bf 
Signed-off-by: miguelgonrod <[email protected]>
@miguelgonrod @Crola1702
Added a flag supression for switch default
01f935e 
Signed-off-by: miguelgonrod <[email protected]>
@miguelgonrod @Crola1702
Removed some log noise
a3d5d01 
Signed-off-by: miguelgonrod <[email protected]>
@Crola1702 Crola1702 force-pushed the fix/backward-switch-warning branch from 0823896 to a3d5d01 Compare December 1, 2025 14:24
@azeey
Copy link
Contributor

azeey commented Dec 1, 2025

@scpeters to take a look at hanging homebrew arm64 job.

@azeey azeey disabled auto-merge December 1, 2025 19:07
@azeey azeey merged commit 5318644 into gazebosim:gz-sim8 Dec 1, 2025
7 of 9 checks passed
@github-project-automation github-project-automation bot moved this from In review to Done in Core development Dec 1, 2025
@scpeters
Copy link
Member

scpeters commented Dec 1, 2025

@scpeters to take a look at hanging homebrew arm64 job.

it's not easy to see now, but the gz_sim-ci-pr_any-homebrew-arm64 job was listed as blocking the merge, since it is now a required CI job, but a build was never started as it was disabled. The arm64 PR jobs were added in gazebo-tooling/release-tools#1412 and are generated by the https://build.osrfoundation.org/job/_dsl_gazebo_libs/ jenkins job. I reran this job and believe it should be fixed now:

@miguelgonrod
Copy link
Author

miguelgonrod commented Dec 2, 2025

https://github.com/Mergifyio backport gz-sim9

@mergify
Copy link
Contributor

mergify bot commented Dec 2, 2025

backport gz-sim9

❌ Command disallowed due to command restrictions in the Mergify configuration.

  • sender-permission >= write

@Crola1702
Copy link
Contributor

Crola1702 commented Dec 2, 2025

https://github.com/Mergifyio backport gz-sim9

@mergify
Copy link
Contributor

mergify bot commented Dec 2, 2025 •
edited
Loading

backport gz-sim9

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Dec 2, 2025
@miguelgonrod @mergify
Fix: silence backward switch default warning (#3203)
4118e4f 
Signed-off-by: miguelgonrod <[email protected]>
(cherry picked from commit 5318644)

# Conflicts:
#	test/backward_vendor/backward-cpp/CMakeLists.txt
@mergify mergify bot mentioned this pull request Dec 2, 2025
Merged
9 tasks
scpeters pushed a commit that referenced this pull request Dec 4, 2025
@miguelgonrod @scpeters
Fix: silence backward switch default warning (#3203)
1db1c8a 
Signed-off-by: miguelgonrod <[email protected]>
(cherry picked from commit 5318644)
arjo129 pushed a commit that referenced this pull request Dec 4, 2025
@mergify @miguelgonrod
Fix: silence backward switch default warning (#3203) (#3206)
1dcbdf7 
(cherry picked from commit 5318644)

Signed-off-by: miguelgonrod <[email protected]>
Co-authored-by: Miguel Angel Gonzalez Rodriguez <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arjo129 arjo129 arjo129 approved these changes

Assignees

No one assigned

Labels

🎵 harmonic Gazebo Harmonic

Projects

Core development
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@miguelgonrod @arjo129 @azeey @scpeters @Crola1702