This is an archive of the discontinued LLVM Phabricator instance.

Differential D108393

[libc++] Include <__iterator/distance.h> instead of <iterator> in a few algorithm headers
ClosedPublic

Authored by jloser on Aug 19 2021, 10:48 AM.

Details

Reviewers
ldionne
• Quuxplusone
zoecarver
cjdb
Mordante
Group Reviewers
Restricted Project
Commits
rG9807626bc721: [libc++] Include <__iterator/distance.h> instead of <iterator> in a few…
Summary

A few headers in algorithm include <iterator> when
<__iterator/distance.h> would suffice. Change them to just include
<__iterator.distance.h>.

Diff Detail

Event Timeline

jloser requested review of this revision.Aug 19 2021, 10:48 AM
jloser created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptAug 19 2021, 10:48 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
jloser added reviewers: ldionne, • Quuxplusone, zoecarver, cjdb.Aug 19 2021, 10:49 AM
jloser edited the summary of this revision. (Show Details)
jloser edited the summary of this revision. (Show Details)
cjdb accepted this revision.Aug 19 2021, 10:51 AM

This seems obviously correct to me.

jloser added a comment.Aug 19 2021, 10:51 AM

@cjdb thanks for the quick review. I don't have commit rights -- do you mind committing on my behalf, please?

cjdb added inline comments.Aug 19 2021, 10:51 AM
libcxx/include/__algorithm/equal.h
16–17

Please sort these alphabetically.

cjdb added a comment.Aug 19 2021, 10:52 AM
In D108393#2955223, @jloser wrote:

@cjdb thanks for the quick review. I don't have commit rights -- do you mind committing on my behalf, please?

Sure. Upload the new patch, and I'll take care of it post-CI.

jloser updated this revision to Diff 367550.Aug 19 2021, 10:53 AM

Sort includes in libcxx/include/__algorithm/equal.h

jloser added a comment.Aug 19 2021, 10:54 AM
In D108393#2955225, @cjdb wrote:
In D108393#2955223, @jloser wrote:

@cjdb thanks for the quick review. I don't have commit rights -- do you mind committing on my behalf, please?

Sure. Upload the new patch, and I'll take care of it post-CI.

Thanks -- just updated the patch to sort the includes.

Mordante accepted this revision as: Mordante.Aug 19 2021, 11:01 AM
Mordante added a subscriber: Mordante.

LGTM, provided the build passes.

ldionne accepted this revision.Aug 19 2021, 11:46 AM
This revision is now accepted and ready to land.Aug 19 2021, 11:46 AM
• Quuxplusone added inline comments.Aug 19 2021, 11:53 AM
libcxx/include/__algorithm/equal.h
16–17

You need <type_traits> in both places now, for add_lvalue_reference_t. Otherwise the modules build breaks.

jloser updated this revision to Diff 367581.Aug 19 2021, 12:05 PM

Include <type_traits> for add_lvalue_reference

jloser marked 2 inline comments as done.Aug 19 2021, 12:05 PM
jloser added inline comments.
libcxx/include/__algorithm/equal.h
16–17

Fixed. Good spot, @Quuxplusone.

• Quuxplusone mentioned this in D108400: [libc++] Eliminate needless `add_lvalue_reference` from <algorithm> helpers. NFCI..Aug 19 2021, 12:11 PM
Harbormaster completed remote builds in B120397: Diff 367581.Aug 19 2021, 2:13 PM
jloser added a comment.Aug 19 2021, 3:05 PM

@Quuxplusone do you want to land this and then have https://reviews.llvm.org/D108400 rebase with it or just pursue https://reviews.llvm.org/D108400 instead of this? CI just passed for this diff.

• Quuxplusone added a comment.Aug 19 2021, 6:26 PM

@Quuxplusone do you want to land this and then have D108400 rebase with it or just pursue D108400 instead of this? CI just passed for this diff.

