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

tree wide: add IWYU pragma: export where needed #20569

Merged
merged 1 commit into from
Apr 14, 2024

Conversation

maribu
Copy link
Member

@maribu maribu commented Apr 14, 2024

Contribution description

We occasionally have some public foo.h header that includes a private foo_arch.h header. Users are expected to include the foo.h header and not the foo_arch.h. However, clangd will claim that the #include of foo.h is unused if only functions / macros/ types / ... from foor_arch.h is used and nothing from foo.h.

This adds the IWYU pragma: export comment to the include of foo_arch.h in foo.h, so that clangd treats functions / macros / types provided by foo_arch.h as if they were instead provided by foo.h, which fixes the false positives.

Testing procedure

In master, the include of irq.h is often incorrectly labeled as unneeded by clangd (e.g. in VS code). This should get rid of these false positives.

Issues/PRs references

See also: #20570

We occasionally have some public `foo.h` header that includes a private
`foo_arch.h` header. Users are expected to include the `foo.h` header
and not the `foo_arch.h`. However, clangd will claim that the `#include`
of `foo.h` is unused if only functions / macros/ types / ... from
`foor_arch.h` is used and nothing from `foo.h`.

This adds the `IWYU pragma: export` comment to the include of
`foo_arch.h` in `foo.h`, so that clangd treats functions / macros /
types provided by `foo_arch.h` as if they were instead provided by
`foo.h`, which fixes the false positives.
@maribu maribu added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 14, 2024
@github-actions github-actions bot added Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: drivers Area: Device drivers Area: sys Area: System labels Apr 14, 2024
@riot-ci
Copy link

riot-ci commented Apr 14, 2024

Murdock results

✔️ PASSED

cf6fa4e tree wide: add IWYU pragma: export where needed

Success Failures Total Runtime
1 0 1 55s

Artifacts

Copy link
Member

@chrysn chrysn left a comment

Choose a reason for hiding this comment

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

I dislike the way this adds pragmas through comments, but that's on https://include-what-you-use.org/, and their tooling doesn't offer a more #pragma-tic way of annotation.

I trust your testing, and on the whole, this looks like a strict improvement. It'll need tooling to not bit-rot, but that shouldn't stop us from fixing the things that can be fixed now.

@maribu maribu added this pull request to the merge queue Apr 14, 2024
Merged via the queue into RIOT-OS:master with commit 3c3c5c2 Apr 14, 2024
29 checks passed
@maribu maribu deleted the tree-wide/IWYU-pragmas branch April 15, 2024 14:37
@mguetschow mguetschow added this to the Release 2024.07 milestone Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: core Area: RIOT kernel. Handle PRs marked with this with care! Area: drivers Area: Device drivers Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants