Hi Jorge, I responded to the initial commit with some comments here: http://reviews.llvm.org/rL260577
-- HT On Thu, Feb 11, 2016 at 7:53 PM, Jorge Teixeira <j.lopes.teixe...@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? > > > 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 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 <rich...@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.teixe...@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 <rich...@metafoo.co.uk> > wrote: > >>>>> On Tue, Feb 9, 2016 at 2:43 PM, Jorge Teixeira > >>>>> <j.lopes.teixe...@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 < > rich...@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@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.teixe...@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.reinterpretc...@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@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@lists.llvm.org > >>>>>>>>> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >>>>>>>>> >> > >>>>>>>>> > > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> _______________________________________________ > >>>>>>>> cfe-commits mailing list > >>>>>>>> cfe-commits@lists.llvm.org > >>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >>>>>>>> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits