This is an archive of the discontinued LLVM Phabricator instance.

[libc] update tidy rules to fix variable formatting
ClosedPublic

Authored by michaelrj on Nov 19 2021, 4:52 PM.

Details

Summary

This commit changes the clang-tidy rules for LLVM-libc to follow the new
format. The next commit applies these rules to the codebase.

The rules are as follows:

CamelCase for classes
lower_case for variables
lower_case for functions
UPPER_CASE for constexpr variables

There are also some exceptions, but the most important one is that any
function or variable that starts with an underscore is exempt from the
formatting.

Diff Detail

Event Timeline

michaelrj created this revision.Nov 19 2021, 4:52 PM
michaelrj requested review of this revision.Nov 19 2021, 4:52 PM
michaelrj updated this revision to Diff 390865.Nov 30 2021, 4:57 PM

update lint rules to better reflect the codebase. Also add a .clang-tidy file to the build folder to prevent the generated code from being formatted (that tends to mess everything up)

michaelrj updated this revision to Diff 391157.Dec 1 2021, 4:02 PM

add an exception for src/__support/CPP because it's supposed to match external code and thus might not match the internal format.

michaelrj updated this revision to Diff 391732.Dec 3 2021, 2:00 PM

add exception for variables starting with underscores and fix exception for functions starting with underscores.

michaelrj edited the summary of this revision. (Show Details)Dec 3 2021, 2:00 PM
sivachandra added inline comments.Dec 6 2021, 3:01 PM
libc/CMakeLists.txt
63

Do we still require this?

libc/src/.clang-tidy
3

Can we add readability-* rules also to this list?

12

Can you give examples as to why this cannot be lower_case?

24

Do we still need this blanket ignore?

michaelrj updated this revision to Diff 392230.Dec 6 2021, 4:51 PM
michaelrj marked 3 inline comments as done.

address comments

libc/CMakeLists.txt
63

yes. The variables in build/projects/libc/config/linux/syscall.h are of a specific format (that being the names of specific registers), and the formatter will raise errors if those aren't specifically ignored.

libc/src/.clang-tidy
3

not with the patch as it currently is, because in the generated signal.h file, the function sigaction is being renamed to __sigaction by the macro #define sigaction __sigaction, which for some reason ignores all of the exceptions I have put in place to handle it. If we rename the struct __sigaction to something that the linter doesn't think would be formatted to sigaction (e.g. __sigaction_struct) then it works fine, but I don't know if that's an acceptable solution.

24

No, removed.

sivachandra accepted this revision.Dec 6 2021, 10:06 PM
sivachandra added inline comments.
libc/src/.clang-tidy
3

I think that can be fixed up as a follow up. You have added the readability-identifier-naming, is that something you forgot to take out?

This revision is now accepted and ready to land.Dec 6 2021, 10:06 PM
lntue accepted this revision.Dec 7 2021, 8:39 AM
michaelrj marked 2 inline comments as done.Dec 7 2021, 10:49 AM
michaelrj added inline comments.
libc/src/.clang-tidy
3

no, that's what enables that specific check.

This revision was automatically updated to reflect the committed changes.
michaelrj marked an inline comment as done.