llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.02k stars 11.96k forks source link

[libc++][test] Should avoid preprocessor directives in macro argument lists #73836

Open mordante opened 11 months ago

mordante commented 11 months ago

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.

StephanTLavavej commented 11 months ago

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 "%X" for duration, for example.)

This is a special case of the general principle that assert(A); followed by assert(B); is better than assert(A && B); in tests.