This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] [test] Fix the put_double, put_long_double tests for clang-cl
ClosedPublic

Authored by mstorsjo on Feb 17 2022, 1:34 AM.

Details

Summary

These tests are hit hard by a bug that is fixed in a newer version
of UCRT. Add a test for the specific bug, and XFAIL the tests if
that bug is present (as it is in CI).

With that bug fixed, there's still one aspect in printf formatting that
differs, that requires ifdeffing out a couple parts of the test (hex
float formatting). Based on reading the C standard for printf formatting,
it seems like this isn't necessarily a proper bug in printf, but just a
case of differing optional behaviour.

(Layering this on top of D119930, to reuse the programSucceeds
function added there.)

Diff Detail

Event Timeline

mstorsjo created this revision.Feb 17 2022, 1:34 AM
mstorsjo requested review of this revision.Feb 17 2022, 1:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 17 2022, 1:34 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

This has got its dependencies (D119930) committed now, so the concept of tests for specific bugs should be vetted.

libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_double.pass.cpp
14–15
14292–14293

Any appetite to transplant these two tests into a separate file that could be XFAIL: msvc'ed? You could call it, like, put_double.hex.pass.cpp.

libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.pass.cpp
14–15
libcxx/utils/libcxx/test/features.py
99

I missed out on D119930; this approach seems reasonable to me too. But what would you think of giving all these Windows UCRT bugs a common prefix, something like win32-broken-utf8-wchar-ctype, win32-broken-printf-g-precision, and so on?
I assume msvc-broken-... would have the wrong connotation because this affects both msvc and clang-cl, right? Would win32-broken- have better connotations? msc-ucrt-broken-??

mstorsjo added inline comments.Feb 24 2022, 3:12 AM
libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_double.pass.cpp
14292–14293

I guess that sounds doable, I'll give that a shot.

libcxx/utils/libcxx/test/features.py
99

Sure, adding a win32- prefix to the bug feature name sounds good to me. I can make a separate review for changing the existing one.

mstorsjo updated this revision to Diff 411061.Feb 24 2022, 3:34 AM

Add a win32- prefix to the feature name, split the hex float formatting tests to separate files to ease XFAILing those specific bits.

mstorsjo updated this revision to Diff 411104.Feb 24 2022, 5:50 AM

Remove XFAIL: LIBCXX-AIX-FIXME from put_double.pass.cpp which now passes.

Quuxplusone accepted this revision.Feb 25 2022, 10:33 AM

LGTM! (1500 lines per test function... this code is just a mess. 😛 Thanks for dealing with it.)

This revision is now accepted and ready to land.Feb 25 2022, 10:33 AM

LGTM! (1500 lines per test function... this code is just a mess. 😛 Thanks for dealing with it.)

Thanks! I'll hold off of pushing this one (together with D120469 ) until next week, to give some time if someone objects to the feature naming style (win32-broken-...).

libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_double.pass.cpp