Proposing clang-tidy checks for libc++ internal development

Hi All,

I’ve been sitting on my own set of clang-tidy checks specific for libc++. They are intended for developers of libc++, not users of it, and having them can be quite helpful. They include:

  • A reserved name check.
  • Attribute not on declaration check + fix.
  • Externally instantiated function is missing inline check + fix.
  • Externally instantiated function with missing or incorrect visibility attributes check + fix.

As more developers begin to work on libc++, it’s useful for these checks to be available to all of them.

I’m looking to find a way to upstream these checks. My question is:

  1. Are these even appropriate to upstream?
  2. If so, where should we put them?

The simple answer is to add a new libcxx-internal module to clang-tidy. The obvious problem is that clang-tidy ships to all users, and to them this module and its checks are just noise.

What do you think?

/Eric

PS. The non-complete and messy versions of the current checks can be found here. I obviously plan to clean them up before submitting for review upstream. https://github.com/efcs/clang-tools-extra/tree/libcxx-tidy/clang-tidy/libcxx

I would be very keen to see these checks added to clang.

Thanks

Jon

Perhaps consider whether it is time to prioritize

https://bugs.llvm.org//show_bug.cgi?id=32739

For anyone working on non-upstreamable ad-hoc or bespoke porting aids, the
current workflow (and buildsystem) is very inconvenient.

Thanks,

Stephen.

Hi All,

I've been sitting on my own set of clang-tidy checks specific for libc++.
They are intended for developers of libc++, not users of it, and having them
can be quite helpful. They include:

* A reserved name check.
* Attribute not on declaration check + fix.
* Externally instantiated function is missing inline check + fix.
* Externally instantiated function with missing or incorrect visibility
attributes check + fix.

As more developers begin to work on libc++, it's useful for these checks to
be available to all of them.

I'm looking to find a way to upstream these checks. My question is:

1. Are these even appropriate to upstream?
2. If so, where should we put them?

The simple answer is to add a new `libcxx-internal` module to clang-tidy.
The obvious problem is that clang-tidy ships to all users, and to them this
module and its checks are just noise.

What do you think?

I think there's already precedent for adding this sort of thing to
clang-tidy. For instance, we have the LLVM module which is largely
only useful to people working on the LLVM code base. I see no problem
with adding a module for libc++ developers as well. My preference
would be to name the new module "libcxx-module" and prefix the checks
with "libcxx-".

~Aaron

I think there's already precedent for adding this sort of thing to
clang-tidy. For instance, we have the LLVM module which is largely
only useful to people working on the LLVM code base. I see no problem
with adding a module for libc++ developers as well. My preference
would be to name the new module "libcxx-module" and prefix the checks
with "libcxx-".

~Aaron

Yes. There is even google and zircon/fuchsia specific checks, that are
company/project specific.

And a potential mechanism to dynamically load clang-tidy plugins is
probably loading the current modules.
I think adding a module now and doing a potential port to a plugin
infrastructure is ok not too much effort.

Best, Jonas

Awesome! Thanks.

I’ll go ahead and factor out a checker and submit it for review.

/Eric

At LibreOffice, we have a set of LibreOffice-specific checkers, which we load and run as part of clang (they use the ASTMatcher framework).

These get run as part of our CI. One of our CI buildbots uses clang on Linux which executes the checkers and produces warnings if any of them fires.

This is quite handy for us for keeping the checkers happy, without needing everyone to install and use clang (not currently possible on all our supported platforms)

Can you post a link to those?

Thanks,

Stephen.

https://cgit.freedesktop.org/libreoffice/core/tree/compilerplugins/clang

Thanks for that. It's interesting that AST matchers are not used.

Thanks,

Stephen.

​I think our usage of clang plugins predates that, or at least predates the
more improved AST matcher API now available, and we've just stuck with the
style we're used to.​