[libc++][ranges] Implement ranges::contains_subrange #66963

ZijunZhaoCCK · 2023-09-21T00:01:41Z

No description provided.

huixie90

Thank you for contributing! I have not looked at the tests yet. Please let me know if my comments on the header file is unclear. Thank you!

libcxx/include/__algorithm/ranges_contains_subrange.h

github-actions · 2023-09-27T00:07:33Z

✅ With the latest revision this PR passed the C/C++ code formatter.

libcxx/include/__algorithm/ranges_contains_subrange.h

shafik · 2023-10-02T17:42:35Z

Please make sure you add a description to your PR. This is what usually goes into the git log and we want those entries to be as descriptive and helpful for folks who read the git logs, thank you.

philnik777 · 2023-10-02T19:18:13Z

Please make sure you add a description to your PR. This is what usually goes into the git log and we want those entries to be as descriptive and helpful for folks who read the git logs, thank you.

Do you have any specific suggestions what should be in the message? The libc++ "I implement something" patches are most of the time quite straight-forward, since you can look at the standard and know what's going on. It's also the best source, since it's the authoritative document here, and is also what anybody who cares to actually look at the patch should check against for correctness.

var-const · 2023-10-13T22:49:43Z

Please make sure you add a description to your PR. This is what usually goes into the git log and we want those entries to be as descriptive and helpful for folks who read the git logs, thank you.

Do you have any specific suggestions what should be in the message? The libc++ "I implement something" patches are most of the time quite straight-forward, since you can look at the standard and know what's going on. It's also the best source, since it's the authoritative document here, and is also what anybody who cares to actually look at the patch should check against for correctness.

+1 -- I'm in favor of descriptive commit messages in general, but in this case I'm not sure what would be useful to add beyond "Implement ranges::contains_subrange".

var-const

Thank you for the patch!

libcxx/include/algorithm

libcxx/test/std/algorithms/alg.nonmodifying/alg.contains/ranges.contains_subrange.pass.cpp

libcxx/include/__algorithm/ranges_contains_subrange.h

ldionne · 2023-11-16T23:28:08Z

Gentle ping. There's outstanding feedback to address on this review. @ZijunZhaoCCK if you don't think you'll have time to pursue this PR anymore, it's all good but please let us know so we can assign it to someone else!

ZijunZhaoCCK · 2023-11-28T17:34:41Z

Gentle ping. There's outstanding feedback to address on this review. @ZijunZhaoCCK if you don't think you'll have time to pursue this PR anymore, it's all good but please let us know so we can assign it to someone else!

Sorry for the late reply! I can’t make it a priority right now, but I’ll definitely do it when things calm down, maybe in January. If someone want to do it right now, I can stop pursuing this PR.

mordante

Thanks for your patch!
I mainly glossed over the patch, and left some comments.

libcxx/include/__algorithm/ranges_contains_subrange.h

…ial errors

H-G-Hristov · 2024-01-05T09:57:51Z

Just a gentle reminder. You should probably also update the release notes, the status page and the feature test macro: #define __cpp_lib_ranges_contains 20XXXXL // also in <algorithm>

2. add several test cases: like when both range and subrange are empty 3. reformat 4. nit to fix

2. add several test cases: like when both range and subrange are empty 3. reformat 4. nit to fix 5. add ranges::contains_subrange() in module.modulemap.in

var-const

LGTM. Thanks a lot for pushing this through and addressing all the feedback!

libcxx/docs/Status/RangesAlgorithms.csv

var-const · 2024-02-13T02:19:54Z

Looks like the CI failure was fixed by #77672 -- merging main should resolve this.

2. add several test cases: like when both range and subrange are empty 3. reformat 4. nit to fix 5. add ranges::contains_subrange() in module.modulemap.in

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 21, 2023

ZijunZhaoCCK marked this pull request as ready for review September 21, 2023 21:21

ZijunZhaoCCK requested a review from a team as a code owner September 21, 2023 21:21

ZijunZhaoCCK self-assigned this Sep 21, 2023

ZijunZhaoCCK requested review from var-const and huixie90 September 21, 2023 21:22

huixie90 requested changes Sep 23, 2023

View reviewed changes

ldionne assigned huixie90 Sep 27, 2023

huixie90 requested changes Sep 29, 2023

View reviewed changes

ZijunZhaoCCK marked this pull request as draft October 2, 2023 05:42

philnik777 added the ranges Issues related to `<ranges>` label Oct 6, 2023

var-const changed the title ~~[libc++] Implement ranges::contains_subrange~~ [libc++][ranges] Implement ranges::contains_subrange Oct 13, 2023

var-const requested changes Oct 14, 2023

View reviewed changes

var-const assigned var-const and unassigned huixie90 and ZijunZhaoCCK Dec 13, 2023

mordante reviewed Dec 16, 2023

View reviewed changes

libcxx/include/__algorithm/ranges_contains_subrange.h Outdated Show resolved Hide resolved

libcxx/include/__algorithm/ranges_contains_subrange.h Show resolved Hide resolved

philnik777 reviewed Dec 18, 2023

View reviewed changes

