search

Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

float.h not working on mingw #40172

Open Quuxplusone opened 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR41202
Status NEW
Importance P normal
Reported by Markus Böck (markus.boeck02@gmail.com)
Reported on 2019-03-22 07:08:19 -0700
Last modified on 2019-03-25 06:08:52 -0700
Version 8.0
Hardware PC Windows NT
CC blitzrakete@gmail.com, craig.topper@gmail.com, dgregor@apple.com, erik.pilkington@gmail.com, ldionne@apple.com, llvm-bugs@lists.llvm.org, martin@martin.st, richard-llvm@metafoo.co.uk, rnk@google.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also 
After compiling clang 8 using mingw gcc 8.1 I wanted to compile one of my
projects but it failed to compile as there is an error when including the file
float.h. Tried the same with Clang 7 and Clang 7.0.1 and did not have any
problems.

Turns out the problem is something with the preprocessor:
#if (__GNUC__ < 4  || (__GNUC__ == 4 && __GNUC_MINOR__ < 6)) \
    || (__clang_major__ >=3)
#if !defined(_FLOAT_H___) && !defined(__FLOAT_H)
#include_next <float.h>
#endif

Clang 7 and 7.0.1 for some reason do not enable the region which has
#include_next yet clang 8 does and fails as it does not fit a next float.h
Quuxplusone commented 5 years ago

Clang has this in its copy of float.h (note the mingw bit):

/* If we're on MinGW, fall back to the system's float.h, which might have
 * additional definitions provided for Windows.
 * For more details see http://msdn.microsoft.com/en-us/library/y0ybw9fy.aspx
 *
 * Also fall back on Darwin to allow additional definitions and
 * implementation-defined values.
 */
#if (defined(__APPLE__) || (defined(__MINGW32__) || defined(_MSC_VER))) && \
    __STDC_HOSTED__ && __has_include_next(<float.h>)

/* Prior to Apple's 10.7 SDK, float.h SDK header used to apply an extra level
 * of #include_next<float.h> to keep Metrowerks compilers happy. Avoid this
 * extra indirection.
 */
#ifdef __APPLE__
#define _FLOAT_H_
#endif

#  include_next <float.h>
...

I conclude that there are too many cooks in the kitchen trying to own float.h. >_>

It looks like the header guard was changed in r339016:

commit 58529c3f5739c7bdb74da8c6ce3e2d16672e9315 Author: Louis Dionne ldionne@apple.com Date: Mon Aug 6 14:29:47 2018 +0000

[clang] Fix broken include_next in float.h

Summary:
The code defines __FLOAT_H and then includes the next <float.h>, which is
guarded on __FLOAT_H so it gets skipped entirely. This commit uses the header
guard __CLANG_FLOAT_H, like other headers (such as limits.h) do.

Reviewers: jfb

Subscribers: dexonsmith, cfe-commits

Differential Revision: https://reviews.llvm.org/D50276

llvm-svn: 339016

diff --git a/clang/lib/Headers/float.h b/clang/lib/Headers/float.h index 44d4d05494f..c04cbbf47eb 100644 --- a/clang/lib/Headers/float.h +++ b/clang/lib/Headers/float.h @@ -21,8 +21,8 @@ ===-----------------------------------------------------------------------=== /

-#ifndef FLOAT_H -#define __FLOAT_H +#ifndef CLANG_FLOAT_H +#define __CLANG_FLOAT_H

/* If we're on MinGW, fall back to the system's float.h, which might have

-#endif / __FLOAT_H / +#endif / __CLANG_FLOAT_H /

Quuxplusone commented 5 years ago

This is fixed in mingw-w64 trunk in this commit: https://sourceforge.net/p/mingw-w64/mingw-w64/ci/82b169c5734a6198d3b4c51a48f82e7b7104f143/

I conclude that there are too many cooks in the kitchen trying to own float.h.

That's very true...

Quuxplusone commented 5 years ago

Do you think the fix I made to float.h was incorrect? It seems to me that the fix is correct, but the MinGW headers are brittle in relying on the include guard provided by the compiler (not that Clang's headers are much more robust). Maybe I'm not aware of some implicit contract between those headers.