Page MenuHomePhabricator

[libc++] NFC: Move unwrap_iter to its own header
AbandonedPublic

Authored by ldionne on Sat, May 29, 1:24 PM.

Details

Summary

This test 996889 again. It seems @Quuxplusone found a solution for the
issue. This tests the original patch with the fix included.

@ldionne feel free to commandeer the patch again.

Depends on D102781

Diff Detail

Event Timeline

Mordante created this revision.Sat, May 29, 1:24 PM
Mordante requested review of this revision.Sat, May 29, 1:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptSat, May 29, 1:24 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Well, my solution was to add this new header (and all new headers) to module.modulemap. So I assume this PR will fail buildkite as-is; it would need to add this header (and/or other new headers) to module.modulemap in order to keep buildkite happy. But I recommend people discuss the issue/solution(s) over on https://reviews.llvm.org/D102781#inline-979125 just to keep it all in the same place.

Mordante updated this revision to Diff 348657.Sat, May 29, 1:28 PM

Rebase on top of D102781, see whether the CI likes this version better.

Well, my solution was to add this new header (and all new headers) to module.modulemap. So I assume this PR will fail buildkite as-is; it would need to add this header (and/or other new headers) to module.modulemap in order to keep buildkite happy. But I recommend people discuss the issue/solution(s) over on https://reviews.llvm.org/D102781#inline-979125 just to keep it all in the same place.

I agree it would be good to keep the discussion in D102781. This is mainly to test whether your approach also works here or more changes are required. The original patch broke the Linux builds. I tested Linux locally with only your __iterator_iterator_traits_h module applied and that fixed it locally.

ldionne commandeered this revision.Mon, May 31, 9:24 AM
ldionne edited reviewers, added: Mordante; removed: ldionne.
ldionne updated this revision to Diff 348821.Mon, May 31, 9:25 AM

Fix CI issues (I hope).

ldionne updated this revision to Diff 349015.Tue, Jun 1, 10:27 AM

Poke CI. I suspect the previous failure is a fluke because it only happens in the back-deployment configuration, and those nodes were problematic in the past few days. Will investigate if it fails again.

ldionne updated this revision to Diff 349090.Tue, Jun 1, 1:33 PM

Poke CI - now this should build since the Apple issues have been fixed.

ldionne accepted this revision as: Restricted Project.Tue, Jun 1, 6:46 PM

Looks like this is working now.

This revision was not accepted when it landed; it landed in state Needs Review.Tue, Jun 1, 6:47 PM
This revision was automatically updated to reflect the committed changes.
ldionne reopened this revision.Wed, Jun 2, 1:01 PM
ldionne abandoned this revision.Wed, Jun 2, 2:54 PM
ldionne added a subscriber: zoecarver.

@zoecarver is going to handle that as part of the algorithm split.