This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Rename the 'libc++' Lit feature to 'llvm-libc++'
ClosedPublic

Authored by ldionne on Sep 30 2021, 11:55 AM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Commits
rGf800560ff1cb: [libc++] Rename the 'libc++' Lit feature to 'llvm-libc++'
Summary

This is to simplify an upcoming change where we distinguish between
flavors of libc++ by adding an apple-libc++ Lit feature.

Diff Detail

Event Timeline

ldionne created this revision.Sep 30 2021, 11:55 AM
ldionne requested review of this revision.Sep 30 2021, 11:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 30 2021, 11:55 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
Quuxplusone added inline comments.
libcxx/test/std/utilities/function.objects/func.search/func.search.bmh/hash.pred.pass.cpp
10 ↗(On Diff #376313)

Off-topic, but what on earth? Why does libc++'s test suite have test cases that are XFAILed on libc++?

libcxx/utils/libcxx/test/params.py
105

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.)
ldionne updated this revision to Diff 376330.Sep 30 2021, 12:42 PM
ldionne marked 2 inline comments as done.

Address review comments

ldionne added inline comments.Sep 30 2021, 12:43 PM
libcxx/test/std/utilities/function.objects/func.search/func.search.bmh/hash.pred.pass.cpp
10 ↗(On Diff #376313)

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.

libcxx/utils/libcxx/test/params.py
105

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.

ldionne updated this revision to Diff 376414.Sep 30 2021, 8:12 PM

Try to poke CI

ldionne updated this revision to Diff 376875.Oct 4 2021, 5:48 AM

Poke again

ldionne accepted this revision.Oct 4 2021, 3:32 PM
This revision is now accepted and ready to land.Oct 4 2021, 3:32 PM
This revision was automatically updated to reflect the committed changes.