Once D108400 gets the OK from @ldionne I'll land it. If D108393 lands first, I'll rebase. If D108400 lands first, I'll close D108393. I don't think it matters whether D108393 is landed or not.

jloser added a comment.Aug 19 2021, 8:04 PM
In D108393#2956426, @Quuxplusone wrote:

@Quuxplusone do you want to land this and then have D108400 rebase with it or just pursue D108400 instead of this? CI just passed for this diff.

Once D108400 gets the OK from @ldionne I'll land it. If D108393 lands first, I'll rebase. If D108400 lands first, I'll close D108393. I don't think it matters whether D108393 is landed or not.

Fair enough. I don't have commit rights, so I'll need someone else to commit this on my behalf if we want to land this standalone.

Closed by commit rG9807626bc721: [libc++] Include <__iterator/distance.h> instead of <iterator> in a few… (authored by • joe_loser, committed by ldionne). · Explain WhyAug 20 2021, 12:02 PM
This revision was automatically updated to reflect the committed changes.
ldionne added a commit: rG9807626bc721: [libc++] Include <__iterator/distance.h> instead of <iterator> in a few….

Revision Contents

PathSize
libcxx/
include/
__algorithm/
3 lines
3 lines

Diff 367855

libcxx/include/__algorithm/equal.h

// -*- C++ -*- // -*- C++ -*-
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// //
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information. // See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
#ifndef _LIBCPP___ALGORITHM_EQUAL_H #ifndef _LIBCPP___ALGORITHM_EQUAL_H
#define _LIBCPP___ALGORITHM_EQUAL_H #define _LIBCPP___ALGORITHM_EQUAL_H
#include <__config> #include <__config>
#include <__algorithm/comp.h> #include <__algorithm/comp.h>
#include <__iterator/distance.h>
#include <__iterator/iterator_traits.h> #include <__iterator/iterator_traits.h>
#include <iterator> // FIXME: replace with <__iterator/distance.h> when it lands #include <type_traits>
cjdbUnsubmitted
Done
ReplyInline Actions

Please sort these alphabetically.

cjdb: Please sort these alphabetically.
QuuxplusoneUnsubmitted
Done
ReplyInline Actions

You need <type_traits> in both places now, for add_lvalue_reference_t. Otherwise the modules build breaks.

Quuxplusone: You need `<type_traits>` in both places now, for `add_lvalue_reference_t`. Otherwise the…
jloserAuthorUnsubmitted
Done
ReplyInline Actions

Fixed. Good spot, @Quuxplusone.

jloser: Fixed. Good spot, @Quuxplusone.
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
#pragma GCC system_header #pragma GCC system_header
#endif #endif
_LIBCPP_PUSH_MACROS _LIBCPP_PUSH_MACROS
#include <__undef_macros> #include <__undef_macros>
▲ Show 20 Lines • Show All 66 Lines • Show Last 20 Lines

libcxx/include/__algorithm/is_permutation.h

// -*- C++ -*- // -*- C++ -*-
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
// //
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information. // See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
// //
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
#ifndef _LIBCPP___ALGORITHM_IS_PERMUTATION_H #ifndef _LIBCPP___ALGORITHM_IS_PERMUTATION_H
#define _LIBCPP___ALGORITHM_IS_PERMUTATION_H #define _LIBCPP___ALGORITHM_IS_PERMUTATION_H
#include <__algorithm/comp.h> #include <__algorithm/comp.h>
#include <__config> #include <__config>
#include <__iterator/distance.h>
#include <__iterator/iterator_traits.h> #include <__iterator/iterator_traits.h>
#include <__iterator/next.h> #include <__iterator/next.h>
#include <iterator> // FIXME: replace with <__iterator/distance.h> when it lands #include <type_traits>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
#pragma GCC system_header #pragma GCC system_header
#endif #endif
_LIBCPP_PUSH_MACROS _LIBCPP_PUSH_MACROS
#include <__undef_macros> #include <__undef_macros>
▲ Show 20 Lines • Show All 143 Lines • Show Last 20 Lines