[libc++][test] Should avoid preprocessor directives in macro argument lists #73836
Comments
|
After looking at these tests a second time (as they helpfully found bugs where MSVC's STL rejects format specifiers), I see what they're doing - they're crafting a big format string to check a whole bunch of specifiers simultaneously, but then they need to vary some of the expected output per-platform. Even aside from the preprocessor issue, this is pretty hard to read, and it makes diagnosing problems much harder because compiler errors (or runtime assertions) will point to the whole blob. I think it would be better if these tests were decomposed into separate checks for each format specifier and expected substring, which would also make it much easier to vary a few expected results with conformant preprocessing. (Decomposing one test like this allowed me to discover that MSVC was rejecting This is a special case of the general principle that |
Found while running libc++'s tests with MSVC's STL. * Avoid MSVC warning C5101: use of preprocessor directive in function-like macro argument list is undefined behavior. + We can easily make this portable by extracting `const bool is_newlib`. + Followup to #73440. + See #73598. + See #73836. * Avoid MSVC warning C4267: 'return': conversion from 'size_t' to 'int', possible loss of data. + This warning is valid, but harmless for the test, so `static_cast<int>` will avoid it. * Avoid MSVC warning C4146: unary minus operator applied to unsigned type, result still unsigned. + This warning is also valid (the scenario is sometimes intentional, but surprising enough that it's worth warning about). This is a C++17 test, so we can easily avoid it by testing `is_signed_v` at compile-time before testing `m < 0` and `n < 0` at run-time. * Silence MSVC warning C4310: cast truncates constant value. + These warnings are being emitted by `T(255)`. Disabling the warning is simpler than attempting to restructure the code. + Followup to #79791. * MSVC no longer emits warning C4521: multiple copy constructors specified. + This warning was removed from the compiler, since at least 2021-12-09.
Found while running libc++'s tests with MSVC's STL. * Avoid MSVC warning C5101: use of preprocessor directive in function-like macro argument list is undefined behavior. + We can easily make this portable by extracting `const bool is_newlib`. + Followup to llvm#73440. + See llvm#73598. + See llvm#73836. * Avoid MSVC warning C4267: 'return': conversion from 'size_t' to 'int', possible loss of data. + This warning is valid, but harmless for the test, so `static_cast<int>` will avoid it. * Avoid MSVC warning C4146: unary minus operator applied to unsigned type, result still unsigned. + This warning is also valid (the scenario is sometimes intentional, but surprising enough that it's worth warning about). This is a C++17 test, so we can easily avoid it by testing `is_signed_v` at compile-time before testing `m < 0` and `n < 0` at run-time. * Silence MSVC warning C4310: cast truncates constant value. + These warnings are being emitted by `T(255)`. Disabling the warning is simpler than attempting to restructure the code. + Followup to llvm#79791. * MSVC no longer emits warning C4521: multiple copy constructors specified. + This warning was removed from the compiler, since at least 2021-12-09.
In #73440 @StephanTLavavej fixes some of these issue, but the files
std/time/time.syn/formatter.duration.pass.cpp
std/time/time.syn/formatter.file_time.pass.cpp
std/time/time.syn/formatter.hh_mm_ss.pass.cpp
std/time/time.syn/formatter.local_time.pass.cpp
std/time/time.syn/formatter.sys_time.pass.cpp
std/time/time.syn/formatter.year.pass.cpp
std/time/time.syn/formatter.year_month.pass.cpp
std/time/time.syn/formatter.year_month_day_last.pass.cpp
std/time/time.syn/formatter.year_month_weekday.pass.cpp
Should be fixed too.
The text was updated successfully, but these errors were encountered: