This is an archive of the discontinued LLVM Phabricator instance.

[libc] Add cmake target for linting libc.
ClosedPublic

Authored by PaulkaToast on Apr 10 2020, 12:50 AM.

Details

Summary

This patch implements running linting on llvm-libc using build rule targets.

  1. adds a new target per entrypoint for linting with the naming convention <qualified_target_name>.__lint__ e.g libc.src.string.strlen.__lint__.
  2. makes the build target for each entrypoint depend on the linting targets so that they run along with compilation of each entrypoint.
  3. adds a lint all target named lint-libc. check-libc now depends on this new target.
  4. linting creates a lot of additional targets from clang and clang-tidy that need to be built so an opt out flag can be passed to cmake: LLVM_LIBC_ENABLE_LINTING.

Diff Detail

Event Timeline

PaulkaToast created this revision.Apr 10 2020, 12:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 10 2020, 12:50 AM
PaulkaToast edited the summary of this revision. (Show Details)Apr 10 2020, 12:51 AM
abrachet added inline comments.
libc/CMakeLists.txt
22

This is quite the dependency. Would it be possible to only require it be built if the installed clang-tidy doesn't have llvmlibc-*.

PaulkaToast edited the summary of this revision. (Show Details)
PaulkaToast added reviewers: sivachandra, abrachet.
PaulkaToast marked 2 inline comments as done.
PaulkaToast added inline comments.
libc/CMakeLists.txt
22

There is now an opt-out available and the linting step will be run on the buildbots as well. (:

sivachandra accepted this revision.Apr 15 2020, 11:58 AM

LGTM. Minor comments inline.

libc/cmake/modules/LLVMLibCRules.cmake
352

Make the prefix .__lint_timestamp__ to make it absolutely clear it is internal.

364

Nit: Put this comment at the end of the line below or align with the indentation below.

380

s/lint_dummy/lint_timestamp ?

This revision is now accepted and ready to land.Apr 15 2020, 11:58 AM
sivachandra added inline comments.Apr 15 2020, 12:10 PM
libc/CMakeLists.txt
22

Sorry, one more nit: can we call this LLVM_LIBC_ENABLE_LINTING as this is really libc specific?

PaulkaToast marked 2 inline comments as done.

Addressed comments. (:

PaulkaToast edited the summary of this revision. (Show Details)
PaulkaToast marked 3 inline comments as done.
This revision was automatically updated to reflect the committed changes.