Skip to content
This repository was archived by the owner on Mar 22, 2023. It is now read-only.

string_view: implement iterator API for string_view#948

Merged
lukaszstolarczuk merged 3 commits intopmem:masterfrom
KFilipek:test-iterators
Oct 26, 2020
Merged

string_view: implement iterator API for string_view#948
lukaszstolarczuk merged 3 commits intopmem:masterfrom
KFilipek:test-iterators

Conversation

@KFilipek
Copy link
Copy Markdown
Contributor

@KFilipek KFilipek commented Oct 20, 2020

This PR introduce iterator API for string_view class.


This change is Reviewable

@KFilipek KFilipek marked this pull request as ready for review October 22, 2020 09:31
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 22, 2020

Codecov Report

Merging #948 into master will increase coverage by 0.23%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #948      +/-   ##
==========================================
+ Coverage   95.94%   96.17%   +0.23%     
==========================================
  Files          48       48              
  Lines        6138     6146       +8     
==========================================
+ Hits         5889     5911      +22     
+ Misses        249      235      -14     
Flag Coverage Δ
#tests_clang_debug_cpp17 96.02% <ø> (+0.18%) ⬆️
#tests_gcc_debug 92.57% <100.00%> (+0.20%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
include/libpmemobj++/string_view.hpp 100.00% <100.00%> (ø)
...j++/container/detail/concurrent_skip_list_impl.hpp 98.12% <0.00%> (+0.13%) ⬆️
...ude/libpmemobj++/container/concurrent_hash_map.hpp 94.57% <0.00%> (+1.10%) ⬆️
include/libpmemobj++/mutex.hpp 86.20% <0.00%> (+6.89%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63110eb...196ac21. Read the comment docs.

Copy link
Copy Markdown
Contributor

@igchor igchor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 4 files at r1.
Reviewable status: 3 of 4 files reviewed, 6 unresolved discussions (waiting on @KFilipek)


tests/external/libcxx/string.view/string.view.iterators/begin.pass.cpp, line 8 at r1 (raw file):

//
//===----------------------------------------------------------------------===//

Intel license


tests/external/libcxx/string.view/string.view.iterators/begin.pass.cpp, line 52 at r1 (raw file):

	test(string_view("123"));
	test(wstring_view(L"123"));
#if TEST_STD_VER >= 11

ifdef not needed


tests/external/libcxx/string.view/string.view.iterators/begin.pass.cpp, line 57 at r1 (raw file):

#endif

#if TEST_STD_VER > 11

.


tests/external/libcxx/string.view/string.view.iterators/end.pass.cpp, line 8 at r1 (raw file):

//
//===----------------------------------------------------------------------===//

.


tests/external/libcxx/string.view/string.view.iterators/end.pass.cpp, line 50 at r1 (raw file):

{
	typedef pmem::obj::string_view string_view;
#if defined(__cpp_lib_char8_t) && __cpp_lib_char8_t >= 201811L

not needed?


tests/external/libcxx/string.view/string.view.iterators/end.pass.cpp, line 71 at r1 (raw file):

#endif

#if TEST_STD_VER > 11

.

Copy link
Copy Markdown
Contributor Author

@KFilipek KFilipek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3 of 3 files at r3.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @igchor)


tests/external/libcxx/string.view/string.view.iterators/begin.pass.cpp, line 8 at r1 (raw file):

Previously, igchor (Igor Chorążewicz) wrote…

Intel license

Done.


tests/external/libcxx/string.view/string.view.iterators/begin.pass.cpp, line 52 at r1 (raw file):

Previously, igchor (Igor Chorążewicz) wrote…

ifdef not needed

Done.


tests/external/libcxx/string.view/string.view.iterators/begin.pass.cpp, line 57 at r1 (raw file):

Previously, igchor (Igor Chorążewicz) wrote…

.

Done.


tests/external/libcxx/string.view/string.view.iterators/end.pass.cpp, line 8 at r1 (raw file):

Previously, igchor (Igor Chorążewicz) wrote…

.

Done.


tests/external/libcxx/string.view/string.view.iterators/end.pass.cpp, line 50 at r1 (raw file):

Previously, igchor (Igor Chorążewicz) wrote…

not needed?

Done.


tests/external/libcxx/string.view/string.view.iterators/end.pass.cpp, line 71 at r1 (raw file):

Previously, igchor (Igor Chorążewicz) wrote…

.

Done.

Copy link
Copy Markdown
Member

@lukaszstolarczuk lukaszstolarczuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 4 files at r1, 3 of 3 files at r3.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @igchor)

Copy link
Copy Markdown
Contributor

@igchor igchor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @KFilipek)


include/libpmemobj++/string_view.hpp, line 73 at r3 (raw file):

	constexpr const_iterator end() const noexcept;
	constexpr const_iterator cend() const noexcept;
#ifdef XXX

You can add support for it in this PR, it;s quite trivial

Copy link
Copy Markdown
Contributor

@igchor igchor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @KFilipek)

@lukaszstolarczuk lukaszstolarczuk merged commit 55351ae into pmem:master Oct 26, 2020
@KFilipek KFilipek deleted the test-iterators branch November 25, 2020 10:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants