This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Make __debug_less::operator() constexpr
ClosedPublic

Authored by thomasanderson on Apr 15 2019, 10:30 AM.

Details

Summary

This is a followup to [1] which added a new __debug_less::operator() overload.
[2] added _LIBCPP_CONSTEXPR_AFTER_CXX17 to the original
__debug_less::operator() between the time of writing [1] and landing it. This
change adds _LIBCPP_CONSTEXPR_AFTER_CXX17 to the new overload too.

[1] https://reviews.llvm.org/rL358423
[2] https://reviews.llvm.org/rL358252

Diff Detail

Repository
rL LLVM

Event Timeline

We should apply this to both versions of __do_compare_assert and to the call operator taking non-const arguments as well.

And can you add additional tests that create and call the debug comparator in a constexpr context in C++17?

We should apply this to both versions of __do_compare_assert and to the call operator taking non-const arguments as well.

They're already constexpr

And can you add additional tests that create and call the debug comparator in a constexpr context in C++17?

Done

EricWF added inline comments.Apr 16 2019, 10:49 PM
libcxx/test/libcxx/algorithms/debug_less.pass.cpp
278 ↗(On Diff #195426)
constexpr bool test_constexpr() {
    std::less<> cmp;
    __debug_less<std::less<> > dcmp(cmp);
    assert(dcmp(1, 2));
    assert(!dcmp(1, 1));
    return true;
}
static_assert(test_constexpr(), "");
EricWF accepted this revision.Apr 16 2019, 10:51 PM
EricWF added inline comments.
libcxx/test/libcxx/algorithms/debug_less.pass.cpp
278 ↗(On Diff #195426)

I think the suggested test is cleaner. No need to sprinkle constexpr on every variable.
Just have a constexpr function and invoke it in a constant expression.

This revision is now accepted and ready to land.Apr 16 2019, 10:51 PM
thomasanderson marked 2 inline comments as done.

Address comments

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2019, 5:51 PM