[libcxx-commits] [PATCH] D129968: [libc++] Add a bunch of missing _LIBCPP_HIDE_FROM_ABI

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

In D129968#3693940 <https://reviews.llvm.org/D129968#3693940>, @Mordante wrote:

> Validate all `main` functions in the tests have the proper signature and have a `return 0;`.

If we go there, we might as well check that `main` is declared as `int main(int, char**)` (and not `void main(...)` or `int main(int, char const**` or other variations).

LGTM, but please ping me in a week to check whether it's fine to commit this (to avoid conflicts when we cherry-pick to LLVM 15).



================
Comment at: libcxx/test/libcxx/clang_query/hide_from_abi_or_visible.query:30
+      hasAttr("attr::AbiTag"),
+      cxxMethodDecl(),
+      isDeleted(),
----------------
Can you explain why you skip those in a 1 line comment?


================
Comment at: libcxx/utils/libcxx/test/features.py:29
+  try:
+    return int(re.search('[0-9]+', commandOutput(cfg, ['clang-query --version'])).group()) >= 13
+  except ConfigurationRuntimeError:
----------------
Mordante wrote:
> Mordante wrote:
> > philnik wrote:
> > > philnik wrote:
> > > > Mordante wrote:
> > > > > Mordante wrote:
> > > > > > Does this also find `clang-query-15`? This is the default name used in the `apt.llvm.org` so I would like that to work out of the box.
> > > > > Can you update the developer documentation with this tool (I think clang-tidy) also isn't listed.
> > > > No, it doesn't find `clang-query-15`. I also don't know how this could be implemented in a way that finds different versions. We don't know which version is the correct one if we find multiple. IMO we shouldn't try to find some executable but rather ask the user to put a `clang-query` into the path. That's a lot safer. With the compiler it's a different story, since we get that through CMake.
> > > What exactly do you mean? Where do we have any developer documentation w.r.t. tooling?
> > https://libcxx.llvm.org/TestingLibcxx.html should list the dependencies to be able to execute the tests.
> > No, it doesn't find `clang-query-15`. I also don't know how this could be implemented in a way that finds different versions. We don't know which version is the correct one if we find multiple. IMO we shouldn't try to find some executable but rather ask the user to put a `clang-query` into the path. That's a lot safer. With the compiler it's a different story, since we get that through CMake.
> 
> It's already possible to select the wanted compiler and several other features using CMake options. I really would like to make this a configuration option too.
I agree we should list tools that *can* be used in the test suite, but we should of course be careful about not requiring them (which is not the case here so it's all good).

@philnik Can you add a section in `TestingLibcxx.rst` that mentions `clang-query` as being an optional dependency of the test suite. We can then add more stuff to it (there's `gdb` and certainly others). I think it will be useful to list them all in a single place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129968