Comments (14)
Might have to do with this line:
https://github.com/ericniebler/range-v3/blob/master/include/range/v3/istream_range.hpp#L63
It does an extraction in the constructor.
from range-v3.
Possible fix. Add a bool to istream_range to determine if the stream is up to date with the current position.
https://gist.github.com/blindley/b2e2f9cf18ceade29a77
from range-v3.
This might work, but it makes me uncomfortable since a mutating operation is happening in cursor::current
and cursor::done
, which are const
member functions. The mitigating factor is that istream_range
only has a non-const begin()
member, so mutation during iteration is implicitly allowed. This is just a strange place for the mutation to happen.
The interaction with the view::take
adaptor happens because the count and the iterator are both incremented before the new count gets compared to N
. We might be able to do something sneaky there, where the underlying iterator isn't incremented if it doesn't need to be. That feels wrong, too; it's likely to screw up something else. Maybe this "fix" is better, or maybe the answer is: Don't Do That. I need to think about it.
from range-v3.
IMO the root of this issue is the unexpected interaction of using a view (which shouldn't mutate anything), and the continuously mutating input range.
Is there a situation where not incrementing the iterator in take
, if it doesn't need to be incremented, leads to unexpected behavior? I've been trying to construct such a case for non-input ranges but the change doesn't seem to break anything (yet).
One shouldn't need to have a "deeper" understanding of take
's internals, and, on a first read of the code snippet above, I expected the input iterator to be advanced 6 times instead of 8. Views are also too tempting not to use them, so...
Maybe we should also check other views like take_while
or unique
. A mutating input range that can be constructed from an initializer_list might be very useful for testing these things.
from range-v3.
Is there a situation where not incrementing the iterator in
take
, if it doesn't need to be incremented, leads to unexpected behavior?
I'm pretty sure that's the wrong fix. I haven't found a case that breaks yet, but it feels all wrong.
I expected the input iterator to be advanced 6 times instead of 8.
I think it's advanced 6 times. That's not the problem.
from range-v3.
@blindley, regarding https://gist.github.com/blindley/b2e2f9cf18ceade29a77, I think you'll find that your suggested fix still has problems. Tell me what this code does (taking 0 elements instead of 3):
for (auto i : istream<std::string>(std::cin) | view::take(0))
std::cout << i << '\n';
for (auto i : istream<std::string>(std::cin) | view::take(0))
std::cout << i << '\n';
I would expect this code to leave std::cin
alone, but I'm pretty sure it won't.
I just don't think there's any good answer here.
from range-v3.
@ericniebler
I tested it with view::take(0)
and it seems to leave cin
alone, exactly as I would expect.
Did you have different results? Or is there something obvious I am missing in my test?
from range-v3.
Come to think of it, that previous test wasn't very convincing. Perhaps this will be.
from range-v3.
OK, try this:
#include <sstream>
#include <range/v3/core.hpp>
#include <range/v3/istream_range.hpp>
int main()
{
std::stringstream str{"1 2 3 4 5 6 7 8 9"};
if(ranges::empty(ranges::istream<int>(str)))
std::cout << "empty!\n";
if(ranges::empty(ranges::istream<int>(str)))
std::cout << "empty!\n";
if(ranges::empty(ranges::istream<int>(str)))
std::cout << "empty!\n";
int i;
str >> i;
std::cout << "first int : " << i << "\n";
}
For me this prints:
first int : 4
Merely asking the istream range if it's empty is reading an int and throwing it away.
from range-v3.
Okay, I concur. I'll see if that can be fixed.
from range-v3.
I've just noticed that the current version of istream_range
seems to suffer from the same problem with empty()
(not that I'm trying to excuse the bug in my own version).
As of now, the only potential solution that is coming to mind is really nasty. It would involve storing a string buffer in the range to serve as a peek into the std::istream
. Things would get really nasty for user-defined types where we have no way of knowing how much data their operator>>
overload might consume. I will continue thinking on this though.
from range-v3.
Right now, I think the best solution is to provide access to the cached value, so that if you stop reading values before the stream runs out, you have a way to get the item that would be lost. That's the best I've come up with.
from range-v3.
Yeah, that's probably much better than what I was thinking. Although I'm curious how it would work (as far as the interface goes), and if you mean to do that instead of, or in addition to something that would solve the problem presented in my original post.
from range-v3.
I don't think there is a solution to your problem. I think istream_range should have a member function to retrieve the cached element. And I think so the adaptors need a base()
member function to fetch the underlying adapted range. That's the best we can do, IMO.
from range-v3.
Related Issues (20)
- ranges::to eigen HOT 2
- C++23's range_adaptor_closure equivalent HOT 2
- Security disclosure policy
- Range-v3 fails to compile on the latest MSVC version with Visual Studio 2022 HOT 2
- CMAKE_CXX_STANDARD 20 and RANGES_CXX_STD 17 - does not work HOT 1
- pipe adaptor support for tuple_algorithm.hpp
- Nested `key_type` for lazy set algorithms formatting?
- UB in std::views::split HOT 7
- action::remove not included
- Error when using function return value directly HOT 1
- MSVC warning C4702: unreachable code with NDEBUG
- JVM / Python / OOP like group_by view
- operator = for ranges::views::concat result HOT 1
- Is there an approach to concat 2 vector? HOT 2
- ranges::views::chunk_by and std::views::chunk_by return different results HOT 1
- ranges::transform cannot be compiled with c++17
- With compiling with gcc >= 11.1 for C++20, constraint depending on itself when checking for member begin on constant ref_view around boost range
- Why isn't repeat_n a range adaptor?
- Implementation Request for ranges::views::pairwise and ranges::views::adjacent
- Make range-v3 a Bazel module (Support Bzlmod) HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from range-v3.