Patch for Bug 26283: float.h is missing mandatory C11 fp macros like DBL_DECIMAL_DIG and LDBL_DECIMAL_DIG

Thu Feb 11 17:20:28 PST 2016

On Thu, Feb 11, 2016 at 4:53 PM, Jorge Teixeira
<j.lopes.teixeira at gmail.com> wrote:
>> You'll also need to change <float.h> to only provide DECIMAL_DIG in C99 onwards.
> Done!
>
>> All of our -std versions are that standard plus applicable Defect
>> Reports. So -std=c89 includes TC1 and TC2, but not Amendment 1 (we
>> have -std=c94 for that, but the only difference from our C89 mode is
>> the addition of digraphs).
> I'll try to find the c89 TC2 and check if anything changed regarding
> these macros (unlikely).
>
>> __STRICT_ANSI__ is defined if Clang has not been asked to provide
>> extensions (either GNU extensions, perhaps via a flag like -std=gnu99,
>> or MS extensions), and is used by C library headers to determine that
>> they should provide a strictly-conforming set of declarations without
>> extensions.
> Ok, so if !defined(__STRICT__ANSI__) clang should always expose "as
> much as possible", including stuff from later versions of the Std.
> and/or eventual extensions, just as it now on float.h and float.c,
> right?

Yes, that's the idea.

>> Testing __STDC_VERSION__ for C94 makes sense if you're trying to
>> detect whether Amendment 1 features should be provided.
> Since this will affect only digraphs, I guess there is no need (for
> float.h, float.c).
>
>>> 3) Lastly, can you expand (...)
>>
>> No, it does not mean that.
>>
>> For PPC64, long double is (sometimes) modeled as a pair of doubles.
>> Under that model, the smallest normalized value for long double is
>> actually larger than the smallest normalized value for double
>> (remember that for a normalized value with exponent E, all numbers of
>> the form 1.XXXXX * 2^E, with the right number of mantissa digits, are
>> exactly representable, so increasing the number of mantissa bits
>> without changing the number of exponent bits increases the magnitude
>> of the smallest normalized positive number).
>>
>> The set of values of long double in this model *is* a superset of the
>> set of values of double.
>>
> I see now, and removed the bogus tests. The patch should now test
> cleanly unless something needs DECIMAL_DIG but did not set the
> appropriate std. level, or defined __STRICT__ANSI__.

Thanks, committed as r260639.

(In future, it's preferable to submit a single patch for all affected
files rather than one patch per file.)

