Skip to content

[libcxx] Unmatched _LIBCPP_POP_MACROS with LIBCXX_ENABLE_LOCALIZATION=OFF #134681

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

Closed
s-barannikov opened this issue Apr 7, 2025 · 6 comments · Fixed by #134874
Closed

[libcxx] Unmatched _LIBCPP_POP_MACROS with LIBCXX_ENABLE_LOCALIZATION=OFF #134681

s-barannikov opened this issue Apr 7, 2025 · 6 comments · Fixed by #134874
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@s-barannikov
Copy link
Contributor

s-barannikov commented Apr 7, 2025

With LIBCXX_ENABLE_LOCALIZATION=OFF a bunch of tests fail to compile with an error:

libcxx/test-suite-install/include/c++/v1/istream:1385:1: error: pragma pop_macro could not pop 'min', no matching push_macro [-Werror,-Wignored-pragmas]
# |  1385 | _LIBCPP_POP_MACROS
# |       | ^

The corresponding _LIBCPP_PUSH_MACROS is guarded by #if _LIBCPP_HAS_LOCALIZATION, while _LIBCPP_POP_MACROS isn't.

@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Apr 7, 2025
@ldionne
Copy link
Member

ldionne commented Apr 7, 2025

This is strange, that didn't come up in our tests. Can you describe exactly what you're running?

@s-barannikov
Copy link
Contributor Author

s-barannikov commented Apr 7, 2025

I'm running libc++ test suite. The tests that unexpectedly fail are:

std/containers/container.adaptors/flat.map/flat.map.iterators/reverse_iterator.pass.cpp
std/containers/container.adaptors/flat.multimap/flat.multimap.iterators/reverse_iterator.pass.cpp
std/containers/container.adaptors/flat.multiset/flat.multiset.iterators/reverse_iterator.pass.cpp
std/containers/container.adaptors/flat.set/flat.set.iterators/reverse_iterator.pass.cpp
Used CMake options
      -DLIBUNWIND_ENABLE_ASSERTIONS=OFF
      -DLIBUNWIND_ENABLE_SHARED=OFF
      -DLIBUNWIND_ENABLE_STATIC=ON
      -DLIBUNWIND_ENABLE_THREADS=OFF
      -DLIBUNWIND_IS_BAREMETAL=ON
      -DLIBUNWIND_SHARED_OUTPUT_NAME="unwind-shared"
      -DLIBUNWIND_USE_COMPILER_RT=ON

      -DLIBCXXABI_BAREMETAL=ON
      -DLIBCXXABI_ENABLE_ASSERTIONS=OFF
      -DLIBCXXABI_ENABLE_EXCEPTIONS=OFF
      -DLIBCXXABI_ENABLE_SHARED=OFF
      -DLIBCXXABI_ENABLE_STATIC_UNWINDER=OFF
      -DLIBCXXABI_ENABLE_THREADS=OFF
      -DLIBCXXABI_NON_DEMANGLING_TERMINATE=OFF
      -DLIBCXXABI_SHARED_OUTPUT_NAME="c++abi-shared"
      -DLIBCXXABI_USE_COMPILER_RT=ON

      -DLIBCXX_ABI_UNSTABLE=ON
      -DLIBCXX_ENABLE_EXCEPTIONS=OFF
      -DLIBCXX_ENABLE_FILESYSTEM=OFF
      -DLIBCXX_ENABLE_MONOTONIC_CLOCK=OFF
      -DLIBCXX_ENABLE_RANDOM_DEVICE=OFF
      -DLIBCXX_ENABLE_RTTI=OFF
      -DLIBCXX_ENABLE_SHARED=OFF
      -DLIBCXX_ENABLE_THREADS=OFF
      -DLIBCXX_ENABLE_LOCALIZATION=OFF
      -DLIBCXX_ENABLE_WIDE_CHARACTERS=OFF
      -DLIBCXX_SHARED_OUTPUT_NAME="c++-shared"
      -DLIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY=ON
      -DLIBCXX_USE_COMPILER_RT=ON

There is nothing special in a custom lit config / arguments.
The compiler is (almost) ToT clang.

@s-barannikov s-barannikov changed the title [libcxx] Unmatched _LIBCPP_POP_MACROS with _LIBCXX_ENABLE_LOCALIZATION=OFF [libcxx] Unmatched _LIBCPP_POP_MACROS with LIBCXX_ENABLE_LOCALIZATION=OFF Apr 7, 2025
@frederick-vs-ja
Copy link
Contributor

This mismatch seems to be real and is probably introduced by #108637.

It seems that we almost never test including <istream> with localization disabled. I wonder whether we should test this.

std/containers/container.adaptors/flat.map/flat.map.iterators/reverse_iterator.pass.cpp
std/containers/container.adaptors/flat.multimap/flat.multimap.iterators/reverse_iterator.pass.cpp
std/containers/container.adaptors/flat.multiset/flat.multiset.iterators/reverse_iterator.pass.cpp
std/containers/container.adaptors/flat.set/flat.set.iterators/reverse_iterator.pass.cpp

At least these test files shouldn't include <iostream>, I believe.

@ldionne
Copy link
Member

ldionne commented Apr 8, 2025

Both @mordante and I are unable to reproduce this issue locally. That's weird.

But by looking at the code I can tell the POP_MACROS is in the wrong place. These tests also clearly shouldn't include <iostream>.

Finally, I do think we should be testing the inclusion of those headers since they're supposed to be no-ops now. I'll make a patch.

@s-barannikov
Copy link
Contributor Author

s-barannikov commented Apr 8, 2025

Both @mordante and I are unable to reproduce this issue locally. That's weird.

Just in case, here are the used features:

All available features: 32-bit-pointer, buildhost=linux, c++26, c++experimental,
can-create-symlinks, clang, clang-21, clang-21.0, clang-21.0.0,
diagnose-if-support, enable-benchmarks=no, executor-has-no-bash,
gcc-style-warnings, has-fblocks, has-fconstexpr-steps, host-has-gdb-with-python,
libcpp-abi-no-compressed-pair-padding, libcpp-abi-version=2,
libcpp-hardening-mode=none, libcpp-has-abi-fix-unordered-container-size-type,
libcpp-has-no-availability-markup, libcpp-has-no-unicode, no-exceptions,
no-filesystem, no-localization, no-monotonic-clock, no-random-device, no-rtti,
no-threads, no-tzdb, no-wide-characters, objective-c++, optimization=speed,
std-at-least-c++03, std-at-least-c++11, std-at-least-c++14, std-at-least-c++17,
std-at-least-c++20, std-at-least-c++23, std-at-least-c++26, stdlib=libc++,
stdlib=llvm-libc++, target=xx-unknown-none, thread-safety, verify-support

Not sure if it makes a difference, but C++ modules are disabled.

ldionne added a commit to ldionne/llvm-project that referenced this issue Apr 8, 2025
As a drive-by, also remove unnecessary includes from flat container tests.

Closes llvm#134681
@ldionne
Copy link
Member

ldionne commented Apr 8, 2025

I did #134874 to solve the immediate problem but I am also working on a patch to enable the testing of headers guarded with NO_LOCALIZATION in the CI, since they don't produce a hard error anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants