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

[Richard Smith via cfe-commits]

On Thu, Feb 11, 2016 at 3:42 PM, Jorge Teixeira
<j.lopes.teixeira at gmail.com> wrote:
> Richard,
>
> Thanks and got it re: test filename and hosted mode.
>
> 1) AFAIK, C89/C90 does not define DECIMAL_DIG, so here is a revised
> patch, with minor comment modifications to explicitly list the (draft)
> normative references that I used.

You'll also need to change <float.h> to only provide DECIMAL_DIG in C99 onwards.

> 2) This might also be a good time to ask you what does clang aim for
> regarding C89/C90, since there are a few "updates" to it, namely
> Technical Corrigendum 1 (ISO/IEC 9899 TCOR1), 1994, Technical
> Corrigendum 2 (ISO/IEC 9899 TCOR2), 1996, and Normative Addendum (aka
> Amendment) 1, 1994 (sometimes known as C94/C95).

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).

> Also, please clarify
> how they all relate to __STRICT_ANSI__ and whether the use of
> __STDC_VERSION__ <= 199409L should be encouraged/avoided.
> I assume that for C99 and C11 clang aims for the latest approved TC,
> and possibly a few approved but yet unpublished Defect Reports.

That's correct.

__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.

Testing __STDC_VERSION__ for C94 makes sense if you're trying to
detect whether Amendment 1 features should be provided.

> I could not find the above info on any docs, so perhaps it could be
> added in www/compatibility.html.
> For completion purposes, the GCC extension modes gnu90, gnu99, etc.
> should also be mentioned, although anything other than "unsupported"
> would open a new can of worms.
>
> 3) Lastly, can you expand on the removal of the *_MIN_EXP comparison
> (http://reviews.llvm.org/rL260610)? From N1570, 6.2.5p10:
> "The set of values of the type float is a subset of the set of values
> of the type double; the set of values of the type double is a subset
> of the set of values of the type long double". So if a larger-width
> type must be able to represent all narrower-width types values, would
> it not also imply that the min normalized exp for larger width types
> must be either equal or more negative?

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.

> 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
>>>>>>>