[PATCH] D34695: _Float16 preprocessor macro definitions

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.


================
Comment at: lib/Headers/float.h:137
 
+#ifdef __STDC_WANT_IEC_60559_TYPES_EXT__
+#  define FLT16_MANT_DIG  __FLT16_MANT_DIG__
----------------
scanon wrote:
> rogfer01 wrote:
> > scanon wrote:
> > > rogfer01 wrote:
> > > > My understanding is that, given that we support TS18661-2 by default, this macro should be predefined by clang and then there is no need to protect these macros.
> > > > 
> > > > You may want to add a test for this in `test/Preprocessor/init.c`.
> > > Where do you see that the `__STDC_WANT_IEC_60559_TYPES_EXT__` macro should be predefined by clang?
> > Hi Steve,
> > 
> > certainly you're right, the TS says
> > 
> > > The new identifiers added to C11 library headers by this part of ISO/IEC TS-18661 are defined or declared by their respective headers only if `__STDC_WANT_IEC_60559_TYPES_EXT__` is defined as a macro at the point in the source file where the appropriate header is first included.
> > 
> > so (if I read this right) these identifiers are only available if such macro is defined when including `float.h`. 
> > 
> > Can I assume from your comment that someone else should define it? Perhaps the `float.h` header itself, some other file in the C-library implementation or the user of the compiler via some `-D__STDC_WANT_IEC_60559_TYPES_EXT__`, but not be predefined by the compiler? If this is the case, then the macros still have to be guarded conditionally (as they were in the original patch).
> > 
> > Does this make sense? Thanks.
> I think we could justify defining it ourselves under non-strict compilation modes; alternatively, system headers might define it for users in non-strict modes.
> 
> My reading of the TS is that in strict mode, these types and macros should be hidden unless the user explicitly requests them by defining `__STDC_WANT_IEC_60559_TYPES_EXT__` themselves.
Thanks, very useful discussion and clarification. I will add some tests for this, which I indeed forgot. Cheers.


https://reviews.llvm.org/D34695