-3

When I run the following code:

std::string myString = "I'm a string.";
const std::string::iterator &myIterator = ++myString.begin();
char c = *myIterator;
std::cout << c << std::endl;

I get a segmentation fault (compiling with O3 optimization). I assume this is because the ++ operator returns a std::string::iterator & rather than a std::string::iterator, and so we get a reference to a temporary. Is there a reason why this is not implemented to cause a compile error? I.e., why isn't the signature the following?

std::string::iterator &std::string::iterator::operator++() &;

Or even better, why doesn't the spec require the following signatures so that we can handle rvalues without a problem?

std::string::iterator &std::string::iterator::operator++() &;
std::string::iterator std::string::iterator::operator++() &&;
Jeremy B.
  • 81
  • 1
  • 7
  • 4
    [Cannot reproduce](https://ideone.com/I9Z8De). Are you sure that it is because of those lines? On a side note, even if `++` returns a reference, `myIterator` is not a reference so it will copy the value – Rakete1111 May 31 '16 at 04:40
  • 3
    This is working fine. Can you post complete code? – Sumeet May 31 '16 at 04:41
  • Yes, you are right. I have modified the example, and it should crash now. – Jeremy B. May 31 '16 at 06:42
  • I'm not sure how your proposed set of function signature would solve the problem - assigning either of them to a reference would still result in a dangling reference. As you mention in a comment, it would be nice for the compiler to issue a warning about assigning a temporary that will immediately be destroyed to a reference. But that's really a separate issue. – Michael Burr May 31 '16 at 07:48
  • It seems to me like the second one would work, since when we bind an rvalue to a reference, the lifetime of the temporary is extended. – Jeremy B. May 31 '16 at 17:58
  • 1
    I agree that it would avoid the error to include those overloads ... but doing that consistently for the whole standard library would be a nightmare, and perhaps also introduce breaking changes to existing code. You're probably stuck with remembering to be careful when declaring const references (and also, there's never any reason to use a const reference to iterator - they're designed to be lightweight and pass by value). – M.M May 31 '16 at 23:37
  • 1
    IMO allowing `++` to be used on prvalue in the first place is a bit fishy, the builtin ++ can only be used on lvalues. But many people seem happy with it. – M.M May 31 '16 at 23:39
  • 1
    In fact it's an implementation detail as to whether this code even compiles: a library could use `char *` as string iterator. – M.M May 31 '16 at 23:41
  • Thank you; that clears it up. I had figured out why my first proposal could cause breaking changes (say, if we did `char c = *(++myString.begin())`), but your other two comments effectively demonstrate how my proposed idiom would lead to inconsistency. – Jeremy B. Jun 01 '16 at 04:24
  • See the comments for http://stackoverflow.com/questions/31359829/why-does-get-helper-of-stdtuple-return-rvalue-reference-instead-of-value/31360610?noredirect=1#comment50703282_31360610 for more discussion on this topic. – Jeremy B. Jun 18 '16 at 06:05
  • No [mcve] then it did not happen – Slava Aug 26 '16 at 17:40

2 Answers2

0

The code you've posted looks legal to me: std::string::iterator myIterator = ++myString.begin(); copies the iterator reference into myIterator so there are no dangling references to temporaries.

My psychic debugging powers tell me that your actual code has one of two problems:

  • Your string is actually empty even though you think it has text in it. In that case ++begin() is invalid.
  • You mutate the string after you get the iterator copy in a way that invalidates the iterator.
Mark B
  • 95,107
  • 10
  • 109
  • 188
  • My psychic prediction is that the code example was supposed to be `std::string::iterator& myIterator = ++myString.begin();` in which case there actually is a dangling reference. – Michael Burr May 31 '16 at 05:26
  • Good call (though I also had a `const` originally). I'm still not sure why this should successfully compile, though. – Jeremy B. May 31 '16 at 06:32
0

@M.M's comments are insightful. In particular, my second suggestion would restrict libraries' options in implementing iterators, since iterators are not required to be incrementable as rvalues (reference: http://en.cppreference.com/w/cpp/iterator/next), and in fact, std::string::iterator could be implemented as a char *, which is not incrementable as an rvalue.

The choice of whether my first suggestion is followed seems to be a library implementation decision, since the standard mandates what iterators can do, not what they can't, in order not to unnecessarily restrict what types can function as iterators. Of course, at this point, changing the implementation could cause breaking changes for users' code.

Jeremy B.
  • 81
  • 1
  • 7