Skip to content

Use IWYU pragma export to avoid clangd warnings for generated headers #902

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
Open
wants to merge 1 commit into
base: rolling
Choose a base branch
from
Open

Use IWYU pragma export to avoid clangd warnings for generated headers #902

wants to merge 1 commit into from

Conversation

adam-leeper-ai
Copy link

Description

Please tell me if the base branch is wrong.

This adds // IWYU pragma: export to the lines in idl.hpp.em that include the generated implementation files.

Is this user-facing behavior change?

It does not affect user-facing behavior for code compilation or runtime.

It improves user-facing behavior for tools like clangd and IWYU:

  • Without these lines, clangd in editors like VSCode/Cursor will complain (see image) that any generated headers you include are not used directly, and tools like IWYU try to remove the main include and instead directly include one of the implementation headers.
image

Did you use Generative AI?

No.

@adam-leeper-ai
Use IWYU pragma export to avoid clangd warnings for generated headers
b301c2a 
Without these lines, clangd in editors like VSCode/Cursor complains that any generated headers are not used directly, and tools like IWYU try to remove the main include and instead directly include one of the implementation headers.

Signed-off-by: Adam Leeper <[email protected]>
fujitatomoya
fujitatomoya approved these changes Sep 18, 2025
View reviewed changes
Copy link
Contributor

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm.

this tells IWYU to treat the marked header as part of the public API for any file including the current header, making dependency management easier and reducing redundant includes. besides, i do not think there is any downside for this.

i would like to have another review before merge.

@fujitatomoya
Copy link
Contributor

Pulls: #902
Gist: https://gist.githubusercontent.com/fujitatomoya/1b4882a1af76188d4a39c97cf1390273/raw/8720e229c713590ecce5c662a6ef26dcabd053b9/ros2.repos
BUILD args: --packages-above-and-dependencies rosidl_generator_cpp
TEST args: --packages-above rosidl_generator_cpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/17026

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@adam-leeper-ai adam-leeper-ai mentioned this pull request Sep 19, 2025
Merged
@mjcarroll
Copy link
Member

Looks spurious, rebuilding:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@mjcarroll
Copy link
Member

Oh, it actually looks like those tests aren't really failing, they were just skipped? I'm assuming because some required package isn't available. Was this intentional @fujitatomoya ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fujitatomoya fujitatomoya fujitatomoya approved these changes

Assignees

@mjcarroll mjcarroll

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@adam-leeper-ai @fujitatomoya @mjcarroll