Skip to content
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

[flang] Fix Darwin build after 4762c6557d15 #84478

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

luporl
Copy link
Contributor

@luporl luporl commented Mar 8, 2024

Select POSIX 2008 standard to avoid including Darwin extensions.
Otherwise, Darwin's math.h header defines HUGE, which conflicts
with Flang's HUGE function.

This is a temporary build fix. More permanent solutions would be to
rename Flang's HUGE or to define _POSIX_C_SOURCE for Flang as a
whole.

This started happening after 4762c65 (#82443), that added the
"utility" include, which seems to include "math.h".

@llvmbot llvmbot added the flang Flang issues not falling into any other category label Mar 8, 2024
@luporl luporl requested a review from klausler March 8, 2024 13:20
Copy link
Contributor

@klausler klausler left a comment

Choose a reason for hiding this comment

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

If this is the best fix for Darwin, please apply it to the other sites in the code that have worked around this bug in the past, too.

flang/include/flang/Evaluate/integer.h:#undef HUGE
flang/include/flang/Evaluate/real.h:#undef HUGE
flang/lib/Evaluate/fold-implementation.h:#undef HUGE

Select POSIX 2008 standard to avoid including Darwin extensions.
Otherwise, Darwin's math.h header defines HUGE, which conflicts
with Flang's HUGE function.

This started happening after 4762c65 (llvm#82443), that added the
"utility" include, which seems to include "math.h".
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 8, 2024

@llvm/pr-subscribers-flang-semantics

Author: Leandro Lupori (luporl)

Changes

Select POSIX 2008 standard to avoid including Darwin extensions.
Otherwise, Darwin's math.h header defines HUGE, which conflicts
with Flang's HUGE function.

This is a temporary build fix. More permanent solutions would be to
rename Flang's HUGE or to define _POSIX_C_SOURCE for Flang as a
whole.

This started happening after 4762c65 (#82443), that added the
"utility" include, which seems to include "math.h".


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

5 Files Affected:

  • (modified) flang/CMakeLists.txt (+8)
  • (modified) flang/include/flang/Evaluate/integer.h (-4)
  • (modified) flang/include/flang/Evaluate/real.h (-4)
  • (modified) flang/lib/Evaluate/fold-implementation.h (-4)
  • (modified) flang/lib/Evaluate/intrinsics-library.cpp (+2-2)
diff --git a/flang/CMakeLists.txt b/flang/CMakeLists.txt
index 21617aeea0215e..71141e5efac488 100644
--- a/flang/CMakeLists.txt
+++ b/flang/CMakeLists.txt
@@ -413,6 +413,14 @@ if (LLVM_COMPILER_IS_GCC_COMPATIBLE)
 
 endif()
 
+# Clang on Darwin enables non-POSIX extensions by default, which allows the
+# macro HUGE to leak out of <math.h> even when it is never directly included,
+# conflicting with Flang's HUGE symbols.
+# Set _POSIX_C_SOURCE to avoid including these extensions.
+if (APPLE)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -D_POSIX_C_SOURCE=200809")
+endif()
+
 list(REMOVE_DUPLICATES CMAKE_CXX_FLAGS)
 
 # Determine HOST_LINK_VERSION on Darwin.
diff --git a/flang/include/flang/Evaluate/integer.h b/flang/include/flang/Evaluate/integer.h
index 2fce4bedfaee0f..977d35c7eecf48 100644
--- a/flang/include/flang/Evaluate/integer.h
+++ b/flang/include/flang/Evaluate/integer.h
@@ -27,10 +27,6 @@
 #include <string>
 #include <type_traits>
 
-// Some environments, viz. clang on Darwin, allow the macro HUGE
-// to leak out of <math.h> even when it is never directly included.
-#undef HUGE
-
 namespace Fortran::evaluate::value {
 
 // Implements an integer as an assembly of smaller host integer parts
diff --git a/flang/include/flang/Evaluate/real.h b/flang/include/flang/Evaluate/real.h
index 62c99cebc31684..5266bd0ef64bfd 100644
--- a/flang/include/flang/Evaluate/real.h
+++ b/flang/include/flang/Evaluate/real.h
@@ -18,10 +18,6 @@
 #include <limits>
 #include <string>
 
-// Some environments, viz. clang on Darwin, allow the macro HUGE
-// to leak out of <math.h> even when it is never directly included.
-#undef HUGE
-
 namespace llvm {
 class raw_ostream;
 }
diff --git a/flang/lib/Evaluate/fold-implementation.h b/flang/lib/Evaluate/fold-implementation.h
index 798bc5f37f6f42..6b3c9416724cb6 100644
--- a/flang/lib/Evaluate/fold-implementation.h
+++ b/flang/lib/Evaluate/fold-implementation.h
@@ -39,10 +39,6 @@
 #include <type_traits>
 #include <variant>
 
-// Some environments, viz. clang on Darwin, allow the macro HUGE
-// to leak out of <math.h> even when it is never directly included.
-#undef HUGE
-
 namespace Fortran::evaluate {
 
 // Utilities
diff --git a/flang/lib/Evaluate/intrinsics-library.cpp b/flang/lib/Evaluate/intrinsics-library.cpp
index e68c5ed3f6a8b1..7315a7a057b102 100644
--- a/flang/lib/Evaluate/intrinsics-library.cpp
+++ b/flang/lib/Evaluate/intrinsics-library.cpp
@@ -299,8 +299,8 @@ struct HostRuntimeLibrary<std::complex<HostT>, LibraryVersion::Libm> {
 /// Define libm extensions
 /// Bessel functions are defined in POSIX.1-2001.
 
-// Remove float bessel functions for AIX as they are not supported
-#ifndef _AIX
+// Remove float bessel functions for AIX and Darwin as they are not supported
+#if !defined(_AIX) && !defined(__APPLE__)
 template <> struct HostRuntimeLibrary<float, LibraryVersion::LibmExtensions> {
   using F = FuncPointer<float, float>;
   using FN = FuncPointer<float, int, float>;

@luporl
Copy link
Contributor Author

luporl commented Mar 8, 2024

If this is the best fix for Darwin, please apply it to the other sites in the code that have worked around this bug in the past, too.

flang/include/flang/Evaluate/integer.h:#undef HUGE
flang/include/flang/Evaluate/real.h:#undef HUGE
flang/lib/Evaluate/fold-implementation.h:#undef HUGE

Thanks for the review. Setting _POSIX_C_SOURCE in header files would fail if <math.h> was already included before these headers. Instead, I have defined it in flang/CMakeLists.txt. I was able to perform a clean build and run check-flang without errors with this change.

@luporl luporl requested review from kkwli and jeanPerier March 8, 2024 19:36
Copy link
Collaborator

@kkwli kkwli left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks

@luporl
Copy link
Contributor Author

luporl commented Mar 11, 2024

Windows CI errors seem unrelated: fatal error C1060: compiler is out of heap space

@luporl luporl merged commit 3149c93 into llvm:main Mar 11, 2024
4 of 5 checks passed
@luporl luporl deleted the luporl-fix-darwin-build branch March 11, 2024 11:25
brooksdavis added a commit to brooksdavis/llvm-project that referenced this pull request Mar 21, 2024
The HUGE definition collides with the HUGE macro from math.h.  Unlike
the fix in 3149c93 (llvm#84478) (largely reverted in f95710c), add
another #undef HUGE since there is no practical way to make FreeBSD's
headers not define HUGE and still define XSI interfaces such as isascii or
strnlen.

Fixes llvm#86038
brooksdavis added a commit that referenced this pull request Apr 9, 2024
The HUGE definition collides with the HUGE macro from math.h. Unlike the
fix in 3149c93 (#84478) (largely reverted in f95710c), add
another #undef HUGE since there is no practical way to make FreeBSD's
headers not define HUGE and still define XSI interfaces such as isascii
or strnlen.

Update comments above `#undef HUGE` instances to reflect the fact that
all major BSD versions (I checked DragonFly, FreeBSD, NetBSD, and
OpenBSD) leak the HUGE macro from math.h to various degrees.

Fixes #86038
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants