correctcomputation / checkedc-clang

This is the primary development repository for 3C, a tool for automatically converting legacy C code to the Checked C extension of C, which aims to enforce spatial memory safety. This repository is a fork of Checked C's.
14 stars 5 forks source link

Automate most of the 3C code style checks. #716

Open mattmccutchen-cci opened 2 years ago

mattmccutchen-cci commented 2 years ago

This is something we've said we've wanted for a long time. It isn't particularly urgent now, but for whatever reason (maybe because I was running clang-tidy on #657 recently), I was inspired to hack it together today.

This PR includes:

  1. Build targets to check for style violations (lint-3c, validate-3c, etc.) and a script to try to automatically fix them (clang/tools/3c/utils/code_style/fix-all.sh).
  2. A quick pass of fixing existing style issues in the 3C code, just so that we can test that the build targets in part (1) pass. Some of these style fixes may not be what we want, and we may do the fix pass over later.
  3. A draft documentation update stating that the automated checks should pass on main at all times (and thus, on every PR before it is merged). We can discuss when or if we actually want to adopt such a policy. Another option is to merge the tools but just have me run them periodically as a better automated version of what I've been doing for the omnibus PRs to Microsoft.

I'm going ahead and marking this PR ready for review of parts (1) and (3). Obviously, a style fix pass on main will be disruptive to our 3C branches in progress, so we can discuss when would be a good time to actually do it. It should be harmless to go ahead and merge part (1) by itself once we are satisfied with the quality of that code; if we want to proceed that way, I can move parts (2) and (3) to another PR and merge this one (after review), or move part (1) to another PR and merge that one.

mattmccutchen-cci commented 2 years ago

Decision from the meeting today: We probably do want to eventually make a policy of requiring the automated style checks to pass on every PR, but we want to gain some experience with the tools first. So the plan:

We're interested in posting the status of all checks that our policy requires for each PR onto the PR as GitHub status checks, though our current setup with the private actions repository makes this nontrivial to implement. Aaron urges that if/when we post the status checks, we find a way to display the status of the regression tests and the style checks separately, e.g., as separate GitHub status checks. Implementation details TBD.