This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Disable tree invariant check in asserts mode
ClosedPublic

Authored by hans on Jul 4 2023, 1:53 AM.

Details

Summary

This is a follow-up to D153672 which removed the old debug mode and moved many of those checks to the regular asserts mode.

The tree invariant check is too expensive for the regular asserts mode, making element removal O(n) instead of O(log n), so disable it until there is a new debug assert category it can be put in.

Diff Detail

Event Timeline

hans created this revision.Jul 4 2023, 1:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 4 2023, 1:53 AM
hans requested review of this revision.Jul 4 2023, 1:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 4 2023, 1:53 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
alexfh accepted this revision.Jul 4 2023, 4:46 AM
alexfh added a subscriber: alexfh.

Thanks for the temporary fix, Hans! We're also affected by this.

LGTM

alexfh added a comment.Jul 4 2023, 4:48 AM

Given the comment in https://reviews.llvm.org/D153672#4468348, I think, this should be fine to submit.

hans added a comment.Jul 4 2023, 6:42 AM

Given the comment in https://reviews.llvm.org/D153672#4468348, I think, this should be fine to submit.

Sounds good to me. Happy to follow up if libc++ maintainers have more input.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 4 2023, 6:42 AM
This revision was automatically updated to reflect the committed changes.

LGTM, but as a matter of process, please try to wait for someone from the libc++ review group before submitting. Thanks for the fix!

libcxx/include/__tree
379–381

CC @var-const in case you want to annotate your TODOs for the debug mode in some special way to find them later.