[libcxx-commits] [PATCH] D147356: Fixing conflicting macro definitions between curses.h and std::filesystem.
Mark de Wever via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Apr 8 11:03:25 PDT 2023
Mordante added a comment.
Before approving I really like to know why this only affects `<filesystem>`. I'm slightly wary to add work-arounds for unrelated external libaries (even when curses is quite Standard).
================
Comment at: libcxx/include/__config:1149
+ _Pragma("pop_macro(\"min\")") _Pragma("pop_macro(\"max\")") _Pragma("pop_macro(\"refresh()\")") \
+ _Pragma("pop_macro(\"move(int, int)\")") _Pragma("pop_macro(\"erase()\")")
----------------
I really would like one entry per line, that makes the code a bit longer, but makes it easier to verify. Especially since the entries are not nicely aligned.
================
Comment at: libcxx/include/__undef_macros:18-26
+#ifdef refresh
+# if defined(__STDC__) || !defined(_NO_PROTO)
+inline int refresh_impl() { return refresh(); }
+# else
+inline extern int refresh_impl() { return refresh(); }
+# endif
+# undef refresh
----------------
philnik wrote:
> These should just be `#undef`ed and added to `_LIBCPP_PUSH_MACROS` and `_LIBCPP_POP_MACROS`. Arguably they should be replaced in the libc, since these names are used everywhere, not just in `<filesystem>`.
Why is this only an issue for filesystem and not for `<vector>`?
================
Comment at: libcxx/test/libcxx/vendor/ibm/curses_filesystem_macro_conflict.compile.pass.cpp:11
+
+#include "curses.h"
+#include <filesystem>
----------------
Why are these IBM specific tests and not using a test feature `has-curses`?
Or why not something like
```
#if __has_include(<curses.h>)
# include <curses.h>
#endif
```
Then the test could be included in `libcxx/test/libcxx/nasty_macros.compile.pass.cpp` instead of IBM only.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147356/new/
https://reviews.llvm.org/D147356
More information about the libcxx-commits
mailing list