[libcxx-commits] [PATCH] D127978: [libc++][test] Replaces TEST_IS_CONSTANT_EVALUATED.

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Jun 20 12:25:20 PDT 2022

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

While I have very little love for `TEST_IS_CONSTANT_EVALUATED`, I am not sure this increase in complexity for the reader is the right way to go forward in order to fix a (arguably kind of spurious) compiler warning. I'm eager to better understand the answer to the questions I've left and see if that makes me see this patch with different eyes.

Comment at: libcxx/test/std/strings/string.view/string.view.comparison/equal.pass.cpp:58
-            if (!TEST_IS_CONSTANT_EVALUATED || TEST_STD_VER >= 20) {
+            if (!is_constant_evaluated(TestedCppVersion::Cpp14) || TEST_STD_VER >= 20) {
                 assert((std::basic_string<CharT, Traits>(v[i]) == v[j]) == expected);
So the goal of this patch is to get rid of tautological comparison warnings in GCC. However, I am not sure this is a proper solution to this. Indeed, `!is_constant_evaluated(TestedCppVersion::Cpp14) || TEST_STD_VER >= 20` could easily be flagged as tautological by GCC (or Clang) if for example `TEST_STD_VER >= 20` evaluates to `true`. It looks like we've simply increased the complexity of the expression enough for GCC not to be as clever as ourselves, and hence it doesn't produce the warning. In other words, I am not convinced this is a long term or robust solution to this issue.

Comment at: libcxx/test/std/strings/string.view/string.view.comparison/equal.pass.cpp:79
+    if (!is_constant_evaluated(TestedCppVersion::Cpp14) || TEST_STD_VER >= 20) {
         assert((std::basic_string<CharT, Traits>(abc) == abc0def) == false);
I am not sure I understand what's the purpose of passing `TestedCppVersion::CppNN` to this function. Is there any case in which the `NN` version passed here should differ from the `NN` version used in the `TEST_CONSTEXPR_CXXNN` macro of the enclosing function? If not, then perhaps it doesn't really make sense to pass a standard version to this function? And if that's the case, then we're basically back to the `TEST_IS_CONSTANT_EVALUATED` macro again.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list