[libc++] tests with picolibc: mark fenv tests as unsupported

[domin144]

The FE_* macros checked for in cfenv.pass.cpp are not required to be defined, if the relevant options are not available. Picolibc happens not to provide these on some platforms.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Dominik Wójt (domin144)

Changes

The FE_* macros checked for in cfenv.pass.cpp are not required to be defined, if the relevant options are available. Picolibc happens not to provide these on some platforms.
Added a test in features.py to skip fenv test automatically.

---

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

3 Files Affected:

• (modified) libcxx/test/std/depr/depr.c.headers/fenv_h.compile.pass.cpp (+1-1)
• (modified) libcxx/test/std/numerics/cfenv/cfenv.syn/cfenv.pass.cpp (+1-1)
• (modified) libcxx/utils/libcxx/test/features.py (+29)

diff --git a/libcxx/test/std/depr/depr.c.headers/fenv_h.compile.pass.cpp b/libcxx/test/std/depr/depr.c.headers/fenv_h.compile.pass.cpp
index dcc97573d6073..a896f5e03c3af 100644
--- a/libcxx/test/std/depr/depr.c.headers/fenv_h.compile.pass.cpp
+++ b/libcxx/test/std/depr/depr.c.headers/fenv_h.compile.pass.cpp
@@ -7,7 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 // Floating point exceptions are required for the FE_... macros to be defined.
-// XFAIL: LIBCXX-PICOLIBC-FIXME
+// REQUIRES: has-compolete-fenv
 
 // <fenv.h>
 
diff --git a/libcxx/test/std/numerics/cfenv/cfenv.syn/cfenv.pass.cpp b/libcxx/test/std/numerics/cfenv/cfenv.syn/cfenv.pass.cpp
index 7b3b490eba273..5a39003d00d0f 100644
--- a/libcxx/test/std/numerics/cfenv/cfenv.syn/cfenv.pass.cpp
+++ b/libcxx/test/std/numerics/cfenv/cfenv.syn/cfenv.pass.cpp
@@ -7,7 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 // Floating point exceptions are required for the FE_... macros to be defined.
-// XFAIL: LIBCXX-PICOLIBC-FIXME
+// REQUIRES: has-compolete-fenv
 
 // <cfenv>
 
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index ccabb48833f10..f5b12f90c0510 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -212,6 +212,35 @@ def _getAndroidDeviceApi(cfg):
           """,
         ),
     ),
+    # Some platform do not provide complete floating point environment.
+    Feature(
+        name="has-compolete-fenv",
+        when=lambda cfg: sourceBuilds(
+            cfg,
+            """
+            #include <fenv.h>
+
+            #if !( \
+                defined FE_DIVBYZERO \
+                && defined FE_INEXACT \
+                && defined FE_INVALID \
+                && defined FE_INEXACT \
+                && defined FE_OVERFLOW \
+                && defined FE_UNDERFLOW \
+                && defined FE_ALL_EXCEPT \
+                && defined FE_DOWNWARD \
+                && defined FE_TONEAREST \
+                && defined FE_TOWARDZERO \
+                && defined FE_UPWARD \
+                && defined FE_DFL_ENV \
+                && defined FE_INEXACT)
+            #error Floating point environment not complete
+            #endif
+
+            int main(int, char**) { return 0; }
+          """,
+        ),
+    ),
     # Check for a Windows UCRT bug (fixed in UCRT/Windows 10.0.20348.0):
     # https://developercommunity.visualstudio.com/t/utf-8-locales-break-ctype-functions-for-wchar-type/1653678
     Feature(

[domin144]

@ldionne @jroelofs

[mordante]

This feels a bit like a maintenance burden, if C adds new macros this need to be updated too.
Since we have not needed this before now I don't feel this solves a generic.
How about adding a comment and an // UNSUPPORTED: pico-lib identifier in these two tests instead?

[domin144]

It used to make sense to me, as I am doing tests on more configurations of picolibc and some of them do provide all the necessary macros (e.g. armv7m with fpu). Now, I realize this is not that useful for libc++ itself, as only one configuration is tested in CI here.

Yet, still I will switch from XFAIL to UNSUPPORTED to cater the configurations, where this test would pass.

[EricWF]

Yeah what value does this provide to libC++

…

On Sat, Dec 9, 2023, 8:16 AM Mark de Wever ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In libcxx/utils/libcxx/test/features.py <#74610 (comment)>: > @@ -212,6 +212,35 @@ def _getAndroidDeviceApi(cfg): """, ), ), + # Some platform do not provide complete floating point environment. + Feature( + name="has-compolete-fenv", + when=lambda cfg: sourceBuilds( + cfg, + """ + #include <fenv.h> + + #if !( \ + defined FE_DIVBYZERO \ This feels a bit like a maintenance burden, if C adds new macros this need to be updated too. Since we have not needed this before now I don't feel this solves a generic. How about adding a comment and an // UNSUPPORTED: pico-lib identifier in these two tests instead? — Reply to this email directly, view it on GitHub <#74610 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAI7G23ZEK5GTP5CYSPNC4TYIRQDVAVCNFSM6AAAAABAJS2J5SVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONZTGY4TOMBYGA> . You are receiving this because your review was requested.Message ID: ***@***.***>

[ldionne]

LGTM in this form -- I agree with other reviewers that adding the FE_foo checking in features.py was kind of iffy.