Skip to content

[clang][headers][Apple] Don't include_next float.h to avoid an unnecessary module dependency #137432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 29, 2025

Conversation

ian-twilightcoder
Copy link
Contributor

float.h doesn't define anything in Apple's SDKs that the clang float.h doesn't undefine, so all the include_next does is add an unnecessary module dependency. Skip the include_next and completely shadow the SDK header.

…ssary module dependency

float.h doesn't define anything in Apple's SDKs that the clang float.h doesn't undefine, so all the include_next does is add an unnecessary module dependency. Skip the include_next and completely shadow the SDK header.
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics labels Apr 26, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 26, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-backend-x86

Author: Ian Anderson (ian-twilightcoder)

Changes

float.h doesn't define anything in Apple's SDKs that the clang float.h doesn't undefine, so all the include_next does is add an unnecessary module dependency. Skip the include_next and completely shadow the SDK header.


Full diff: https://github.com/llvm/llvm-project/pull/137432.diff

1 Files Affected:

  • (modified) clang/lib/Headers/float.h (+2-11)
diff --git a/clang/lib/Headers/float.h b/clang/lib/Headers/float.h
index e5c439a9d47ae..84551af473b28 100644
--- a/clang/lib/Headers/float.h
+++ b/clang/lib/Headers/float.h
@@ -18,21 +18,12 @@
  * additional definitions provided for Windows.
  * For more details see http://msdn.microsoft.com/en-us/library/y0ybw9fy.aspx
  *
- * Also fall back on Darwin and AIX to allow additional definitions and
+ * Also fall back on AIX to allow additional definitions and
  * implementation-defined values.
  */
-#if (defined(__APPLE__) || defined(__MINGW32__) || defined(_MSC_VER) ||        \
-     defined(_AIX)) &&                                                         \
+#if (defined(__MINGW32__) || defined(_MSC_VER) || defined(_AIX)) &&            \
     __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>
 
 /* Undefine anything that we'll be redefining below. */

@ian-twilightcoder
Copy link
Contributor Author

This was originally added in d93779d / d89a1eb (can't find either of these in Phabricator) for _HAS_SUBNORM, but that's since been obsoleted by https://reviews.llvm.org/D37302. So including this header just causes an unnecessary include and module build of Apple's float.h.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming @rjmccall or @ldionne have no concerns

@ian-twilightcoder ian-twilightcoder requested review from rjmccall and removed request for rjmccall April 28, 2025 16:23
Copy link
Member

@cyndyishida cyndyishida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT this seems like a good cleanup for detangling sdk dependencies where its safe to.

@rjmccall
Copy link
Contributor

Messing around with #include_next is scary, but the problems are always thorny reasons that only come up in intense testing; I don't see anything obviously wrong, and I trust @ian-twilightcoder to have done the investigation properly. LGTM.

@ian-twilightcoder ian-twilightcoder merged commit 557ddc2 into llvm:main Apr 29, 2025
15 checks passed
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…ssary module dependency (llvm#137432)

float.h doesn't define anything in Apple's SDKs that the clang float.h
doesn't undefine, so all the include_next does is add an unnecessary
module dependency. Skip the include_next and completely shadow the SDK
header.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…ssary module dependency (llvm#137432)

float.h doesn't define anything in Apple's SDKs that the clang float.h
doesn't undefine, so all the include_next does is add an unnecessary
module dependency. Skip the include_next and completely shadow the SDK
header.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…ssary module dependency (llvm#137432)

float.h doesn't define anything in Apple's SDKs that the clang float.h
doesn't undefine, so all the include_next does is add an unnecessary
module dependency. Skip the include_next and completely shadow the SDK
header.
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
…ssary module dependency (llvm#137432)

float.h doesn't define anything in Apple's SDKs that the clang float.h
doesn't undefine, so all the include_next does is add an unnecessary
module dependency. Skip the include_next and completely shadow the SDK
header.
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
…ssary module dependency (llvm#137432)

float.h doesn't define anything in Apple's SDKs that the clang float.h
doesn't undefine, so all the include_next does is add an unnecessary
module dependency. Skip the include_next and completely shadow the SDK
header.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 clang:headers Headers provided by Clang, e.g. for intrinsics clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants