[PATCH] D12577: std::string asan annotations

[Kostya Serebryany via llvm-commits]

kcc added a comment.

looks reasonable. Please avoid space-only and {}-only changes.

Hopefully Marshall can have a look too.


================
Comment at: include/string:1782
@@ +1781,3 @@
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
+      __p -= 8 / sizeof(value_type) ;
+#endif // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
----------------
this division deserves a comment

================
Comment at: include/string:2354
@@ +2353,3 @@
+    __annotate_new(__sz);
+    for (; __first != __last; ++__first, (void) ++__p) {
+      traits_type::assign(*__p, *__first);
----------------
looks like you are only adding {}
do you need to? 

================
Comment at: include/string:2470
@@ -2342,4 +2469,3 @@
     __invalidate_all_iterators();
-    if (__n_copy != 0)
-        traits_type::copy(_VSTD::__to_raw_pointer(__p),
-                          _VSTD::__to_raw_pointer(__old_p), __n_copy);
+    if (__n_copy != 0) {
+      traits_type::copy(_VSTD::__to_raw_pointer(__p),
----------------
{}-only change? 

================
Comment at: include/string:2705
@@ -2564,3 +2704,3 @@
     }
-    else
+    else {
         __grow_by_and_replace(__cap, __sz + __n - __cap, __sz, __sz, 0, __n, __s);
----------------
{}-only change? 

================
Comment at: include/string:2963
@@ -2810,1 +2962,3 @@
+            __annotate_grow(__sz, __sz + __n);
+
             if (__n_move != 0)
----------------
extra line? 

================
Comment at: include/string:2973
@@ -2817,2 +2972,3 @@
         }
+
         __sz += __n;
----------------
extra line? 

================
Comment at: include/string:3068
@@ -2908,1 +3067,3 @@
     {
+        if (__new_sz > __sz) {
+          __annotate_grow(__sz, __new_sz);
----------------
seems to be a common pattern. Hide it inside __annotate_*? 


http://reviews.llvm.org/D12577