Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement vector constructor to support single-pass input iterator range #160

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

winner245
Copy link
Contributor

@winner245 winner245 commented Oct 19, 2024

目前的 vector 实现并不真正支持只能单次使用的 input_iterator,目前的实现本质上要求输入迭代器必须至少是 forward_iterator。这个问题不仅存在于构造函数,也存在于其他接受迭代器range的成员函数如 insert()/assign(),以及其他容器中。

此 PR 只对 vector ctor 增加了 input_iterator 支持。如果仓库作者愿意接受这个特性支持的话,我很乐意在此方向上继续贡献代码。

@winner245
Copy link
Contributor Author

winner245 commented Oct 21, 2024

我上面说 assign(b, e) 不支持 input iterator,不算完全准确,确切地说是目前的 assign() 对 input iterator 的支持是有 bug 的,表面上看用 tag dispatching 实现了两个 copy_assign() 重载,但是针对 input iterator 的重载 copy_assign(,,input_iterator_tag) 实际上是有 bug 的,因为它在 else 分支调用的是 insert(),而 insert() 并没有真正支持 input iterator,所以一旦走到这个分支就会出错。最简单的测试就是对空 vector 调用 assign(input_iter_b, intput_iter_e)@Peas-Li

@Peas-Li
Copy link

Peas-Li commented Oct 21, 2024

都是支持的吧,您修改的vector(Iter first, Iter last),在作者的代码中虽然只是调用了range_init,但是range_init调用了uninitialized_copy,uninitialized_copy实现了对内置类型和自定义类型的重载,前者又调用了mystl::copy,这个函数实现了三个版本的重载,其中就有对输入迭代器的版本实现

@winner245
Copy link
Contributor Author

@Peas-Li range_init() 一开始就用了 distance(first, last),而 input_iterator 是单次使用,用完后就走到了尽头。后面 init_space() 分配一段未初始化内存,而 uninitialized_copy() 会立即返回。

@Peas-Li
Copy link

Peas-Li commented Oct 21, 2024

确实是,明白了,我对初始化为6个int长度的vector,用assign进行8个in长度istringstream的覆盖:
std::istringstream inputStream("6 2 3 4 5 5 7 8");
mystl::istream_iterator beg(inputStream), end;
mystl::vector v11({9,8,7,6,5,4});
v11.assign(beg, end);
为什么会输出七个长度:
v11 : 6 2 3 4 5 5 7

@winner245
Copy link
Contributor Author

winner245 commented Oct 21, 2024

这是由 std::istream_iterator 的工作方式决定的。它的构造函数 istream_iterator(istream_type& is) 会先读一次,并将读到 value 缓存下来。它的 operator* 是直接读取缓存的 value,不会读输入流,真正读输入流的是++ (只有 ++ 操作能耗尽输入流)。一旦输入流耗尽,执行++的迭代器就走到了尽头。注意我说的是 “执行++的迭代器”,这并不包含从它拷贝得来的其他迭代器。因为 istream_iterator 的 copy constructor 是浅拷贝,每份拷贝的迭代器都会将从源头迭代器那里拷贝的缓存 value 也缓存下来。因此,即便源头迭代器走完,其他浅拷贝的迭代器依然能解引用读取缓存的 value,只有当这个迭代器也执行一次 ++ 后,该迭代器才立即走到尽头 (只有++操作会检测输入流是否已经抵达了 EOS)。在你的测试代码中,std::distance() 让传递给该函数内的迭代器走到了尽头,但源头迭代器还缓存着第 7 个 value,所以 copy_insert 函数会将第 7 个 value 也插入 (这既不是 copy_insert 的锅,也不是 istream_iterator 的锅,而是 assign() 针对 input_iterator 的重载有问题,导致执行了不当的代码)。要获得深刻理解,得自行看 std::istream_iterator 的源码。

下面的测试代码说明了一切:

std::istringstream is("1 2 3");
std::istream_iterator beg{is}, end;
std::istream_iterator iter1{beg}, iter2{beg};

while (beg != end) {
std::cout << *beg++ << ' ';
}
std::cout << '\n';

std::cout << "beg == end ? " << std::boolalpha << (beg == end) << '\n';

while (iter1 != end) {
std::cout << *iter1++ << '\n';
}
std::cout << "iter1 == end ? " << (iter1 == end) << '\n';

while (iter2 != end) {
std::cout << *iter2++ << '\n';
}
std::cout << "iter2 == end ? " << (iter2 == end) << '\n';

@Peas-Li
Copy link

Peas-Li commented Oct 22, 2024

看了你写的istream_iterator 和std里面的istream_iterator ,确实都存在这个问题,只有在不正当使用输入迭代器才会出现这样的情况,所以这个仓库不只是vector,list,deque等都有问题,作者习惯用输入迭代器计算distance,再给容器赋值

@Peas-Li
Copy link

Peas-Li commented Oct 22, 2024

甚至你可以看到在我的pull requests中指明了_deque::insert_dispatch函数中,对输入迭代器执行了--操作

@frederick-vs-ja
Copy link
Contributor

@winner245 @Peas-Li 两个 PR 有很多共同部分。要不先提取出公共部分来 PR?
我尽量联系 @Alinshans review 下。

@winner245
Copy link
Contributor Author

好的,那我来把公共部分的 PR 提交以下

@winner245
Copy link
Contributor Author

已完成 rebase 操作

@winner245
Copy link
Contributor Author

重新修改了 input iterator 版 ctor 的实现,让其调用 range_init(,, input_iterator_tag) 重载,因此,将原来的 range_init 修改为了如下两个具有不同 iterator tag 的重载版:

range_init(,, input_iterator_tag);
range_init(,, forward_iterator_tag);

相信现在的版本更能与现有 code base 有机结合在一起。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants