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

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

Quuxplusone added inline comments.


================
Comment at: libcxx/src/debug.cpp:441
     __c_node* cj = j != nullptr ? j->__c_ : nullptr;
-    return ci != nullptr && ci == cj;
+    return ci == cj;  // N3644
 }
----------------
Mordante wrote:
> This seems trivial, but I think it should land in a separate commit. (I don't mind to do that after accepting this patch.)
> I assume this is what you mean with "Land the fix to support N3644 "Null Forward Iterators" in debug mode." correct?
> 
> Just curious how did you discover this?
It all started with D100029. :) That led to looking at `_LIBCPP_DEBUG == 2` assertions, which led to looking at all code under `#if _LIBCPP_DEBUG == 2` (and incidentally D100737 too), which led to D100730, which revealed that `<span>` was completely broken in debug mode, we just didn't have any tests specifically for span-in-debug-mode. So, since the apparent intent is that everything //should// just work in debug mode... let's test it!

Running tests locally with `-D_LIBCPP_DEBUG=1` turned up some failures due to N3644 immediately, so it just happened that I fixed this line before finishing the buildkite-related parts of the patch. Then I ran tests locally and marked the remaining offenders with `XFAIL`; but, having already fixed N3644, I didn't actually know how many tests are affected by it — just "at least one."

But now I know, and have filed D100881! :)


================
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)
 
----------------
I still haven't figured out why https://buildkite.com/llvm-project/libcxx-ci/builds/2676#7b1ae675-f9cd-4f9d-8b4a-6db9ec8f8e53 is so unhappy with this script now...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100866