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