Skip to content

[Infra] The Great VPR Pragma PR #3077

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AlexandreSinger
Copy link
Contributor

VPR is moving to a style that uses "#pragma once" instead of header gaurds. These are less error prone and may be slightly more performant.

Converted all of the header gaurds in VPR into pragma once's.

Also moved all pragma onces to the top of all header files to maintain a consistent style. It is a good idea to have them as the very first line in all header files.

While going through all header files, cleaned up any extra header includes which were including things they did not need.

@AlexandreSinger AlexandreSinger changed the title [Infra] The Big VPR Pragma Commit [Infra] The Great VPR Pragma PR May 22, 2025
@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code labels May 22, 2025
@AlexandreSinger
Copy link
Contributor Author

@soheilshahrouz Sorry for the larger PR. This PR moves all include files to use pragma once. Please review when you have a moment.

Copy link
Contributor

@soheilshahrouz soheilshahrouz left a comment

Choose a reason for hiding this comment

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

Thanks Alex.

I left a few comments aboud inlucding header files from catch2. I think because catch2 is a submodule in this project, we should include its header files with double double quotation marks.

@AlexandreSinger
Copy link
Contributor Author

@soheilshahrouz Thank you so much for your review. I would agree that header files that are included in the project should use double quotations, however library headers should use brackets.

In this case we should be using brackets since VTR actually includes Catch2 as system headers:

# Some catch2 headers generate warnings, so treat them as system headers to suppress warnings
target_include_directories(Catch2
SYSTEM
PUBLIC
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/libcatch2/src>
$<INSTALL_INTERFACE:include>
)

I am ok with being vetoed on this, I am not married to it, but I think using angle brackets is proper in this case. What do you think?

VPR is moving to a style that uses "#pragma once" instead of header
gaurds. These are less error prone and may be slightly more performant.

Converted all of the header gaurds in VPR into pragma once's.

Also moved all pragma onces to the top of all header files to maintain a
consistent style. It is a good idea to have them as the very first line
in all header files.

While going through all header files, cleaned up any extra header
includes which were including things they did not need.
@AlexandreSinger
Copy link
Contributor Author

@soheilshahrouz Ah! I see, I just looked at the code and what you are saying is that it already used double quotation marks and I added angle brackets justs on the ones I added. I understand why thats not good. I turned them into double quotation marks so this does not block the PR; however, we should eventually revisit this topic. Not urgent at all though.

@AlexandreSinger AlexandreSinger force-pushed the feature-include-cleanup branch from 6b8b6bc to d2fa91d Compare May 24, 2025 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants