This is an archive of the discontinued LLVM Phabricator instance.

[libcxx][ci] enables assertions for runtimes-build
ClosedPublic

Authored by cjdb on Jun 7 2021, 5:11 PM.

Details

Summary

This will catch nasty Clang bugs like
https://bugs.llvm.org/show_bug.cgi?id=50592 before we merge stuff into
libc++ main.

Diff Detail

Event Timeline

cjdb created this revision.Jun 7 2021, 5:11 PM
cjdb requested review of this revision.Jun 7 2021, 5:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 7 2021, 5:11 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
cjdb added a subscriber: phosek.Jun 7 2021, 5:12 PM

This will fail until @phosek merges two libc++ reverts.

phosek accepted this revision.Jun 7 2021, 5:15 PM

LGTM

cjdb updated this revision to Diff 350461.Jun 7 2021, 5:31 PM

restarting CI

Mordante accepted this revision as: Mordante.Jun 7 2021, 11:09 PM

The change is fine by me, but I've some concerns with the build node used for this feature.
The Apple nodes aren't the fastest neither the most reliable build nodes we have.
This combination makes this node less than ideal for build iterations.
So I'm not happy to _only_ enable it for this node, but the improvements can be done in a separate patch.

@ldionne how do our Linux build nodes get their clang version and would it be easy to turn on the assertions for them?
Alternatively would it make sense to have one Linux node build clang HEAD with assertions to test C++2b?
I assume in the future would be nice to have a clang HEAD when we need compiler features to implement C++2b library features.

ldionne accepted this revision.Jun 8 2021, 6:44 AM

@Mordante The runtimes build doesn't run on macOS nodes, so I think this is fine.

This revision is now accepted and ready to land.Jun 8 2021, 6:44 AM

@Mordante The runtimes build doesn't run on macOS nodes, so I think this is fine.

Then my bad. Then we don't need additional patches.

cjdb updated this revision to Diff 350686.Jun 8 2021, 12:28 PM

rebases to reactivate CI

cjdb updated this revision to Diff 350727.Jun 8 2021, 3:36 PM

trying CI once again

This revision was automatically updated to reflect the committed changes.