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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In D100866#2705673 <https://reviews.llvm.org/D100866#2705673>, @Quuxplusone wrote:

> Eliminate all my fixes from this review, now that buildkite is happy. Now this review consists only of what will be committed as a single commit, following my original "roadmap" idea. Tests that fail at the moment are marked `XFAIL`; I'll submit PRs to fix them individually.

Seems you forgot to restore the removed builds from libcxx/utils/ci/buildkite-pipeline.yml
Can you update this file to give the patch a full CI run.



================
Comment at: libcxx/src/debug.cpp:441
     __c_node* cj = j != nullptr ? j->__c_ : nullptr;
-    return ci != nullptr && ci == cj;
+    return ci == cj;  // N3644
 }
----------------
Quuxplusone wrote:
> 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! :)
Thanks for the information and the new patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100866