[libcxx-commits] [PATCH] D141805: [libc++] Refactor clang-query checks to clang-tidy checks to get less obscure error messages

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
Comment at: libcxx/test/tools/clang_tidy_checks/hide_from_abi.cpp:36
+              hasAttr(clang::attr::AbiTag),
+              cxxMethodDecl(), // We have explicitly instantiated classes and some of their methods don't have these attributes
+              isDeleted(),
----------------
philnik wrote:
> Mordante wrote:
> > philnik wrote:
> > > Mordante wrote:
> > > > I assume this has to to with the list of `hasName` above. If so let's move the comment there and please elaborate a bit more why these functions are excluded. Just to make sure that it's clear why this is needed.
> > > > 
> > > > A question, is `std::ranges::__stable_sort` also added to this list? Or only names in `std::` ?
> > > I'm, not sure what you are referring to with `I assume this has to do with the list of hasName above`.  The functions are excluded because they can't be marked `[[gnu::always_inline]]` for various reasons.
> > > 
> > > This check only looks through functions, so `ranges::__stable_sort` isn't related.
> > > 
> > Can you then add a comment like `These functions are excluded because they can't be marked [[gnu::always_inline]].`
> > What happens when `std::ranges::__stable_sort` is a function? I forgot it was a cpo
> It would also be ignored: https://godbolt.org/z/YPdY6WqP7
Then I think it would be better to test against the fully qualified name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141805