This is an archive of the discontinued LLVM Phabricator instance.

[libc++][test] Miscellaneous MSVC cleanups
ClosedPublic

Authored by CaseyCarter on Oct 8 2019, 6:56 PM.

Details

Summary
  • Silence unused-local-typedef warnings: map.cons/assign_initializer_list.pass.cpp (and the set.cons variant) uses a local typedef only within LIBCPP_ASSERTs, so clang diagnoses it as unused when testing non-libc++.
  • Add missing include: c.math/abs.pass.cpp uses std::numeric_limits but failed to #include <limits>.
  • Don't test non-type: A "recent" change to meta.trans.other/underlying_type.pass.cpp unconditionally tests the type F which is conditionally defined.
  • Use hash<long long> instead of hash<short> with int in unordered_meow deduction guide tests to avoid truncation warnings.
  • Convert 3.14 explicitly in midpoint.float.pass since MSVC incorrectly diagnoses float meow = 3.14; as truncating.

Diff Detail

Event Timeline

CaseyCarter created this revision.Oct 8 2019, 6:56 PM
Quuxplusone added inline comments.
test/std/containers/unord/unord.map/unord.map.cnstr/deduct.pass.cpp
70 ↗(On Diff #223974)

Alternatively, if you wanted to change the instances of hash<short> to hash<>, that would be fine with me. (Just as long as it's something distinguishable from the default of hash<int>.)

CaseyCarter added inline comments.Oct 9 2019, 10:15 AM
test/std/containers/unord/unord.map/unord.map.cnstr/deduct.pass.cpp
70 ↗(On Diff #223974)

I do like the idea of avoiding rather than suppressing the warning. I'll change these tests to use hash<long long> instead of hash<short>, which avoids truncating conversions and preserves the "neither int nor long" property which provides some assurance the deduction guides aren't getting the type from the key type or mapped type.

CaseyCarter edited the summary of this revision. (Show Details)

Avoid rather than suppress truncation warnings in unordered_meow deduction guide tests.

CaseyCarter marked 2 inline comments as done.Oct 9 2019, 10:19 AM
test/std/containers/unord/unord.map/unord.map.cnstr/deduct.pass.cpp
70 ↗(On Diff #223974)

Awesome. The CTAD tests LGTM! No opinion on the rest.

(It occurs to me that CTAD makes std::hash() mean std::hash<void>(), so maybe I should have avoided hash<void> and equal_to<void> in these tests to begin with.)

ldionne accepted this revision.Oct 9 2019, 1:30 PM

You have commit access, right? If so, go ahead. Otherwise, LMK and I can commit this for you.

This revision is now accepted and ready to land.Oct 9 2019, 1:30 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 9 2019, 3:24 PM
Herald added a subscriber: christof. · View Herald Transcript

You have commit access, right? If so, go ahead. Otherwise, LMK and I can commit this for you.

Yes, I apparently still have commit access ;) Thanks!