[llvm] r187313 - [APFloat] Move setting fcNormal in zeroSignificand() to calling code.

[Michael Gottesman]

On Jul 27, 2013, at 6:34 PM, Chandler Carruth <chandlerc at google.com> wrote:

> For this and some of the other APFloat changes, I would really like to see unit tests. It seems especially important given the fragility of the APFloat class.
> 
I agree that unit tests for APFloat are important. As such, I have been making an effort in the past bit to fix that especially with respect to the arithmetic operations of APFloat. That being said do you have any specific revisions in mind besides this one? I am asking because I think I have been pretty diligent about committing unit tests, but if I missed anything I would like to fix the issue = ).

With regards to this specific commit, before I made this commit I actually thought about whether or not to commit a unit test. I reached the conclusion to not do so since APFloat::zeroSignificand is a private method and:

1. According to the GTest Advanced Guide (http://code.google.com/p/googletest/wiki/AdvancedGuide#Testing_Private_Code), one should only test public methods of a class.
2. From what I can tell the way to unit test a private method with gtest is to include some sort of conditional compilation in the header (so you can make the test class a friend class when testing but not include the text when including the file normally) and after looking a bit through LLVM I did not see any precedent of the sort.

(As always if I misspoke above please correct me. I do not claim to be a guru in how gtest works/gtest culture/its usage in LLVM).

On the other hand, if the community is ok with creating the precedent, I am more than happy to commit the necessary code so I can write a unit test for zeroSignificand = ).

Tell me what you think,
Michael

> 
> On Jul 27, 2013 2:53 PM, "Michael Gottesman" <mgottesman at apple.com> wrote:
> Author: mgottesman
> Date: Sat Jul 27 16:49:21 2013
> New Revision: 187313
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=187313&view=rev
> Log:
> [APFloat] Move setting fcNormal in zeroSignificand() to calling code.
> 
> Zeroing the significand of a floating point number does not necessarily cause a
> floating point number to become finite non zero. For instance, if one has a NaN,
> zeroing the significand will cause it to become +/- infinity.
> 
> Modified:
>     llvm/trunk/lib/Support/APFloat.cpp
> 
> Modified: llvm/trunk/lib/Support/APFloat.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APFloat.cpp?rev=187313&r1=187312&r2=187313&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/APFloat.cpp (original)
> +++ llvm/trunk/lib/Support/APFloat.cpp Sat Jul 27 16:49:21 2013
> @@ -778,6 +778,7 @@ APFloat::bitwiseIsEqual(const APFloat &r
>  APFloat::APFloat(const fltSemantics &ourSemantics, integerPart value) {
>    initialize(&ourSemantics);
>    sign = 0;
> +  category = fcNormal;
>    zeroSignificand();
>    exponent = ourSemantics.precision - 1;
>    significandParts()[0] = value;
> @@ -845,7 +846,6 @@ APFloat::significandParts()
>  void
>  APFloat::zeroSignificand()
>  {
> -  category = fcNormal;
>    APInt::tcSet(significandParts(), 0, partCount());
>  }
> 
> @@ -2301,9 +2301,9 @@ APFloat::convertFromHexadecimalString(St
>  {
>    lostFraction lost_fraction = lfExactlyZero;
> 
> +  category = fcNormal;
>    zeroSignificand();
>    exponent = 0;
> -  category = fcNormal;
> 
>    integerPart *significand = significandParts();
>    unsigned partsCount = partCount();
> @@ -2512,6 +2512,7 @@ APFloat::convertFromDecimalString(String
>               (D.normalizedExponent + 1) * 28738 <=
>                 8651 * (semantics->minExponent - (int) semantics->precision)) {
>      /* Underflow to zero and round.  */
> +    category = fcNormal;
>      zeroSignificand();
>      fs = normalize(rounding_mode, lfLessThanHalf);
> 
> @@ -3398,6 +3399,7 @@ APFloat APFloat::getSmallestNormalized(c
>    //   exponent = 0..0
>    //   significand = 10..0
> 
> +  Val.category = fcNormal;
>    Val.zeroSignificand();
>    Val.sign = Negative;
>    Val.exponent = Sem.minExponent;
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130729/8e828b53/attachment.html>