[PATCH] D131963: [libc++] Add custom clang-tidy checks

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D131963#3831786 <https://reviews.llvm.org/D131963#3831786>, @smeenai wrote:

> In D131963#3811811 <https://reviews.llvm.org/D131963#3811811>, @ldionne wrote:
>
>> I'd be fine with this as-is if it works in the CI.
>>
>> IIUC, the current blocker for this is that the `ClangConfig.cmake` installed by LLVM is not robust to the dev packages missing. If you do `find_package(Clang 16.0)`, it will error out if the dev packages are not present, since it will try to reference static libraries (and other artifacts) that don't exist and try to create IMPORTED targets for those. @smeenai @beanz do you know more about that, or do you know someone that does? Do you know who set up the CMake config files so that `find_package(Clang)` would work in the first place? I'd also welcome your input on the `ClangConfigVersion.cmake.in` changes.
>>
>> Just for the context, what we're trying to do here is simply use `clang-tidy`'s development packages from the libc++ build to add our own custom clang-tidy checks.
>>
>> Accepting because this LGTM, although we do have some stuff to resolve before this can be shipped.
>
> IIRC, the intended solution was to use `LLVM_DISTRIBUTION_COMPONENTS` (https://llvm.org/docs/BuildingADistribution.html). When you use that option, the generated CMake package files (at least in the install tree; I'm not sure about the ones in the build tree) should only contain imported targets that were part of your distribution. Multi-distribution support extends this even further, where the file defining the imported targets for a distribution is only imported if it's present, so things should work as expected both with and without the dev packages. Is that workable for your use case?

That's a thing the vendor has to change, right? If we only get the targets which are actually available it should work just fine.



================
Comment at: libcxx/test/tools/clang_tidy_checks/robust_against_adl.cpp:22
+AST_MATCHER(clang::UnresolvedLookupExpr, isCustomizationPoint) {
+  // TODO: Are make_error_code and make_error_condition actually customization points?
+  return std::ranges::any_of(
----------------
jwakely wrote:
> jwakely wrote:
> > Mordante wrote:
> > > ldionne wrote:
> > > > This is funny, I actually saw a LWG issue about that go by a few weeks ago. I'll try to get more details.
> > > @ldionne @jwakely posted this bug report about it https://github.com/llvm/llvm-project/issues/57614
> > They're definitely customization points. If they weren't, specializing `is_error_code_enum` and `is_error_condition_enum` would be completely useless and serve no purpose.
> > 
> > LWG confirmed that and [LWG 3629](https://wg21.link/lwg3629) is Tentatively Ready now.
> I created https://reviews.llvm.org/D134943 to fix those customization points.
Thanks for the info and patch @jwakely!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131963/new/

https://reviews.llvm.org/D131963