[PATCH] Add limitations to loop convert user doc

[Jack Yang]

  Addressed review comments:
  - more meaningful subheadings
  - replaced 'arrow operator' with 'operator->()'
  - grammatical issues

Hi revane, silvas, gribozavr,

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

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D552?vs=1360&id=1367#toc

Files:
  docs/LoopConvertTransform.rst

Index: docs/LoopConvertTransform.rst
===================================================================
--- docs/LoopConvertTransform.rst
+++ docs/LoopConvertTransform.rst
@@ -17,9 +17,6 @@
 Risk
 ====
 
-TODO: Add code examples for which incorrect transformations are performed
-when the risk level is set to "Risky" or "Reasonable".
-
 Risky
 ^^^^^
 
@@ -41,6 +38,11 @@
   for (int i = 0; i < obj.getVector().size(); ++i)
     obj.foo(10); // using 'obj' is considered risky
 
+See
+:ref:`Range-based loops evaluates end() only once<IncorrectRiskyTransformation>`
+for an example of an incorrect transformation when the maximum acceptable risk
+level is set to `risky`.
+
 Reasonable (Default)
 ^^^^^^^^^^^^^^^^^^^^
 
@@ -118,3 +120,139 @@
   for (auto & elem : v)
     cout << elem;
 
+Limitations
+===========
+
+There are certain situations where the tool may erroneously perform
+transformations that remove information and change semantics. Users of the tool
+should be aware of the behaviour and limitations of the transform outlined by
+the cases below.
+
+Comments inside loop headers are not preserved
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Comments inside the original loop header are ignored and deleted when
+transformed.
+
+.. code-block:: c++
+
+  for (int i = 0; i < N; /* This will be deleted */ ++i) { }
+
+Range-based loops evaluates end() only once
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+The C++11 range-based for loop calls ``.end()`` only once during the
+initialization of the loop. If in the original loop ``.end()`` is called after
+each iteration the semantics of the transformed loop may differ.
+
+.. code-block:: c++
+
+  // The following is semantically equivalent to the C++11 range-based for loop,
+  // therefore the semantics of the header will not change.
+  for (iterator it = container.begin(), e = container.end(); it != e; ++it) { }
+
+  // Instead of calling .end() after each iteration, this loop will be
+  // transformed to call .end() only once during the initialization of the loop,
+  // which may affect semantics.
+  for (iterator it = container.begin(); it != container.end(); ++it) { }
+
+.. _IncorrectRiskyTransformation:
+
+As explained above, calling member functions of the container in the body
+of the loop is considered `risky`. If the called member function modifies the
+container the semantics of the converted loop will differ due to ``.end()``
+being called only once.
+
+.. code-block:: c++
+
+  bool flag = false;
+  for (vector<T>::iterator it = vec.begin(); it != vec.end(); ++it) {
+    // Add a copy of the first element to the end of the vector.
+    if (!flag) {
+      // This line makes this transformation 'risky'.
+      vec.push_back(*it);
+      flag = true;
+    }
+    cout << *it;
+  }
+
+The original code above prints out the contents of the container including the
+newly added element while the converted loop, shown below, will only print the
+original contents and not the newly added element.
+
+.. code-block:: c++
+
+  bool flag = false;
+  for (auto & elem : vec) {
+    // Add a copy of the first element to the end of the vector.
+    if (!flag) {
+      // This line makes this transformation 'risky'
+      vec.push_back(elem);
+      flag = true;
+    }
+    cout << elem;
+  }
+
+Semantics will also be affected if ``.end()`` has side effects. For example, in
+the case where calls to ``.end()`` are logged the semantics will change in the
+transformed loop if ``.end()`` was originally called after each iteration.
+
+.. code-block:: c++
+
+  iterator end() {
+    num_of_end_calls++;
+    return container.end();
+  }
+
+Overloaded operator->() with side effects
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Similarly, if ``operator->()`` was overloaded to have side effects, such as
+logging, the semantics will change. If the iterator's ``operator->()`` was used
+in the original loop the dot operator of a container element will be used
+instead due to the implicit dereference as part of the range-based for loop.
+Therefore any side effect of the overloaded ``operator->()`` will no longer be
+performed.
+
+.. code-block:: c++
+
+  for (iterator it = c.begin(); it != c.end(); ++it) {
+    it->func(); // Using operator->()
+  }
+  // Will be transformed to:
+  for (auto & elem : c) {
+    elem.func(); // No longer using operator->()
+  }
+
+
+Pointers and references circumvent risk analysis
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+While most of the transform's risk analysis is dedicated to determining whether
+the iterator or container was modified within the loop, it is possible to
+circumvent the analysis by accessing and modifying the container through a
+pointer or reference.
+
+If the container were directly used instead of using the pointer or reference
+the following transformation would have only been applied at the ``-risk=risky``
+level since calling a member function of the container is considered `risky`.
+The transform cannot identify expressions associated with the container that are
+different than the one used in the loop header, therefore the transformation
+below ends up being performed at the ``-risk=safe`` level.
+
+.. code-block:: c++
+
+  vector<int> vec;
+
+  vector<int> *ptr = &vec;
+  vector<int> &ref = vec;
+
+  for (vector<int>::iterator it = vec.begin(), e = vec.end(); it != e; ++it) {
+    if (!flag) {
+      // Accessing and modifying the container is considered risky, but the risk
+      // level is not raised here.
+      ptr->push_back(*it);
+      ref.push_back(*it);
+      flag = true;
+    }
+  }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D552.4.patch
Type: text/x-patch
Size: 5598 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130322/c2bb28f5/attachment.bin>