This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Overhaul all tests for assertions and debug mode
ClosedPublic

Authored by ldionne on Mar 11 2022, 7:00 AM.

Details

Reviewers
None
Group Reviewers
Restricted Project
Commits
rGf6fd1c1438f0: [libc++] Overhaul all tests for assertions and debug mode
Summary

Prior to this patch, there was no distinction between tests that check
basic assertions and tests that check full-fledged iterator debugging
assertions. Both were disabled when support for the debug mode is not
provided in the dylib, which is stronger than it needs to be.

Furthermore, all of the tests using "debug_macros.h" that contain more
than one assertion in them were broken -- any code after the first
assertion would never be executed.

This patch refactors all of our assertion-related tests to:

  1. Be enabled whenever they can, i.e. basic assertions tests are run even when the debug mode is disabled.
  2. Use the superior check_assertion.h (previously debug_mode_helper.h) instead of debug_macros.h, which allows multiple assertions in the same program.
  3. Coalesce some tests into the same file to make them more readable.
  4. Use consistent naming for test files -- no more db{1,2,3,...,10} tests.

This is a large but mostly mechanical patch.

Diff Detail

Event Timeline

ldionne created this revision.Mar 11 2022, 7:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 11 2022, 7:00 AM
Herald added a subscriber: arphaman. · View Herald Transcript
ldionne requested review of this revision.Mar 11 2022, 7:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 11 2022, 7:00 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

It would have been possible but very tedious to split this into smaller patches due to the number of touched files. I believe the easiest way to review this is to trust that I'm indeed doing what I explain in the commit summary, and review whether that plan seems reasonable instead (hopefully it does).

ldionne updated this revision to Diff 415092.Mar 14 2022, 7:39 AM

Address hopefully most CI failures.

ldionne updated this revision to Diff 415164.Mar 14 2022, 11:03 AM

Hopefully fix CI on Windows.

ldionne accepted this revision as: Restricted Project.Mar 14 2022, 1:39 PM

Ci is green -- the Apple System failure is a flake. I'll ship this tomorrow if nobody chimes in, since this is pretty mechanical.

This revision was not accepted when it landed; it landed in state Needs Review.Mar 15 2022, 7:56 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
libcxx/test/support/container_debug_tests.h