> Thanks for the learning experience,
>
> JT
>
>
>
>>> From /test/Preprocessor/init.cpp:
>>> // PPC64:#define __DBL_MIN_EXP__ (-1021)
>>> // PPC64:#define __FLT_MIN_EXP__ (-125)
>>> // PPC64:#define __LDBL_MIN_EXP__ (-968)
>>>
>>> This issue happened before
>>> (https://lists.gnu.org/archive/html/bug-gnulib/2011-08/msg00262.html,
>>> http://www.openwall.com/lists/musl/2013/11/15/1), but all it means is
>>> that ppc64 is not compliant with C without soft-float. The test is
>>> valid and should stay, and if someone tries to compile for ppc64 in
>>> c89, c99 or c11 modes, clang should 1) use soft float (bad idea), 2)
>>> issue a diagnostic saying that that arch cannot meet the desired C
>>> standard without a big performance penalty - the diag should be
>>> suppressible with some special cmd line argument.
>>> Thus, I added the tests back and the FAIL for PPC64 for the time
>>> being, with a comment. If you know of a way to skip only the specific
>>> *_MIN_EXP and *_MIN_10_EXP tests, please add it, because there might
>>> be more similar cases in the future.
>>>
>>> JT
>>>
>>> On Thu, Feb 11, 2016 at 3:04 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>>> Thanks, I modified the test to also test C89 and C99 modes and
>>>> committed this as r260577.
>>>>
>>>> On Thu, Feb 11, 2016 at 11:29 AM, Jorge Teixeira
>>>> <j.lopes.teixeira at gmail.com> wrote:
>>>>> Here is a revised test, which I renamed to c11-5_2_4_2_2p11.c instead
>>>>> of float.c because I am only checking a subset of what the standard
>>>>> mandates for float.h, and because there were similar precedents, like
>>>>> test/Preprocessor/c99-*.c. Feel free to override, though.
>>>>
>>>> test/Preprocessor/c99-* are an aberration. The goal would be that this
>>>> test grows to cover all of the parts of float.h that we define, so
>>>> float.c seems like the appropriate name for it.
>>>>
>>>>> The first part checks for basic compliance with the referred C11
>>>>> paragraph, the second for internal consistency between the underscored
>>>>> and exposed versions of the macros.
>>>>> No attempt was made to support C99 or C89.
>>>>>
>>>>> I am not very clear on the proper use of the whole lit.py / RUN
>>>>> framework, so someone should really confirm if what I wrote is
>>>>> correct. The goal was to test both hosted and freestanding
>>>>> implementations with C11, and expect no diagnostics from either.
>>>>
>>>> We generally avoid testing hosted mode, because we don't want the
>>>> success of our tests to depend on the libc installed on the host
>>>> system.
>>>>
>>>>> Thanks for the help,
>>>>>
>>>>> JT
>>>>>
>>>>> On Tue, Feb 9, 2016 at 5:56 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>>>>> On Tue, Feb 9, 2016 at 2:43 PM, Jorge Teixeira
>>>>>> <j.lopes.teixeira at gmail.com> wrote:
>>>>>>> Richard,
>>>>>>>
>>>>>>> Can you be more specific?
>>>>>>>
>>>>>>> I assume you mean something like my newly attached .h file that tests
>>>>>>> very basic implementation compliance (i.e., it's required, but not
>>>>>>> sufficient), but I would need a bit more guidance about the structure
>>>>>>> of the file, how to perform the tests, and where to exactly place and
>>>>>>> name the file within test/Headers.
>>>>>>>
>>>>>>> I some sort of template exists, or if someone else takes point and
>>>>>>> makes it, I can "port" the attached p11 test cases. I am unsure of how
>>>>>>> to perform a more normative compliance - for example, to assert that
>>>>>>> LDBL_DECIMAL_DIG is 21 on x86-64 and that indeed those many digits are
>>>>>>> guaranteed to be correct, etc. This is probably not possible / does
>>>>>>> not make sense.
>>>>>>
>>>>>> That looks like a decent basic test for this. The test should be named
>>>>>> something like test/Headers/float.c, and needs to contain a "RUN:"
>>>>>> line so that the test runner infrastructure knows how to run it. You
>>>>>> can look at test/Header/limits.cpp for an example of how this works.
>>>>>>
>>>>>> We already have platform-specific tests that __LDBL_DECIMAL_DIG__ is
>>>>>> the right value, so you could test the values are correct by checking
>>>>>> that LDBL_DECIMAL_DIG == __LDBL_DECIMAL_DIG__.
>>>>>>
>>>>>>> JT
>>>>>>>
>>>>>>> On Tue, Feb 9, 2016 at 3:58 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>>>>>>> Patch looks good. Please also add a testcase to test/Headers.
>>>>>>>>
>>>>>>>> On Tue, Feb 9, 2016 at 12:08 PM, Hubert Tong via cfe-commits
>>>>>>>> <cfe-commits at lists.llvm.org> wrote:
>>>>>>>>> I see no immediate issue with this patch, but I am not one of the usual
>>>>>>>>> reviewers for this part of the code base.
>>>>>>>>>
>>>>>>>>> -- HT
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Feb 9, 2016 at 2:56 PM, Jorge Teixeira <j.lopes.teixeira at gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> Thanks Hubert. Somehow I omitted that prefix when typing the macros,
>>>>>>>>>> and I did not noticed it when I was testing because on my arch
>>>>>>>>>> DECIMAL_DIG is defined to be the LDBL version...
>>>>>>>>>>
>>>>>>>>>> Updated patch is attached.
>>>>>>>>>>
>>>>>>>>>> JT
>>>>>>>>>>
>>>>>>>>>> On Tue, Feb 9, 2016 at 1:41 PM, Hubert Tong
>>>>>>>>>> <hubert.reinterpretcast at gmail.com> wrote:
>>>>>>>>>> > There is a __LDBL_DECIMAL_DIG__ predefined macro. __DECIMAL_DIG__ will
>>>>>>>>>> > not
>>>>>>>>>> > always be the same as __LDBL_DECIMAL_DIG__.
>>>>>>>>>> >
>>>>>>>>>> > -- HT
>>>>>>>>>> >
>>>>>>>>>> > On Mon, Feb 8, 2016 at 11:26 PM, Jorge Teixeira via cfe-commits
>>>>>>>>>> > <cfe-commits at lists.llvm.org> wrote:
>>>>>>>>>> >>
>>>>>>>>>> >> Hi, I filed the bug (https://llvm.org/bugs/show_bug.cgi?id=26283) some
>>>>>>>>>> >> time ago and nobody picked it up, so here is a trivial patch exposing
>>>>>>>>>> >> the missing macros, that to the best of my ability were already
>>>>>>>>>> >> present as the internal underscored versions.
>>>>>>>>>> >>
>>>>>>>>>> >> Perhaps a more general bug about C11 floating point (lack of)
>>>>>>>>>> >> conformance should be filed, so that some form of unit test/macro
>>>>>>>>>> >> validation could be worked on, but this patch does scratch my current
>>>>>>>>>> >> itch.
>>>>>>>>>> >>
>>>>>>>>>> >> Successfully tested on x86-64 Xubuntu 14.04 with clang 3.8 from the
>>>>>>>>>> >> ppa, patched with the attached diff.
>>>>>>>>>> >>
>>>>>>>>>> >> First contribution, so feel free to suggest improvements or point to
>>>>>>>>>> >> more detailed step-by-step instructions/guidelines.
>>>>>>>>>> >>
>>>>>>>>>> >> Cheers,
>>>>>>>>>> >>
>>>>>>>>>> >> JT
>>>>>>>>>> >>
>>>>>>>>>> >> _______________________________________________
>>>>>>>>>> >> cfe-commits mailing list
>>>>>>>>>> >> cfe-commits at lists.llvm.org
>>>>>>>>>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>>>>>> >>
>>>>>>>>>> >
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> _______________________________________________
>>>>>>>>> cfe-commits mailing list
>>>>>>>>> cfe-commits at lists.llvm.org
>>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>>>>>