[libcxx-commits] [PATCH] D110870: [libc++] Rename the 'libc++' Lit feature to 'llvm-libc++'

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/test/std/utilities/function.objects/func.search/func.search.bmh/hash.pred.pass.cpp:10
 // UNSUPPORTED: c++03, c++11, c++14
-// XFAIL: stdlib=libc++
 
----------------
Quuxplusone wrote:
> Off-topic, but what on earth? Why does libc++'s test suite have test cases that are XFAILed //on libc++?//
It's a common misconception that this is only "libc++"'s test suite. The intent is for it to be a general C++ Standard Library conformance test suite, and for it to be usable by any implementation of the C++ Standard Library. Libc++ is merely one of those implementations, and arguably the one that makes the most use of the test suite.

But in reality, libstdc++ and msvc do use our test suite from time to time, too.


================
Comment at: libcxx/utils/libcxx/test/params.py:105
+                 - msvc: The Microsoft implementation of the C++ Standard Library.
+                """,
             actions=lambda stdlib: [
----------------
Quuxplusone wrote:
> I don't really know the use-case here, but maybe consider whether you want to
> - call the non-Apple case `upstream-libc++` or `trunk-libc++` instead of `llvm-libc++`
> - have an umbrella `libc++` feature which can be `REQUIRE`'d against, in addition to the finer-grained subfeatures. (This would let you avoid changing the test suite at all. Any day we can avoid introducing regexes into the test suite, is a good day.)
> call the non-Apple case `upstream-libc++` or `trunk-libc++` instead of `llvm-libc++`

Yeah, I considered a few names but: `upstream-libc++` makes it look like there's a "downstream" libc++ (as in a fork of libc++), which is not the case. It's more that it's configured differently. `trunk-libc++` introduces a notion of being at the top of `HEAD`, which isn't really what I want to convey. I think it makes sense to talk about LLVM libc++ as opposed to <vendor> libc++, kind of how we distinguish LLVM Clang and Apple Clang. In a way, LLVM can be seen as a "vendor" for the toolchain that is released on llvm.org.

> have an umbrella libc++ feature which can be REQUIRE'd against, in addition to the finer-grained subfeatures. (This would let you avoid changing the test suite at all. Any day we can avoid introducing regexes into the test suite, is a good day.)

I thought about that too but then refrained from doing it because I thought to myself "we added support for regular expressions just for that, why not use it?". Maybe that was the wrong call. I'll add a top-level feature as you suggest.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110870