Skip to content

Format clang/test/3C #481

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

Merged
merged 20 commits into from
Mar 12, 2021
Merged

Conversation

mattmccutchen-cci
Copy link
Member

@mattmccutchen-cci mattmccutchen-cci commented Mar 11, 2021

Previous discussion. I think Mike has already approved this in principle, but I'm giving everyone one more chance to raise any concerns before it goes into our pending PR to Microsoft (checkedc#993). Of course, the next step will be to make the analogous change to our main branch, but we can take more time to review that if we want. (Update 2021-03-30: I'm finally doing this in #518.)

path.

I apparently broke this in #412 and neglected to test testgenerator.py.
This is to increase confidence that we're not losing anything when we
re-run the test-updating programs after running clang-format.

- alloc_type_param: This file crashes test_updater because 3C's
  rewriting of the inline struct declaration violates test_updater's
  assumption that the number of lines in the file does not change.
  Just make the file unmanaged for now.

- extstructfields: Teach test_updater not to strip "//" comments other
  than the "// RUN" and "//CHECK" that it generates, and move the
  "// UNSUPPORTED" comment below the "// RUN" block where test_updater
  wants it.

- funcptr{1,2}: Use a special "/* GENERATE CHECK */" marker to tell
  test_updater to generate CHECK comments on the
  function-pointer-related lines we care about since it's too difficult
  to recognize them textually.

- ptrptr: Make test_updater generate CHECK comments on all array
  declarations. This gives us the CHECK comment we want in this test
  case, plus two others that seem harmless.
The changes look harmless aside from whitespace, which will be redone
soon anyway. The addition of checked regions is presumably due to #426.

(testgenerator.py and processor.py already had no diffs.)
- Use f-strings for the RUN lines. This requires Python 3.

- Disable formatting of the lists of test files, which we want to keep
  flat. While we're here, remove the b_tests list from test_updater.py,
  which doesn't use it.

- Add parentheses around some expressions to give YAPF better ways to
  break them. Break up another few lines manually.

Still no diffs in the managed test files.
Since YAPF doesn't change behavior, it doesn't remove trailing
whitespace in multi-line string literals; that will come later when we
update the programs to generate output compliant with clang-format.

Still no diffs in the managed test files.
get CHECK comments.

This is consistent with all other managed tests, and it would be
difficult for test_updater.py to generate CHECK comments that would pass
clang-format for multi-line declarations like these.
- The output is now fully compliant with clang-format (assuming, in the
  case of test_updater.py and processor.py, that the input was). This
  required addressing a number of issues: reformatting the templates
  that generate code for insertion into the output files, recognizing
  `extern` declarations that are wrapped across several lines, correctly
  indenting the generated CHECK comments, and maybe more I've missed.

- Adjust the generated comments (other than the special RUN and CHECK
  comments) to fit the 80-character limit. We've configured clang-format
  not to reflow comments, so we need to check this separately.

- While I'm here, make some tweaks to the output that are not necessary
  for compliance but still nice: most notably, fix some inconsistent
  whitespace within generated CHECK comments.

- Make further changes to the test-updating programs (not already made
  by YAPF) so they themselves fit the 80-character limit: wrap comments
  and break up string literals.
testgenerator.py, as a baseline for fixes.
FileCheck automatically recognizes any specified prefix followed by
"-NEXT".
"CHECK:" for no apparent reason.

(I left the spaces unchanged if they seemed intended to mirror the
indentation of the output of 3C.)

This cleanup is not necessary for the tests to pass, but I thought it
made sense to do along with fixing the CHECK lines that didn't have a
colon in the right place.
(And use `// clang-format off` to stop it from happening again.)
Apparently I forgot to do this before. clang-format made only one
change, which is wrong but consistent with what we already let it do to
many .c files and not worth fixing now.
@mattmccutchen-cci mattmccutchen-cci merged commit b2a910c into pr-to-microsoft-202102 Mar 12, 2021
@mattmccutchen-cci mattmccutchen-cci deleted the format-clang-test-3c branch March 12, 2021 18:13
mattmccutchen-cci added a commit that referenced this pull request Mar 30, 2021
- Run clang-format and apply exclusions as appropriate.

- Manually fix an over-length comment.
mattmccutchen-cci added a commit that referenced this pull request Mar 30, 2021
- Run clang-format and apply exclusions as appropriate.

- Manually fix an over-length comment.
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.

1 participant