[libcxx-commits] [openmp] [flang] [compiler-rt] [lld] [clang] [polly] [mlir] [libcxx] [clang-tools-extra] [libcxxabi] [lldb] [llvm] [libunwind] [libc] [libc++][numeric] P0543R3: Saturation arithmetic (PR #77967)

[Mark de Wever via libcxx-commits]

https://github.com/mordante requested changes to this pull request.

> I did some re-imagining of the tests, there is a some redundancy but it is cleared that nothing was missed.  The tests are formatted in tabular form manually to make them easier to read.

Redundant code in tests when if improves readability is not too bad. I really like how the new testing code looks. It was a lot easier to follow. Thanks a lot!

>  I also used the Clang 18's "Placeholder variables with no name" to make the test a bit cleaner, so I disabled the unsupported compilers.
> If the above is unacceptable, it can be fixed easily. For now it makes it easier to read and review.

IMO we should not disable tests for compilers, only for the reason to be able to use newer language features in the test. Note if the header requires language features I'm fine to disable that for a compiler. In this case we can easily allow clang-17 with a minor change as pointed out in the review.

The patch is getting close to be ready.



https://github.com/llvm/llvm-project/pull/77967