[PATCH] Add limitations to loop convert user doc

[Edwin Vane]

================
Comment at: docs/LoopConvertTransform.rst:167
@@ +166,3 @@
+
+As explained previously, calling the container in the body of the loop is
+considered `risky`. If the called member function modifies the container, the
----------------
Dmitri Gribenko wrote:
> ... calling member functions of the container
> 
> Or is it actually only calling non-const member functions?
The transform doesn't check if the member function is const or not. In fact, even if it were, there could be mutable member variables that might get changed or the function might const_cast away the const-ness of 'this'. It's safer just to bail out if any calls are made on the container.

================
Comment at: docs/LoopConvertTransform.rst:211
@@ +210,3 @@
+dereference an iterator, it may cause changes in semantics if ``operator*()``
+is overloaded.
+
----------------
Actually, iterator::operator*() is called by the implementation of the range-based for loop (see C++11 [stmt.ranged] p1). The real sticky point is where the original loop may have used iterator::operator->() the new loop will have all of these calls replaced with elem.<whatever> and if operator->() was doing logging or some other side-effect then semantics will be different.

I'd also use the term 'side effect' and use 'logging' as a specific example. We can only imagine what crazy stuff might exist out in the wild that causes things to break.

================
Comment at: docs/LoopConvertTransform.rst:216
@@ +215,3 @@
+
+While most of loop convert's risk analysis is dedicated to determining whether
+the iterator or container was modified, it is possible to circumvent the
----------------
Say 'transform' instead of 'loop convert'. The full proper name is Loop Convert Transform.

================
Comment at: docs/LoopConvertTransform.rst:221
@@ +220,3 @@
+
+The following transformation is performed at the ``-risk=safe`` level. If we had
+directly used the container instead of the pointer or reference, the
----------------
Sean Silva wrote:
> Can you give the user a bit more insight into why?
Avoid the 'royal we'.

================
Comment at: docs/LoopConvertTransform.rst:154
@@ +153,3 @@
+initialization of the loop. If in the original loop, ``.end()`` is called after
+each iteration, the semantics of the transformed loop will differ.
+
----------------
Use 'may' instead of 'will' at this point. This is not always the case. You're about to explain such a trouble situation.

================
Comment at: docs/LoopConvertTransform.rst:159
@@ +158,3 @@
+  // Instead of calling .end() after each iteration, it will be transformed
+  // to call it only once during the initialization of the loop, which will
+  // affect semantics.
----------------
Again 'may' instead of 'will'. 'it' => '.end()'

================
Comment at: docs/LoopConvertTransform.rst:158
@@ +157,3 @@
+
+  // Instead of calling .end() after each iteration, it will be transformed
+  // to call it only once during the initialization of the loop, which will
----------------
'it' => 'this loop'

================
Comment at: docs/LoopConvertTransform.rst:169
@@ +168,3 @@
+considered `risky`. If the called member function modifies the container, the
+semantics of the converted loop will differ due to this iterator limitation.
+
----------------
Let's be more specific than 'this iterator limitation'. Perhaps something like '...due to .end() only being called once at the start of the loop.

================
Comment at: docs/LoopConvertTransform.rst:175
@@ +174,3 @@
+  for (vector<T>::iterator it = vec.begin(); it != vec.end(); ++it) {
+    if (!flag) { // Add a copy of the first element to the end of the vector
+      vec.push_back(*it); // This line makes this transformation 'risky'
----------------
Maybe consider putting the comments on the line before or vertically aligning them to the right.

================
Comment at: docs/LoopConvertTransform.rst:197
@@ +196,3 @@
+
+This limitation may also arise if each call to ``.end()`` is logged. If
+``.end()`` is called after each iteration in the original loop, and if it does
----------------
I'd reword this paragraph. We're talking about several different limitations in this section. In this case we're talking about how semantics will be affected if ``.end()`` has side effects such as logging.


http://llvm-reviews.chandlerc.com/D552