This is an archive of the discontinued LLVM Phabricator instance.

Differential D104746

[libc++] Remove ad-hoc modules tests that are now unnecessary
ClosedPublic

Authored by ldionne on Jun 22 2021, 1:58 PM.

Details

Reviewers
• Quuxplusone
Group Reviewers
Restricted Project
Commits
rG0c0628c92c0d: [libc++] Remove ad-hoc modules tests that are now unnecessary
Summary

Since we now have modules-enabled CI, it is now redundant to have ad-hoc
tests that check arbitrary things about our modules support. Instead,
the whole test suite should pass with modules enabled, period.

This patch also removes the module cache path workaround: one would
expect that modules work properly without that workaround. If that
isn't the case and we do run into flaky test failures, we can re-enable
the workaround temporarily (but that would be very vexing and we should
fix Clang ASAP if that's the case).

Diff Detail

Event Timeline

ldionne created this revision.Jun 22 2021, 1:58 PM
Herald added a subscriber: arichardson. · View Herald TranscriptJun 22 2021, 1:58 PM
ldionne requested review of this revision.Jun 22 2021, 1:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 22 2021, 1:58 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
• Quuxplusone accepted this revision as: • Quuxplusone.Jun 22 2021, 2:13 PM
• Quuxplusone added a subscriber: • Quuxplusone.
• Quuxplusone added inline comments.
libcxx/test/libcxx/modules/cinttypes_exports.compile.pass.cpp
24–28

I'd feel better if the commit message explicitly pointed to an existing libcxx/test/ test that would verify this same behavior (namely, that I can use either int8_t or std::int8_t after including nothing but <cinttypes>). However, IMHO this is still fine to clean up, regardless.

ldionne added inline comments.Jun 22 2021, 4:27 PM
libcxx/test/libcxx/modules/cinttypes_exports.compile.pass.cpp
24–28

See libcxx/test/std/input.output/file.streams/c.files/cinttypes.pass.cpp. For each header we remove in libcxx/test/modules, I grepped for the corresponding test in libcxx/test/std and found one.

Harbormaster completed remote builds in B110496: Diff 353779.Jun 22 2021, 7:31 PM
This revision was not accepted when it landed; it landed in state Needs Review.Jun 23 2021, 6:43 AM
Closed by commit rG0c0628c92c0d: [libc++] Remove ad-hoc modules tests that are now unnecessary (authored by ldionne). · Explain Why
This revision was automatically updated to reflect the committed changes.
ldionne added a commit: rG0c0628c92c0d: [libc++] Remove ad-hoc modules tests that are now unnecessary.

Revision Contents

Diff 353953

libcxx/test/libcxx/modules/cinttypes_exports.compile.pass.cpp

  • This file was deleted.
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// This test fails on Windows because the underlying libc headers on Windows
// are not modular
// XFAIL: LIBCXX-WINDOWS-FIXME
// Some headers are not available when these features are disabled, but they
// still get built as part of the 'std' module, which breaks the build.
// UNSUPPORTED: libcpp-has-no-threads
// UNSUPPORTED: libcpp-has-no-localization
// UNSUPPORTED: libcpp-has-no-filesystem-library
// REQUIRES: modules-support
// ADDITIONAL_COMPILE_FLAGS: -fmodules
// Test that <cinttypes> re-exports <cstdint>
#include <cinttypes>
int main(int, char**) {
int8_t x; (void)x;
std::int8_t y; (void)y;
QuuxplusoneUnsubmitted
Not Done
ReplyInline Actions

I'd feel better if the commit message explicitly pointed to an existing libcxx/test/ test that would verify this same behavior (namely, that I can use either int8_t or std::int8_t after including nothing but <cinttypes>). However, IMHO this is still fine to clean up, regardless.

Quuxplusone: I'd feel better if the commit message explicitly pointed to an existing `libcxx/test/` test…
ldionneAuthorUnsubmitted
Done
ReplyInline Actions

See libcxx/test/std/input.output/file.streams/c.files/cinttypes.pass.cpp. For each header we remove in libcxx/test/modules, I grepped for the corresponding test in libcxx/test/std and found one.

ldionne: See `libcxx/test/std/input.output/file.streams/c.files/cinttypes.pass.cpp`. For each header we…
return 0;
}

libcxx/test/libcxx/modules/clocale_exports.compile.pass.cpp

  • This file was deleted.
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// This test fails on Windows because the underlying libc headers on Windows
// are not modular
// XFAIL: LIBCXX-WINDOWS-FIXME
// Some headers are not available when these features are disabled, but they
// still get built as part of the 'std' module, which breaks the build.
// UNSUPPORTED: libcpp-has-no-threads
// UNSUPPORTED: libcpp-has-no-localization
// UNSUPPORTED: libcpp-has-no-filesystem-library
// UNSUPPORTED: c++03
// REQUIRES: modules-support
// ADDITIONAL_COMPILE_FLAGS: -fmodules
#include <clocale>
int main(int, char**) {
std::lconv l; (void)l;
using T = decltype(std::setlocale(0, ""));
using U = decltype(std::localeconv());
return 0;
}

libcxx/test/libcxx/modules/cstdint_exports.compile.pass.cpp

  • This file was deleted.
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// This test fails on Windows because the underlying libc headers on Windows
// are not modular
// XFAIL: LIBCXX-WINDOWS-FIXME
// Some headers are not available when these features are disabled, but they
// still get built as part of the 'std' module, which breaks the build.
// UNSUPPORTED: libcpp-has-no-threads
// UNSUPPORTED: libcpp-has-no-localization
// UNSUPPORTED: libcpp-has-no-filesystem-library
// Test that <cstdint> re-exports <stdint.h>
// REQUIRES: modules-support
// ADDITIONAL_COMPILE_FLAGS: -fmodules
#include <cstdint>
int main(int, char**) {
int8_t x; (void)x;
std::int8_t y; (void)y;
return 0;
}

libcxx/test/libcxx/modules/inttypes_h_exports.compile.pass.cpp

  • This file was deleted.
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// This test fails on Windows because the underlying libc headers on Windows
// are not modular
// XFAIL: LIBCXX-WINDOWS-FIXME
// Some headers are not available when these features are disabled, but they
// still get built as part of the 'std' module, which breaks the build.
// UNSUPPORTED: libcpp-has-no-threads
// UNSUPPORTED: libcpp-has-no-localization
// UNSUPPORTED: libcpp-has-no-filesystem-library
// Test that intypes.h re-exports stdint.h
// REQUIRES: modules-support
// ADDITIONAL_COMPILE_FLAGS: -fmodules
#include <inttypes.h>
int main(int, char**) {
int8_t x; (void)x;
return 0;
}

libcxx/test/libcxx/modules/stdint_h_exports.compile.pass.cpp

  • This file was deleted.
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// Some headers are not available when these features are disabled, but they
// still get built as part of the 'std' module, which breaks the build.
// UNSUPPORTED: libcpp-has-no-threads
// UNSUPPORTED: libcpp-has-no-localization
// UNSUPPORTED: libcpp-has-no-filesystem-library
// Test that int8_t and the like are exported from stdint.h, not inttypes.h
// REQUIRES: modules-support
// ADDITIONAL_COMPILE_FLAGS: -fmodules
// XFAIL: LIBCXX-WINDOWS-FIXME
#include <stdint.h>
int main(int, char**) {
int8_t x; (void)x;
return 0;
}

libcxx/test/libcxx/modules/stds_include.sh.cpp

  • This file was deleted.
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// Test that we can include libc++ headers when building with modules
// enabled in various Standard modes. This is a common source of breakage
// since the 'std' module will include all headers, so if something in a
// header fails under a standard mode, importing anything will fail.
// This test fails on Windows because the underlying libc headers on Windows
// are not modular
// XFAIL: LIBCXX-WINDOWS-FIXME
// Some headers are not available when these features are disabled, but they
// still get built as part of the 'std' module, which breaks the build.
// UNSUPPORTED: libcpp-has-no-threads
// UNSUPPORTED: libcpp-has-no-localization
// UNSUPPORTED: libcpp-has-no-filesystem-library
// REQUIRES: modules-support
// NOTE: The -std=XXX flag is present in %{flags}, so we overwrite it by passing it after %{flags}.
// FIXME: Remove the `-DINVALIDATE_CACHE_FOO` macros. Their purpose is to workaround a bug in older Clang versions
// the same modules caches were reused across standard dialects.
// RUN: %{cxx} %{flags} %{compile_flags} -fmodules -fcxx-modules -fsyntax-only -std=c++03 -DINVALIDATE_CACHE_CXX03 %s
// RUN: %{cxx} %{flags} %{compile_flags} -fmodules -fcxx-modules -fsyntax-only -std=c++11 -DINVALIDATE_CACHE_CXX11 %s
// RUN: %{cxx} %{flags} %{compile_flags} -fmodules -fcxx-modules -fsyntax-only -std=c++14 -DINVALIDATE_CACHE_CXX14 %s
// RUN: %{cxx} %{flags} %{compile_flags} -fmodules -fcxx-modules -fsyntax-only -std=c++17 -DINVALIDATE_CACHE_CXX17 %s
// RUN: %{cxx} %{flags} %{compile_flags} -fmodules -fcxx-modules -fsyntax-only -std=c++2a -DINVALIDATE_CACHE_CXX20 %s
#include <vector>
int main(int, char**) {
return 0;
}

libcxx/utils/libcxx/test/features.py

Show All 37 LinesDEFAULT_FEATURES = [
Feature(name='libcpp-no-if-constexpr', when=lambda cfg: '__cpp_if_constexpr' not in featureTestMacros(cfg)), Feature(name='libcpp-no-if-constexpr', when=lambda cfg: '__cpp_if_constexpr' not in featureTestMacros(cfg)),
Feature(name='libcpp-no-structured-bindings', when=lambda cfg: '__cpp_structured_bindings' not in featureTestMacros(cfg)), Feature(name='libcpp-no-structured-bindings', when=lambda cfg: '__cpp_structured_bindings' not in featureTestMacros(cfg)),
Feature(name='libcpp-no-deduction-guides', when=lambda cfg: featureTestMacros(cfg).get('__cpp_deduction_guides', 0) < 201611), Feature(name='libcpp-no-deduction-guides', when=lambda cfg: featureTestMacros(cfg).get('__cpp_deduction_guides', 0) < 201611),
Feature(name='libcpp-no-concepts', when=lambda cfg: featureTestMacros(cfg).get('__cpp_concepts', 0) < 201907), Feature(name='libcpp-no-concepts', when=lambda cfg: featureTestMacros(cfg).get('__cpp_concepts', 0) < 201907),
Feature(name='has-fobjc-arc', when=lambda cfg: hasCompileFlag(cfg, '-xobjective-c++ -fobjc-arc') and Feature(name='has-fobjc-arc', when=lambda cfg: hasCompileFlag(cfg, '-xobjective-c++ -fobjc-arc') and
sys.platform.lower().strip() == 'darwin'), # TODO: this doesn't handle cross-compiling to Apple platforms. sys.platform.lower().strip() == 'darwin'), # TODO: this doesn't handle cross-compiling to Apple platforms.
Feature(name='objective-c++', when=lambda cfg: hasCompileFlag(cfg, '-xobjective-c++ -fobjc-arc')), Feature(name='objective-c++', when=lambda cfg: hasCompileFlag(cfg, '-xobjective-c++ -fobjc-arc')),
# Note: We use a custom modules cache path to make sure that we don't reuse
# the default one, which can be shared across builds. This is important
# because we define macros in headers files, and a change in these macros
# doesn't seem to invalidate modules cache entries, which means we could
# build against now-invalid cached headers from a previous build.
Feature(name='modules-support',
when=lambda cfg: hasCompileFlag(cfg, '-fmodules'),
actions=lambda cfg: [AddCompileFlag('-fmodules-cache-path=%t/ModuleCache')]),
Feature(name='non-lockfree-atomics', Feature(name='non-lockfree-atomics',
when=lambda cfg: sourceBuilds(cfg, """ when=lambda cfg: sourceBuilds(cfg, """
#include <atomic> #include <atomic>
struct Large { int storage[100]; }; struct Large { int storage[100]; };
std::atomic<Large> x; std::atomic<Large> x;
int main(int, char**) { (void)x.load(); return 0; } int main(int, char**) { (void)x.load(); return 0; }
""")), """)),
# TODO: Remove this feature once compiler-rt includes __atomic_is_lockfree() # TODO: Remove this feature once compiler-rt includes __atomic_is_lockfree()
▲ Show 20 Lines • Show All 107 Lines • Show Last 20 Lines
libcxx/test/libcxx/modules/clocale_exports.compile.pass.cpp