[libc] Remove unused target conditionals for Apple platforms #119030
Conversation
It seems that LIBC_TARGET_OS_IS_MACOS and LIBC_TARGET_OS_IS_IPHONE were never actually used in the code, so these definitions can be removed. I came across these because libc++ now depends on llvm-libc to build (for from_chars), and the unguarded use of TargetConditionals.h broke some of our downstream configurations. There are some platforms for which __APPLE__ is defined but that don't provide TargetConditionals.h. If there is a need to keep defining those, the compiler also provides some uglier macro definitions like __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ that do not require including any header.
|
@llvm/pr-subscribers-libc Author: Louis Dionne (ldionne) ChangesIt seems that LIBC_TARGET_OS_IS_MACOS and LIBC_TARGET_OS_IS_IPHONE were never actually used in the code, so these definitions can be removed. I came across these because libc++ now depends on llvm-libc to build (for from_chars), and the unguarded use of TargetConditionals.h broke some of our downstream configurations. There are some platforms for which APPLE is defined but that don't provide TargetConditionals.h. If there is a need to keep defining those, the compiler also provides some uglier macro definitions like Full diff: https://github.com/llvm/llvm-project/pull/119030.diff 1 Files Affected:
diff --git a/libc/src/__support/macros/properties/os.h b/libc/src/__support/macros/properties/os.h
index 1c8fd5721ce679..807ce1812735c0 100644
--- a/libc/src/__support/macros/properties/os.h
+++ b/libc/src/__support/macros/properties/os.h
@@ -25,18 +25,6 @@
#define LIBC_TARGET_OS_IS_WINDOWS
#endif
-#if (defined(__apple__) || defined(__APPLE__) || defined(__MACH__))
-// From https://stackoverflow.com/a/49560690
-#include "TargetConditionals.h"
-#if defined(TARGET_OS_OSX)
-#define LIBC_TARGET_OS_IS_MACOS
-#endif
-#if defined(TARGET_OS_IPHONE)
-// This is set for any non-Mac Apple products (IOS, TV, WATCH)
-#define LIBC_TARGET_OS_IS_IPHONE
-#endif
-#endif
-
#if defined(__Fuchsia__)
#define LIBC_TARGET_OS_IS_FUCHSIA
#endif
|
|
What's the policy for merging? I assume this can be merged once CI is green? |
|
Correct. If this patch was urgent it would be okay to merge before the CI finished, but for normal patches we recommend waiting for the CI. |
It seems that LIBC_TARGET_OS_IS_MACOS and LIBC_TARGET_OS_IS_IPHONE were never actually used in the code, so these definitions can be removed.
I came across these because libc++ now depends on llvm-libc to build (for from_chars), and the unguarded use of TargetConditionals.h broke some of our downstream configurations. There are some platforms for which
__APPLE__is defined but that don't provide TargetConditionals.h.If there is a need to keep defining those, the compiler also provides some uglier macro definitions like
__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__that do not require including any header.