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

Migrate P4Testgen tooling to core P4C (IWYU, clang-format, cpplint, clang-tidy, git hooks) #3663

Merged
merged 9 commits into from
Nov 8, 2022

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Nov 4, 2022

This PR does a couple things.

  • It moves configurations files and scripts for IWYU, clang-format, cpplint, clang-tidy, and some git hooks from the P4Tools back end to the P4C main tools folder.
  • It adds configuration files for clang-format and clang-tidy to P4C. Both of these enforce a specific style in coding and formatting. The style is configured such that it closely preserves P4C's current style.
  • This PR also removes the cpplint test from CI and adds it as a separate PR using a dedicated script. The advantage of this is that cpplint is decoupled from the CI end-to-end tests. One should get feedback much earlier now. There are also a couple files which were not checked by cpplint. They are covered now.
  • The linting PR also ensures that submodules do not accidentally change the upstream branch (e.g., a private fork). It also enforces formatting of .cpp and .h files (for P4Tools only for now).
  • There are no scripts to run IWYU yet. This can be added in a subsequent PR.
  • clang-format is only applied to P4Tools, but can also easily be applied to the rest of the compiler, if desired. The one problem is that it touches a lot of code.

Open for comments on this one. @davidbolvansky This PR might also be interesting to you.

Fixes #3516.

Permissions.


Small updates.
@fruffy fruffy force-pushed the fruffy/tooling branch 2 times, most recently from c5b1af5 to 6eefb32 Compare November 4, 2022 23:04
@fruffy fruffy marked this pull request as ready for review November 6, 2022 22:16
@@ -29,7 +29,6 @@ set (BMV2_SIMPLE_SWITCH_SRCS
simple_switch/simpleSwitch.h
simple_switch/options.h
)
add_cpplint_files (${CMAKE_CURRENT_SOURCE_DIR} "${BMV2_SIMPLE_SWITCH_SRCS}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these still checked?

Copy link
Collaborator Author

@fruffy fruffy Nov 6, 2022

Choose a reason for hiding this comment

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

They are checked by a new CI run now. It should actually more thorough than what we had before because you do not have to manually add files for linting any more.

We are adding yet another CI test, but, thankfully, this one only takes about 1-2 minutes.

@@ -144,7 +143,8 @@ void UBPFTableBase::emitInstance(EBPF::CodeBuilder *builder, EBPF::TableKind tab
auto scalar = new UBPFScalarType(tb);
keyTypeStr = scalar->getAsString();
} else if (keyType->is<IR::Type_StructLike>()) {
keyTypeStr = cstring("struct ") + keyTypeName.c_str();;
keyTypeStr = cstring("struct ") + keyTypeName.c_str();
;
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete

@davidbolvansky
Copy link
Contributor

Very nice, thanks!

@fruffy fruffy merged commit e25e930 into main Nov 8, 2022
@fruffy fruffy deleted the fruffy/tooling branch November 10, 2022 17:18
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.

Merge the testgen tools with the p4c tools
3 participants