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

Add IWYU export pragmas to protect internal headers #756

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

adzenith
Copy link
Contributor

@adzenith adzenith commented Oct 8, 2024

We use Include What You Use to manage #includes across a large codebase. IWYU automatically adds and removes #include statements in each source file to match what symbols are being used in that file. By default it tries to directly include internal nanobind headers when it detects symbols from those files.

This PR adds export pragmas to tell IWYU basically the same thing as the comment on line 38 tells humans: that nanobind.h itself exports all of the internal symbols, and that the transitive headers should not be directly included.

We've been carrying this patch for a while and it's been working great for us.

Thanks for making nanobind!

@wojdyr
Copy link
Contributor

wojdyr commented Oct 8, 2024

These pragmas are also respected by Include Cleaner from clangd.

Recently, I've been evaluating Zed and got warnings about headers without installing or configuring anything. Apparently, the trend in code editors is to come with batteries included, and for C++ on Linux that's the clangd language server.

@wjakob
Copy link
Owner

wjakob commented Oct 8, 2024

It looks kind of ugly :-( . Does // IWYU pragma: begin_exports also work?

@adzenith
Copy link
Contributor Author

adzenith commented Oct 8, 2024

Yeah, sounds great, lemme change it real quick.

Include What You Use tries to add the internal nanobind headers to files using nanobind.h. Tell it that it should rely on nanobind.h to export all of the internal symbols using the `export` pragma.
https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md#iwyu-pragma-export
@wjakob wjakob merged commit 95ae0a2 into wjakob:master Oct 16, 2024
31 checks passed
@adzenith adzenith deleted the iwyu-pragmas branch October 16, 2024 10:42
@adzenith
Copy link
Contributor Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants