[libcxx-commits] [PATCH] D147356: Fixing conflicting macro definitions between curses.h and the standard library.

[Nicole Rabjohn via Phabricator via libcxx-commits]

nicolerabjohn marked 2 inline comments as done.
nicolerabjohn added inline comments.


================
Comment at: libcxx/test/libcxx/vendor/ibm/curses_filesystem_macro_conflict.compile.pass.cpp:11
+
+#include "curses.h"
+#include <filesystem>
----------------
Mordante wrote:
> nicolerabjohn wrote:
> > Mordante wrote:
> > > 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.
> > > 
> > So I agree that these don't need to be IBM-specific - `libcxx/test/libcxx/nasty_macros.compile.pass.cpp` might not be the best choice here either because it's currently XFAILED on AIX for unrelated reasons. AIX is affected by this patch, so I would like these test cases to run on AIX as well. I'll look around and see if I can fix the XFAIL, or find a more appropriate spot for these tests. 
> I think then we really should make sure the AIX tests are fixed, in a separate patch. The nasty macros are really intended for these tests. That also is used to avoid regressions. If a header doesn't push the macros and in C++26 is gets an `erase` function it will be caught in our CI.
Fair enough - I'll take a look at why it was XFAILed to see if it can be fixed, or if the problematic headers can be split out. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147356/new/

https://reviews.llvm.org/D147356