This is an archive of the discontinued LLVM Phabricator instance.

[Clang][unittests] Silence trucation warning with MSVC
ClosedPublic

Authored by aganea on Jul 5 2022, 10:42 AM.

Details

Summary

I'm seeing the following with MSVC 2022:

C:\git\llvm-project\clang\unittests\StaticAnalyzer\RangeSetTest.cpp(97): warning C4309: 'initializing': truncation of constant value
C:\git\llvm-project\clang\unittests\StaticAnalyzer\RangeSetTest.cpp(948): note: see reference to class template instantiation '`anonymous-namespace'::TestValues<F>' being compiled
C:\git\llvm-project\clang\unittests\StaticAnalyzer\RangeSetTest.cpp(942): note: while compiling class template member function 'void `anonymous-namespace'::RangeSetCastToPromotionConversionTest_Test_Test<T>::TestBody(void)'

The issue here is that RangeSetCastToPromotionConversionTest tries to instantiate the template at L947, using TV = TestValues<F>; which ends up trying to assign at L94, static constexpr T ToA = FromA + 2; which overflows with T=char. Ideally, we wouldn't need FromA/ToA/FromB/ToB for this test.

Diff Detail

Event Timeline

aganea created this revision.Jul 5 2022, 10:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 5 2022, 10:42 AM
aganea requested review of this revision.Jul 5 2022, 10:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 5 2022, 10:42 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Isn't it better to silence this warning with a pragma instead of disabling it for the whole file?

aganea updated this revision to Diff 442366.Jul 5 2022, 11:28 AM

Use #pragma

ASDenysPetrov accepted this revision.EditedJul 5 2022, 11:44 AM

@aganea Thank you for fixing this.
TestValues structure implies to hold a set of values which can do some kind of convertions including truncations. This is what tests are about. That's true, it may happen that some test cases don't need some values. You can carry them out to some TestValues2 structure and use them instead. That also would work. So it's up to you. I'm OK with both solutions.

This revision is now accepted and ready to land.Jul 5 2022, 11:44 AM
thieta accepted this revision.Jul 5 2022, 1:54 PM

Thanks for switching to pragma!

aganea added a comment.Jul 5 2022, 5:31 PM

Thanks all for reviewing!

This revision was landed with ongoing or failed builds.Jul 5 2022, 5:34 PM
This revision was automatically updated to reflect the committed changes.