[PATCH] D27540: [libcxx] [test] Fix MSVC warning C4244 "conversion from 'X' to 'Y', possible loss of data", part 3/7.

2016-12-08 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.

Huh. Thanks for the analysis. LGTM.


https://reviews.llvm.org/D27540



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D27540: [libcxx] [test] Fix MSVC warning C4244 "conversion from 'X' to 'Y', possible loss of data", part 3/7.

2016-12-08 Thread Stephan T. Lavavej via Phabricator via cfe-commits
STL_MSFT added a comment.

I checked, and Clang 3.8.0 behaves identically to C1XX in the pair scenario. 
Here's the test case:

  #include 
  
  template  struct Pair {
  A a;
  B b;
  
  template  Pair(X&& x, Y&& y) : 
a(std::forward(x)), b(std::forward(y)) { }
  };
  
  int main() {
  Pair p(11, 22);
  return p.a == 11 && p.b == 22 ? 0 : 1;
  }

With `-Wconversion`, this emits:

  prog.cc:7:62: warning: implicit conversion loses integer precision: 'int' to 
'short' [-Wconversion]
  template  Pair(X&& x, Y&& y) : 
a(std::forward(x)), b(std::forward(y)) { }
  
~^~
  prog.cc:11:24: note: in instantiation of function template specialization 
'Pair::Pair' requested here
  Pair p(11, 22);
 ^
  prog.cc:7:85: warning: implicit conversion loses integer precision: 'int' to 
'short' [-Wconversion]
  template  Pair(X&& x, Y&& y) : 
a(std::forward(x)), b(std::forward(y)) { }

 ~^~

So, if you were compiling your tests with `-Wconversion` and not suppressing 
warnings in "system headers", you'd see this in libc++ too.


https://reviews.llvm.org/D27540



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D27540: [libcxx] [test] Fix MSVC warning C4244 "conversion from 'X' to 'Y', possible loss of data", part 3/7.

2016-12-08 Thread Stephan T. Lavavej via Phabricator via cfe-commits
STL_MSFT added a comment.

I can't possibly defend C1XX's behavior in your foo() scenario - but the pair 
scenarios being fixed here are different. pair's constructor from (X&&, 
Y&&) is perfect forwarding, so while the compiler can see that literals are 
being passed to (int&&, int&&), when it instantiates the constructor's 
definition, it just sees short being constructed from a perfectly-forwarded 
int, and warns about truncation there. C1XX doesn't attempt to "see through" 
the function call and notice that the arguments were literals. Its behavior is 
arguably desirable because the instantiation happens once per TU, and yet there 
may be other calls that are passing runtime-valued ints.

I chose static_cast here because it was non-invasive, although ugly. 
There's a more invasive but prettier possibility - change the shorts to longs 
so that widening instead of truncation happens. I could do that if you want.


https://reviews.llvm.org/D27540



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D27540: [libcxx] [test] Fix MSVC warning C4244 "conversion from 'X' to 'Y', possible loss of data", part 3/7.

2016-12-07 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF added a comment.

> MSVC seriously emits a warning for void foo(short); foo(0); because the 
> literal 0 is an int?

Oh my goodness it does... Your compiler is bad and it should feel bad. It's not 
like you can write `foo(0s)`.


https://reviews.llvm.org/D27540



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D27540: [libcxx] [test] Fix MSVC warning C4244 "conversion from 'X' to 'Y', possible loss of data", part 3/7.

2016-12-07 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF added a comment.

MSVC seriously emits a warning for `void foo(short); foo(0);` because the 
literal `0` is an int? If so you really should fix that in MSVC; That's a bogus 
warning. Does it emit a warning for `short x = 3;`?


https://reviews.llvm.org/D27540



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D27540: [libcxx] [test] Fix MSVC warning C4244 "conversion from 'X' to 'Y', possible loss of data", part 3/7.

2016-12-07 Thread Stephan T. Lavavej via Phabricator via cfe-commits
STL_MSFT created this revision.
STL_MSFT added reviewers: EricWF, mclow.lists.
STL_MSFT added a subscriber: cfe-commits.

[libcxx] [test] Fix MSVC warning C4244 "conversion from 'X' to 'Y', possible 
loss of data", part 3/7.

Add static_cast when constructing pair from (Something, 
int).


https://reviews.llvm.org/D27540

Files:
  
test/std/containers/unord/unord.map/unord.map.modifiers/insert_hint_rvalue.pass.cpp
  test/std/containers/unord/unord.map/unord.map.modifiers/insert_rvalue.pass.cpp
  
test/std/containers/unord/unord.multimap/unord.multimap.modifiers/insert_hint_rvalue.pass.cpp
  
