This is an archive of the discontinued LLVM Phabricator instance.

[libcxx][test] tests for strengthened `noexcept` are non-portable
ClosedPublic

Authored by CaseyCarter on Jan 22 2022, 12:44 PM.

Diff Detail

Event Timeline

CaseyCarter requested review of this revision.Jan 22 2022, 12:44 PM
CaseyCarter created this revision.
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald TranscriptJan 22 2022, 12:44 PM

I actually don't see any textual support for our metaprogrammy conditional-noexcepts in view_interface at all. @zoecarver if you're still around, what was the rationale for all the conditional-noexcept in D101737? Would anything important break if we just got rid of it all?

Not only would removing the noexcept-specifications be simpler, safer, and faster-to-compile, but it would also jibe with @ldionne's general philosophy of keeping as close to the Standard as possible. The Standard doesn't say that these functions should ever be noexcept, so by making ours sometimes-noexcept, we might be opening up portability pitfalls for libc++'s users.

ldionne accepted this revision.Feb 8 2022, 2:50 PM

IIRC the idea was to make functions noexcept as much as we could because that's conforming and we thought that we'd never actually want to do anything else but terminate if an exception was thrown from those. But revisiting this, I agree it's a weird choice to have made and I would support removing the non-required noexcepts (in particular the conditional ones that add complexity). @Quuxplusone if you want to make a pass at those, be my guest, I'll be happy to review.

In the meantime, this looks OK to me since that behavior is indeed libc++ specific. I think the CI failure is OK to ignore.

This revision is now accepted and ready to land.Feb 8 2022, 2:50 PM
This revision was landed with ongoing or failed builds.Feb 8 2022, 2:54 PM
This revision was automatically updated to reflect the committed changes.