-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[libc] remove MPFR and related tests in full build #87693
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
Conversation
@llvm/pr-subscribers-libc Author: Schrodinger ZHU Yifan (SchrodingerZhu) ChangesIn full build mode, the fuzzing tests fail to build. This PR works around the annoying behavior that MPFR pulls in system C++ headers blindly (see errors below). The fix is rather ugly but it seems to work. Let me know if you have better ideas.
Full diff: https://github.com/llvm/llvm-project/pull/87693.diff 1 Files Affected:
diff --git a/libc/utils/MPFRWrapper/mpfr_inc.h b/libc/utils/MPFRWrapper/mpfr_inc.h
index 58fa7b25a9f210..0d8afbed39859f 100644
--- a/libc/utils/MPFRWrapper/mpfr_inc.h
+++ b/libc/utils/MPFRWrapper/mpfr_inc.h
@@ -17,7 +17,21 @@
// MPFR header can be included in manner allowed in that repo.
#include "CustomMPFRIncluder.h"
#else
+
+extern "C" {
+#pragma push_macro("__cplusplus")
+#if defined(__clang__)
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wbuiltin-macro-redefined"
+#endif
+#undef __cplusplus
#include <mpfr.h>
+#if defined(__clang__)
+#pragma clang diagnostic pop
+#endif
+#pragma pop_macro("__cplusplus")
+}
+
#endif
#endif // LLVM_LIBC_UTILS_MPFRWRAPPER_MPFR_INC_H
|
Should we have a "LEAN_AND_MEAN" version of MPFR headers that define only the things we needed? |
In full build mode, if anything EVER pulls in system headers other than strictly kernel headers:
I'm beginning to think that, slightly orthogonal to the refactoring of headers (#87598, #87017), we would be better served by REQUIRING a But if fuzzing NEEDS MPFR to compare results, then perhaps we don't allow fuzzing in full build mode. Or in full build mode, we don't invoke/use MPFR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't think of a better way at the moment.
the fuzzing tests aren't supported in fullbuild mode right now. If we really want to support fuzzing in fullbuild then this seems like the best way to do it. |
If we want to support fuzzing against full build mode, then we should rebuild MPFR against our libc rather than the system libc. Otherwise libc/fuzzing/ is in some twighlight zone half way between full build and overlay mode. I don't want to support that. |
Now that I think about it, probably we should disable building MPFR in full build mode until MPFR could be built with our libc, especially because of this: https://github.com/llvm/llvm-project/blob/main/libc/utils/MPFRWrapper/CMakeLists.txt#L12 |
7461890
to
133bb48
Compare
I have updated this disable MPFR tests in full build. |
In full build mode, the fuzzing tests fail to build. This PR works around the annoying behavior that MPFR pulls in system C++ headers blindly (see errors below). The fix is rather ugly but it seems to work. Let me know if you have better ideas.