test/std/containers/unord/unord.multimap/unord.multimap.modifiers/insert_rvalue.pass.cpp
  test/std/utilities/utility/pairs/pair.astuple/get_const.pass.cpp
  test/std/utilities/utility/pairs/pair.astuple/get_const_rv.pass.cpp
  test/std/utilities/utility/pairs/pair.astuple/get_non_const.pass.cpp
  test/std/utilities/utility/pairs/pair.astuple/get_rv.pass.cpp
  test/std/utilities/utility/pairs/pairs.pair/assign_const_pair_U_V.pass.cpp
  test/std/utilities/utility/pairs/pairs.pair/assign_rv_pair_U_V.pass.cpp
  test/std/utilities/utility/pairs/pairs.pair/const_pair_U_V_cxx03.pass.cpp
  test/std/utilities/utility/pairs/pairs.pair/copy_ctor.pass.cpp
  test/std/utilities/utility/pairs/pairs.pair/move_ctor.pass.cpp
  test/std/utilities/utility/pairs/pairs.pair/swap.pass.cpp
  test/std/utilities/utility/pairs/pairs.spec/comparison.pass.cpp
  test/std/utilities/utility/pairs/pairs.spec/non_member_swap.pass.cpp

Index: test/std/utilities/utility/pairs/pairs.spec/non_member_swap.pass.cpp
===
--- test/std/utilities/utility/pairs/pairs.spec/non_member_swap.pass.cpp
+++ test/std/utilities/utility/pairs/pairs.spec/non_member_swap.pass.cpp
@@ -20,8 +20,8 @@
 {
 {
 typedef std::pair P1;
-P1 p1(3, 4);
-P1 p2(5, 6);
+P1 p1(3, static_cast(4));
+P1 p2(5, static_cast(6));
 swap(p1, p2);
 assert(p1.first == 5);
 assert(p1.second == 6);
Index: test/std/utilities/utility/pairs/pairs.spec/comparison.pass.cpp
===
--- test/std/utilities/utility/pairs/pairs.spec/comparison.pass.cpp
+++ test/std/utilities/utility/pairs/pairs.spec/comparison.pass.cpp
@@ -27,8 +27,8 @@
 {
 {
 typedef std::pair P;
-P p1(3, 4);
-P p2(3, 4);
+P p1(3, static_cast(4));
+P p2(3, static_cast(4));
 assert( (p1 == p2));
 assert(!(p1 != p2));
 assert(!(p1 <  p2));
@@ -38,8 +38,8 @@
 }
 {
 typedef std::pair P;
-P p1(2, 4);
-P p2(3, 4);
+P p1(2, static_cast(4));
+P p2(3, static_cast(4));
 assert(!(p1 == p2));
 assert( (p1 != p2));
 assert( (p1 <  p2));
@@ -49,8 +49,8 @@
 }
 {
 typedef std::pair P;
-P p1(3, 2);
-P p2(3, 4);
+P p1(3, static_cast(2));
+P p2(3, static_cast(4));
 assert(!(p1 == p2));
 assert( (p1 != p2));
 assert( (p1 <  p2));
@@ -60,8 +60,8 @@
 }
 {
 typedef std::pair P;
-P p1(3, 4);
-P p2(2, 4);
+P p1(3, static_cast(4));
+P p2(2, static_cast(4));
 assert(!(p1 == p2));
 assert( (p1 != p2));
 assert(!(p1 <  p2));
@@ -71,8 +71,8 @@
 }
 {
 typedef std::pair P;
-P p1(3, 4);
-P p2(3, 2);
+P p1(3, static_cast(4));
+P p2(3, static_cast(2));
 assert(!(p1 == p2));
 assert( (p1 != p2));
 assert(!(p1 <  p2));
@@ -84,8 +84,8 @@
 #if TEST_STD_VER > 11
 {
 typedef std::pair P;
-constexpr P p1(3, 4);
-constexpr P p2(3, 2);
+constexpr P p1(3, static_cast(4));
+constexpr P p2(3, static_cast(2));
 static_assert(!(p1 == p2), "");
 static_assert( (p1 != p2), "");
 static_assert(!(p1 <  p2), "");
Index: test/std/utilities/utility/pairs/pairs.pair/swap.pass.cpp
===
--- test/std/utilities/utility/pairs/pairs.pair/swap.pass.cpp
+++ test/std/utilities/utility/pairs/pairs.pair/swap.pass.cpp
@@ -29,8 +29,8 @@
 {
 {
 typedef std::pair P1;
-P1 p1(3, 4);
-P1 p2(5, 6);
+P1 p1(3, static_cast(4));
+P1 p2(5, static_cast(6));
 p1.swap(p2);
 assert(p1.first == 5);
 assert(p1.second == 6);
Index: test/std/utilities/utility/pairs/pairs.pair/move_ctor.pass.cpp
===
--- test/std/utilities/utility/pairs/pairs.pair/move_ctor.pass.cpp
+++ test/std/utilities/utility/pairs/pairs.pair/move_ctor.pass.cpp
@@ -31,7 +31,7 @@
 {
 typedef std::pair P1;