libcxx/include/__algorithm/ranges_contains_subrange.h Outdated Show resolved Hide resolved

libcxx/include/__algorithm/ranges_contains_subrange.h Outdated Show resolved Hide resolved

ZijunZhaoCCK closed this Jan 2, 2024

ZijunZhaoCCK force-pushed the libcxx-ranges-contains-subrange branch from f713b60 to 67c2e35 Compare January 2, 2024 22:41

ZijunZhaoCCK added 4 commits January 2, 2024 14:42

[libc++] Implement ranges::contains_subrange

647f5fe

format

5a1d555

Add a newline

7cb4d06

1. call distance(r) directly 2. replace auto with int to avoid potent…

aee12dd

…ial errors

Merge branch 'llvm:main' into libcxx-ranges-contains-subrange

3abea8e

ZijunZhaoCCK and others added 15 commits January 8, 2024 18:15

Merge branch 'llvm:main' into libcxx-ranges-contains-subrange

c4085b3

update code

b0f9bac

Merge branch 'llvm:main' into libcxx-ranges-contains-subrange

92005d4

update code

72d92de

Merge branch 'llvm:main' into libcxx-ranges-contains-subrange

97b5563

1. Call ranges::search() in ranges::contains_subrange()

65dd43c

2. add several test cases: like when both range and subrange are empty 3. reformat 4. nit to fix

Merge branch 'llvm:main' into libcxx-ranges-contains-subrange

f68d62d

1. Call ranges::search() in ranges::contains_subrange()

cf639d1

2. add several test cases: like when both range and subrange are empty 3. reformat 4. nit to fix 5. add ranges::contains_subrange() in module.modulemap.in

Merge branch 'llvm:main' into libcxx-ranges-contains-subrange

f61a18f

1. Call ranges::search() in ranges::contains_subrange()

a5ecb2b

2. add several test cases: like when both range and subrange are empty 3. reformat 4. nit to fix 5. add ranges::contains_subrange() in module.modulemap.in

Merge branch 'llvm:main' into libcxx-ranges-contains-subrange

e3ec859

1. Call ranges::search() in ranges::contains_subrange()

9d64b67

2. add several test cases: like when both range and subrange are empty 3. reformat 4. nit to fix 5. add ranges::contains_subrange() in module.modulemap.in

Merge branch 'llvm:main' into libcxx-ranges-contains-subrange

a586862

1. Call ranges::search() in ranges::contains_subrange()

d13b6e1

2. add several test cases: like when both range and subrange are empty 3. reformat 4. nit to fix 5. add ranges::contains_subrange() in module.modulemap.in

Merge branch 'llvm:main' into libcxx-ranges-contains-subrange

384a487

ldionne marked this pull request as ready for review January 10, 2024 19:12

var-const approved these changes Feb 13, 2024

View reviewed changes

libcxx/docs/Status/RangesAlgorithms.csv Outdated Show resolved Hide resolved

libcxx/docs/Status/RangesAlgorithms.csv Outdated Show resolved Hide resolved

ZijunZhaoCCK and others added 7 commits February 12, 2024 22:22

Merge branch 'llvm:main' into libcxx-ranges-contains-subrange

7b66ab9

Merge branch 'llvm:main' into libcxx-ranges-contains-subrange

2b1c067

1. Call ranges::search() in ranges::contains_subrange()

16145b2

2. add several test cases: like when both range and subrange are empty 3. reformat 4. nit to fix 5. add ranges::contains_subrange() in module.modulemap.in

Merge branch 'llvm:main' into libcxx-ranges-contains-subrange

d359566

1. Call ranges::search() in ranges::contains_subrange()

a6fcbb5

2. add several test cases: like when both range and subrange are empty 3. reformat 4. nit to fix 5. add ranges::contains_subrange() in module.modulemap.in

Merge branch 'llvm:main' into libcxx-ranges-contains-subrange

a3de363

1. Call ranges::search() in ranges::contains_subrange()

423eeca

2. add several test cases: like when both range and subrange are empty 3. reformat 4. nit to fix 5. add ranges::contains_subrange() in module.modulemap.in

ZijunZhaoCCK merged commit a6b846a into llvm:main Feb 13, 2024

frederick-vs-ja mentioned this pull request Mar 28, 2024

<algorithm>: contains_subrange should not use std::distance #86833

Closed

[libc++][ranges] Implement ranges::contains_subrange #66963

[libc++][ranges] Implement ranges::contains_subrange #66963

Uh oh!

Conversation

ZijunZhaoCCK commented Sep 21, 2023

Uh oh!

huixie90 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shafik commented Oct 2, 2023

Uh oh!

philnik777 commented Oct 2, 2023

Uh oh!

var-const commented Oct 13, 2023

Uh oh!

var-const left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ldionne commented Nov 16, 2023

Uh oh!

ZijunZhaoCCK commented Nov 28, 2023

Uh oh!

mordante left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

H-G-Hristov commented Jan 5, 2024

Uh oh!

var-const left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

var-const commented Feb 13, 2024

Uh oh!

Uh oh!

github-actions bot commented Sep 27, 2023 •

edited

Loading