[libcxx-commits] [PATCH] D100866: Add a buildkite for _LIBCPP_DEBUG=1.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone marked an inline comment as done.
Quuxplusone added inline comments.


================
Comment at: libcxx/cmake/caches/Generic-debug.cmake:1
+set(LIBCXX_TEST_PARAMS "std=c++2b" "debug_level=1" "additional_features=LIBCXX-DEBUG-FIXME" CACHE STRING "")
+set(LIBCXXABI_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")
----------------
ldionne wrote:
> This shouldn't set `std=c++2b` explicitly, I believe we should be using the default here.
I want to make sure that we're testing new features as soon as they're pull-requested; testing only C++14 mode wouldn't have caught the issues with `<span>`. Of course we could add debug-mode testing for multiple versions, like `std=c++03` AND `std=c++2b` and maybe something in between; I thought that was overkill, but I'd do it if you want it.

I //don't// think we want to be testing only C++14, though; I think we want //at least// C++20, and I see only upsides to testing C++2b (given that the goal here is to catch things at pull-request time, and people tend to pull-request mostly features that are enabled in '20 and '2b, not '14 anymore).

How about if I provide just `generic-cxx03-debug` and `generic-cxx2b-debug`, with those names? (This also helps to indicate that they're not the opposite of `generic-no-debug`, as brought up below.)


================
Comment at: libcxx/utils/ci/run-buildbot:518
+generic-debug)
+    export CC=clang-tot
+    export CXX=clang++-tot
----------------
ldionne wrote:
> Should be `clang` and `clang++`, like the other jobs?
> 
> Also, please move close to the `generic-no-debug)` step.
(A) `clang-tot` turned out to be the magic incantation that makes the `std=c++2b` parameter work. Compare to the `generic-cxx2b)` case. (If we stopped trying to test 2b pull requests and went to 20 or earlier, then this could indeed be just `clang`.)

(B) Will do. But notice that `generic-no-debug` is actually talking about a build configuration that //doesn't build debug.cpp into the library//; it's not the "opposite" of `generic-debug`. All of `generic-no-{debug,filesystem,localization,random_device}` have the same structure of "normal compilation, just don't link in some .cpp files to the library."


================
Comment at: libcxx/utils/libcxx/test/config.py:484
         self.cxx.compile_flags += ['-D_LIBCPP_DEBUG=%s' % debug_level]
+        self.config.available_features.add('debug_level_%s' % debug_level)
 
----------------
ldionne wrote:
> You can rebase on top of 3d8f2059b95d3e9d6785255949283b258d7ca121.
I'll do so. But actually I'm pretty sure that all the trouble I had was completely due to writing `clang`/`std=c++2b` instead of (either `clang-tot`/`std=c++2b` or `clang`/`std=c++20`) above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100866