Skip to content

Capture this explicitly in the InterfaceAPI find callback - #1699

Merged
caguero merged 2 commits into
mainfrom
caguero/fix_deprecated_this_capture
Aug 13, 2026
Merged

Capture this explicitly in the InterfaceAPI find callback#1699
caguero merged 2 commits into
mainfrom
caguero/fix_deprecated_this_capture

Conversation

@caguero

@caguero caguero commented Aug 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦟 Bug fix

Part of gazebosim/gz-cmake#580

Summary

[=] implicitly captured this, which is deprecated in C++20. The lambda only uses this->modelDir, so the capture is now explicit. No behavior change, and it is valid C++17, so it can land before any standard bump.

Before:

test/integration/interface_api.cc:151:9: warning: implicit capture of ‘this’ via ‘[=]’ is deprecated in C++20 [-Wdeprecated]

Backport Policy

  • This is safe to backport to the following versions:
    • Jetty
    • Ionic
    • Harmonic
    • Fortress
  • This should not be backported
  • I am not sure
  • Other (fill in yourself)

Checklist

  • Signed all commits for DCO
  • Added a screen capture or video to the PR description that demonstrates the fix (as needed)
  • 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)
  • Updated Bazel files (if adding new files). Created an issue otherwise.
  • 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 "Assisted-by" to your commits. (See this policy for more info.)

Assisted-by: Claude Opus 5

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by and Generated-by messages.

@caguero
Capture this explicitly in the InterfaceAPI find callback
9f3f70b 
Assisted-by: Claude Opus 5
Signed-off-by: Carlos Aguero <caguero@honurobotics.com>
@caguero
caguero requested review from azeey and scpeters as code owners August 12, 2026 18:02
@caguero caguero mentioned this pull request Aug 12, 2026
Open
28 tasks
luca-della-vedova
luca-della-vedova approved these changes Aug 13, 2026
View reviewed changes
Comment thread test/integration/interface_api.cc Outdated
[=](const std::string &_file)
[this](const std::string &_file)
{
return sdf::filesystem::append(modelDir, _file);

@luca-della-vedova luca-della-vedova Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we change this to this->modelDir as well? So it's clear at a glance why this is necessary here.

@caguero caguero Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, done in c2f24b8. It also matches the style used elsewhere in this fixture.

@caguero caguero Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes! Added in c2f24b8

@github-project-automation github-project-automation Bot moved this from Inbox to In review in Core development Aug 13, 2026
@caguero
Use this->modelDir inside the callback for clarity
c2f24b8 
Assisted-by: Claude Opus 5
Signed-off-by: Carlos Aguero <caguero@honurobotics.com>
@caguero
caguero merged commit 36536d8 into main Aug 13, 2026
15 checks passed
@caguero
caguero deleted the caguero/fix_deprecated_this_capture branch August 13, 2026 13:42
@github-project-automation github-project-automation Bot moved this from In review to Done in Core development Aug 13, 2026
@scpeters

scpeters commented Aug 13, 2026

Copy link
Copy Markdown
Member

is there a reason not to backport to Jetty?

@caguero

caguero commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

is there a reason not to backport to Jetty?

We could if we think we'd eventually use C++20 with Jetty (which perhaps we do it when compiling it as a vendor package). It's only fixing a warning when using C++20.

@scpeters

scpeters commented Aug 13, 2026

Copy link
Copy Markdown
Member

is there a reason not to backport to Jetty?

We could if we think we'd eventually use C++20 with Jetty (which perhaps we do it when compiling it as a vendor package). It's only fixing a warning when using C++20.

this is exactly my thinking, since the Jetty vendor packages are using c++20 with Lyrical

at any rate, it seems low risk to backport

@caguero

caguero commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@Mergifyio backport sdf16

@mergify

mergify Bot commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

backport sdf16

✅ Backports have been created

Details

@mergify mergify Bot mentioned this pull request Aug 13, 2026
Merged
19 tasks
caguero added a commit that referenced this pull request Aug 13, 2026
@caguero
Capture this explicitly in the InterfaceAPI find callback (#1699)
e7498ab 
Assisted-by: Claude Opus 5
Signed-off-by: Carlos Agüero <caguero@honurobotics.com>
(cherry picked from commit 36536d8)
scpeters pushed a commit that referenced this pull request Aug 13, 2026
@caguero @scpeters
Capture this explicitly in the InterfaceAPI find callback (#1699)
9a26f90 
Assisted-by: Claude Opus 5
Signed-off-by: Carlos Agüero <caguero@honurobotics.com>
(cherry picked from commit 36536d8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@luca-della-vedova luca-della-vedova luca-della-vedova approved these changes
@azeey azeey Awaiting requested review from azeey azeey is a code owner
@scpeters scpeters Awaiting requested review from scpeters scpeters is a code owner

Assignees

No one assigned

Labels

🎡 rotary

Projects

Core development
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@caguero @scpeters @luca-della-vedova @azeey