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

Fix inline local includes edge cases #290

Merged
merged 7 commits into from
Jul 18, 2024
Merged

Conversation

loostrum
Copy link
Member

Description

  • #include must be the start of a line
  • the inlined included is placed in the same line as the original #include

Related issues:

This fixes cases such as

#ifdef __HIP__
//include "disabled_header.h"
#include "some_header.h"
#endif

Before this PR, the commented out include would be inlined, and both inlined headers would be placed at the top of the file.

Instructions to review the pull request

  • Check that CHANGELOG.md has been updated if necessary

- #include must be the start of a line
- the inlined included is placed in the same line as the original
  #include
Copy link
Contributor

@csbnw csbnw left a comment

Choose a reason for hiding this comment

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

Nice fix! I added a small suggestion to document the new code.

cmake/cudawrappers-helper.cmake Show resolved Hide resolved
cmake/cudawrappers-helper.cmake Show resolved Hide resolved
cmake/cudawrappers-helper.cmake Show resolved Hide resolved
@csbnw
Copy link
Contributor

csbnw commented Jul 18, 2024

Could you update the CHANGELOG.md, e.g. add something like "Made inlining of includes more robust" to the changes.

@loostrum loostrum force-pushed the fix-inline-local-includes branch from 9b1bf44 to 172f42a Compare July 18, 2024 11:14
@loostrum loostrum merged commit 499cc57 into main Jul 18, 2024
7 checks passed
@loostrum loostrum deleted the fix-inline-local-includes branch July 18, 2024 11:20
matmanc pushed a commit that referenced this pull request Jan 20, 2025
* Changes to incline local includes:
- #include must be the start of a line
- the inlined included is placed in the same line as the original
  #include

* Update cmake/cudawrappers-helper.cmake

Co-authored-by: Bram Veenboer <[email protected]>

* Update cmake/cudawrappers-helper.cmake

Co-authored-by: Bram Veenboer <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update cmake/cudawrappers-helper.cmake

Co-authored-by: Bram Veenboer <[email protected]>

* Add updates to inline_local_includes to CHANGELOG

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: Bram Veenboer <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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.

2 participants