FormatTimeInMillisAsSeconds in gtest/src/gtest.cc:3504 causes compile errors when using conversion checking

[GoogleCodeExporter]

Using gcc 4.8.2 with option -Wconversion causes compile errors in the above 
source. Attached a patch.

Original issue reported on code.google.com by ChristTh...@gmail.com on 7 Mar 2015 at 3:57

Attachments:

• patch

[GoogleCodeExporter]

This was valid C++, relying on implicit conversion to double.
I don't know that we need to be clear of all possible warnings on all possible 
compilers.

-Wconversion is not included in -Wall or -Wextra because it triggers so often 
on safe code.

https://gcc.gnu.org/wiki/NewWconversion#Frequently_Asked_Questions

"Implicit conversions are very common in C. This tied with the fact that there 
is no data-flow in front-ends (see next question) results in hard to avoid 
warnings for perfectly working and valid code. Wconversion is designed for a 
niche of uses (security audits, porting 32 bit code to 64 bit, etc.) where the 
programmer is willing to accept and workaround invalid warnings. Therefore, it 
shouldn't be enabled if it is not explicitly requested."

Original comment by billydon...@google.com on 7 Mar 2015 at 4:05

[GoogleCodeExporter]

Understood. But do you think the patch I suggested would be wrong ? If not, 
wouldn't it be ok to include it and make life easier for those that do use the 
compiler option during 32-bit to 64-bit port ?

Original comment by ChristTh...@gmail.com on 7 Mar 2015 at 4:11

[GoogleCodeExporter]

If you're using that compiler option, it is emitting warnings, not errors.

There are indeed 64-bit values of 'ms', representing intervals of about 150k+ 
years, for which the conversion to double will be lossy. The printing of those 
intervals will be similarly lossy, as stringstream will print only 6 places of 
precision by default. So the change i correct.

I don't want to commit to keeping gtest free of such warnings, though.

Original comment by billydon...@google.com on 7 Mar 2015 at 7:19