[libcxx-commits] [PATCH] D126667: [libc++][CI] Updates GCC to version 12.
Mark de Wever via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Jun 17 12:00:12 PDT 2022
Mordante created this revision.
Herald added subscribers: arphaman, arichardson.
Herald added a project: All.
Mordante updated this revision to Diff 435660.
Mordante added a comment.
Mordante updated this revision to Diff 436135.
Mordante updated this revision to Diff 436154.
Mordante added subscribers: philnik, ldionne.
Mordante edited the summary of this revision.
Mordante updated this revision to Diff 437952.
Mordante added reviewers: ldionne, philnik.
Mordante published this revision for review.
Herald added projects: libc++, libc++abi.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.
Rebased on the new Jammy based CI.
Mordante added a comment.
Rebased and fixed some issues.
Mordante added a comment.
Test whether -Wno-tautological-compare is still needed.
Mordante added a comment.
Rebased on D127978 <https://reviews.llvm.org/D127978> to see whether that commit fixes all new GCC-12 diagnostics.
================
Comment at: libcxx/utils/libcxx/test/params.py:33
+ # function. (This mostely happens in C++11 mode.)
+ # '-Wno-tautological-compare',
+
----------------
This disabled leads to a lot of errors in C++11 with GCC especially in the string tests.
The issue is the code is tested `constexpr` starting with C++20. Therefore the compiler can deduce that in C++11 mode the function never is `constexpr` and thus the test always `false`.
```
In file included from /home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/std/strings/basic.string/string.modifiers/string_copy/copy.pass.cpp:20:
/home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/std/strings/basic.string/string.modifiers/string_copy/copy.pass.cpp: In function 'void test(S, typename S::value_type*, typename S::size_type, typename S::size_type)':
/home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/support/test_macros.h:146:68: error: 'std::is_constant_evaluated' always evaluates to false in a non-'constexpr' function [-Werror=tautological-compare]
146 | # define TEST_IS_CONSTANT_EVALUATED __builtin_is_constant_evaluated()
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
/home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/std/strings/basic.string/string.modifiers/string_copy/copy.pass.cpp:38:15: note: in expansion of macro 'TEST_IS_CONSTANT_EVALUATED'
38 | else if (!TEST_IS_CONSTANT_EVALUATED)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
```
I see several possible solutions
* Disable the compile flag in C++11 (This works since we only test GCC in C++11 and C++latest.)
* Disable the compile flag unconditionally
* add macros like `TEST_IS_CONSTANT_EVALUATED_AFTER_11`
I think solution 1 would be the best, @ldionne, @philnik WDYT?
================
Comment at: libcxx/utils/libcxx/test/params.py:33
+ # function. (This mostely happens in C++11 mode.)
+ # '-Wno-tautological-compare',
+
----------------
Mordante wrote:
> This disabled leads to a lot of errors in C++11 with GCC especially in the string tests.
>
> The issue is the code is tested `constexpr` starting with C++20. Therefore the compiler can deduce that in C++11 mode the function never is `constexpr` and thus the test always `false`.
>
> ```
> In file included from /home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/std/strings/basic.string/string.modifiers/string_copy/copy.pass.cpp:20:
>
> /home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/std/strings/basic.string/string.modifiers/string_copy/copy.pass.cpp: In function 'void test(S, typename S::value_type*, typename S::size_type, typename S::size_type)':
>
> /home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/support/test_macros.h:146:68: error: 'std::is_constant_evaluated' always evaluates to false in a non-'constexpr' function [-Werror=tautological-compare]
>
> 146 | # define TEST_IS_CONSTANT_EVALUATED __builtin_is_constant_evaluated()
>
> | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
>
> /home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/std/strings/basic.string/string.modifiers/string_copy/copy.pass.cpp:38:15: note: in expansion of macro 'TEST_IS_CONSTANT_EVALUATED'
>
> 38 | else if (!TEST_IS_CONSTANT_EVALUATED)
>
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~
> ```
> I see several possible solutions
> * Disable the compile flag in C++11 (This works since we only test GCC in C++11 and C++latest.)
> * Disable the compile flag unconditionally
> * add macros like `TEST_IS_CONSTANT_EVALUATED_AFTER_11`
>
> I think solution 1 would be the best, @ldionne, @philnik WDYT?
Do you know if the `TEST_IS_CONSTANT_EVALUATED` cases are the only ones? I think we should refactor them anyways. In hindsight this should have never been a macro. I would change it to something like
```
namespace test {
enum Version {
Cpp20,
Cpp23,
};
constexpr bool is_constant_evaluated_impl() {
#if TEST_STD_VER >= 20 && defined(__cpp_lib_is_constant_evaluated) && __cpp_lib_is_constant_evaluated >= 201811L
return std::is_constant_evaluated();
#else
return false;
#endif
}
constexpr bool is_constant_evaluated(Version ver) {
switch (ver) {
case Cpp20:
return is_constant_evaluated_impl() && TEST_STD_VER >= 20;
case Cpp23:
return is_constant_evaluated_impl() && TEST_STD_VER >= 23;
}
assert(false);
}
}
```
This allows us to replace the current uses of `TEST_IS_CONSTANT_EVALUATED` with `test::is_constant_evaluated(test::Cpp20)` and for `constexpr std::unique_ptr` use `test::is_constant_evaluated(test::Cpp23)`. I also think that we should just remove the `__builtin_is_constant_evaluated()` part. It's unused anyways currently. If we need it at any point we can still add it later. According to https://godbolt.org/z/cqdv88xez GCC is also happy with this reworked version.
================
Comment at: libcxx/utils/libcxx/test/params.py:33
+ # function. (This mostely happens in C++11 mode.)
+ # '-Wno-tautological-compare',
+
----------------
philnik wrote:
> Mordante wrote:
> > This disabled leads to a lot of errors in C++11 with GCC especially in the string tests.
> >
> > The issue is the code is tested `constexpr` starting with C++20. Therefore the compiler can deduce that in C++11 mode the function never is `constexpr` and thus the test always `false`.
> >
> > ```
> > In file included from /home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/std/strings/basic.string/string.modifiers/string_copy/copy.pass.cpp:20:
> >
> > /home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/std/strings/basic.string/string.modifiers/string_copy/copy.pass.cpp: In function 'void test(S, typename S::value_type*, typename S::size_type, typename S::size_type)':
> >
> > /home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/support/test_macros.h:146:68: error: 'std::is_constant_evaluated' always evaluates to false in a non-'constexpr' function [-Werror=tautological-compare]
> >
> > 146 | # define TEST_IS_CONSTANT_EVALUATED __builtin_is_constant_evaluated()
> >
> > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
> >
> > /home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/std/strings/basic.string/string.modifiers/string_copy/copy.pass.cpp:38:15: note: in expansion of macro 'TEST_IS_CONSTANT_EVALUATED'
> >
> > 38 | else if (!TEST_IS_CONSTANT_EVALUATED)
> >
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~
> > ```
> > I see several possible solutions
> > * Disable the compile flag in C++11 (This works since we only test GCC in C++11 and C++latest.)
> > * Disable the compile flag unconditionally
> > * add macros like `TEST_IS_CONSTANT_EVALUATED_AFTER_11`
> >
> > I think solution 1 would be the best, @ldionne, @philnik WDYT?
> Do you know if the `TEST_IS_CONSTANT_EVALUATED` cases are the only ones? I think we should refactor them anyways. In hindsight this should have never been a macro. I would change it to something like
> ```
> namespace test {
> enum Version {
> Cpp20,
> Cpp23,
> };
>
> constexpr bool is_constant_evaluated_impl() {
> #if TEST_STD_VER >= 20 && defined(__cpp_lib_is_constant_evaluated) && __cpp_lib_is_constant_evaluated >= 201811L
> return std::is_constant_evaluated();
> #else
> return false;
> #endif
> }
>
> constexpr bool is_constant_evaluated(Version ver) {
> switch (ver) {
> case Cpp20:
> return is_constant_evaluated_impl() && TEST_STD_VER >= 20;
> case Cpp23:
> return is_constant_evaluated_impl() && TEST_STD_VER >= 23;
>
> }
> assert(false);
> }
> }
> ```
> This allows us to replace the current uses of `TEST_IS_CONSTANT_EVALUATED` with `test::is_constant_evaluated(test::Cpp20)` and for `constexpr std::unique_ptr` use `test::is_constant_evaluated(test::Cpp23)`. I also think that we should just remove the `__builtin_is_constant_evaluated()` part. It's unused anyways currently. If we need it at any point we can still add it later. According to https://godbolt.org/z/cqdv88xez GCC is also happy with this reworked version.
I'm not sure whether these are the only ones. If we go with your approach or `TEST_IS_CONSTANT_EVALUATED_AFTER_11` it should be a separate patch. Then this patch needs to be on top of that one. Then it's easier to investigate whether more work is needed.
Interesting approach. I'll give this one some more thought.
================
Comment at: libcxx/utils/libcxx/test/params.py:33
+ # function. (This mostely happens in C++11 mode.)
+ # '-Wno-tautological-compare',
+
----------------
Mordante wrote:
> philnik wrote:
> > Mordante wrote:
> > > This disabled leads to a lot of errors in C++11 with GCC especially in the string tests.
> > >
> > > The issue is the code is tested `constexpr` starting with C++20. Therefore the compiler can deduce that in C++11 mode the function never is `constexpr` and thus the test always `false`.
> > >
> > > ```
> > > In file included from /home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/std/strings/basic.string/string.modifiers/string_copy/copy.pass.cpp:20:
> > >
> > > /home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/std/strings/basic.string/string.modifiers/string_copy/copy.pass.cpp: In function 'void test(S, typename S::value_type*, typename S::size_type, typename S::size_type)':
> > >
> > > /home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/support/test_macros.h:146:68: error: 'std::is_constant_evaluated' always evaluates to false in a non-'constexpr' function [-Werror=tautological-compare]
> > >
> > > 146 | # define TEST_IS_CONSTANT_EVALUATED __builtin_is_constant_evaluated()
> > >
> > > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
> > >
> > > /home/libcxx-builder/.buildkite-agent/builds/28dd04a444a9-1/llvm-project/libcxx-ci/libcxx/test/std/strings/basic.string/string.modifiers/string_copy/copy.pass.cpp:38:15: note: in expansion of macro 'TEST_IS_CONSTANT_EVALUATED'
> > >
> > > 38 | else if (!TEST_IS_CONSTANT_EVALUATED)
> > >
> > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~
> > > ```
> > > I see several possible solutions
> > > * Disable the compile flag in C++11 (This works since we only test GCC in C++11 and C++latest.)
> > > * Disable the compile flag unconditionally
> > > * add macros like `TEST_IS_CONSTANT_EVALUATED_AFTER_11`
> > >
> > > I think solution 1 would be the best, @ldionne, @philnik WDYT?
> > Do you know if the `TEST_IS_CONSTANT_EVALUATED` cases are the only ones? I think we should refactor them anyways. In hindsight this should have never been a macro. I would change it to something like
> > ```
> > namespace test {
> > enum Version {
> > Cpp20,
> > Cpp23,
> > };
> >
> > constexpr bool is_constant_evaluated_impl() {
> > #if TEST_STD_VER >= 20 && defined(__cpp_lib_is_constant_evaluated) && __cpp_lib_is_constant_evaluated >= 201811L
> > return std::is_constant_evaluated();
> > #else
> > return false;
> > #endif
> > }
> >
> > constexpr bool is_constant_evaluated(Version ver) {
> > switch (ver) {
> > case Cpp20:
> > return is_constant_evaluated_impl() && TEST_STD_VER >= 20;
> > case Cpp23:
> > return is_constant_evaluated_impl() && TEST_STD_VER >= 23;
> >
> > }
> > assert(false);
> > }
> > }
> > ```
> > This allows us to replace the current uses of `TEST_IS_CONSTANT_EVALUATED` with `test::is_constant_evaluated(test::Cpp20)` and for `constexpr std::unique_ptr` use `test::is_constant_evaluated(test::Cpp23)`. I also think that we should just remove the `__builtin_is_constant_evaluated()` part. It's unused anyways currently. If we need it at any point we can still add it later. According to https://godbolt.org/z/cqdv88xez GCC is also happy with this reworked version.
> I'm not sure whether these are the only ones. If we go with your approach or `TEST_IS_CONSTANT_EVALUATED_AFTER_11` it should be a separate patch. Then this patch needs to be on top of that one. Then it's easier to investigate whether more work is needed.
>
> Interesting approach. I'll give this one some more thought.
I like @philnik 's approach, but I am not sure it will resolve the issues. Technically, GCC could still figure out that we're returning false if it evaluates the `is_constant_evaluated(Cpp20)` expression deep enough. But yeah, I'd definitely welcome a patch that tries to do this -- personally I've never been a huge fan of the `TEST_IS_CONSTANT_EVALUATED` macro anyways.
================
Comment at: libcxxabi/test/catch_member_function_pointer_02.pass.cpp:18
// This is likely a bug in their implementation. Investigation needed.
-// XFAIL: gcc-11
+// XFAIL: gcc-11, gcc-12
----------------
I'll remove gcc-11 before landing or with the next iteration of this patch.
Note this patch is expected to fail until the CI has GCC 12 installed.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126667
Files:
libcxx/docs/index.rst
libcxx/test/libcxx/gdb/gdb_pretty_printer_test.sh.cpp
libcxx/test/std/ranges/range.factories/range.single.view/assign.pass.cpp
libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.bool.pass.cpp
libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.c_string.pass.cpp
libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.char_array.pass.cpp
libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.const_char_array.pass.cpp
libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.floating_point.pass.cpp
libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.handle.pass.cpp
libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.pointer.pass.cpp
libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.string.pass.cpp
libcxx/test/std/utilities/format/format.functions/format.locale.pass.cpp
libcxx/test/std/utilities/format/format.functions/format.pass.cpp
libcxx/test/std/utilities/format/format.functions/format_to.locale.pass.cpp
libcxx/test/std/utilities/format/format.functions/format_to.pass.cpp
libcxx/test/std/utilities/format/format.functions/format_to_n.locale.pass.cpp
libcxx/test/std/utilities/format/format.functions/format_to_n.pass.cpp
libcxx/test/std/utilities/format/format.functions/formatted_size.locale.pass.cpp
libcxx/test/std/utilities/format/format.functions/formatted_size.pass.cpp
libcxx/test/std/utilities/format/format.functions/locale-specific_form.pass.cpp
libcxx/test/std/utilities/format/format.functions/vformat.locale.pass.cpp
libcxx/test/std/utilities/format/format.functions/vformat.pass.cpp
libcxx/test/std/utilities/format/format.functions/vformat_to.locale.pass.cpp
libcxx/test/std/utilities/format/format.functions/vformat_to.pass.cpp
libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.create/make_unique.sizezero.pass.cpp
libcxx/test/std/utilities/tuple/tuple.tuple/tuple.cnstr/deduct.pass.cpp
libcxx/test/std/utilities/utility/pairs/pairs.pair/implicit_deduction_guides.pass.cpp
libcxx/utils/ci/run-buildbot
libcxxabi/test/catch_member_function_pointer_02.pass.cpp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D126667.437952.patch
Type: text/x-patch
Size: 18258 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20220617/c5583760/attachment-0001.bin>
More information about the libcxx-commits
mailing list