[libcxx-commits] [PATCH] D122593: Fix CtsDeqpTestCases failures

Mon Mar 28 11:14:31 PDT 2022

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

In D122593#3411724 <https://reviews.llvm.org/D122593#3411724>, @ZijunZhao wrote:

> Question: This format is amended by linter automatically. Does it need to be changed back?

No, you can ignore the linter suggestions. You can actually use `arc diff --nolint` when uploading patches to avoid these warnings, which are not very relevant in libc++.

================
Comment at: libcxx/include/__support/android/locale_bionic.h:34-45
+// HACK: Not in upstream NDK or libc++.
+// Upstream now supports using ToT libc++ with old NDKs, but as such it is now
+// *only* compatible with the NDK. That will need to be fixed both for the
+// platorm and for the NDK-in-platform use case since neither has
+// android/ndk-version.h.
+
+// If we do not have this header, we are in a platform build rather than an NDK
----------------
I don't really have a concern with this patch per se because it's self contained in bionic specific headers, however this doesn't really seem to be meant for upstreaming. At least, we should update the comment so that it makes more sense to carry upstream.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122593