[libcxx-commits] [PATCH] D114915: [libc++][NFC] Address clang-tidy warnings in include/

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__algorithm/move.h:68
         return _VSTD::__move_constexpr(__first, __last, __result);
-    } else {
-        return _VSTD::__rewrap_iter(__result,
-            _VSTD::__move(_VSTD::__unwrap_iter(__first),
-                          _VSTD::__unwrap_iter(__last),
-                          _VSTD::__unwrap_iter(__result)));
-    }
+        
+    return _VSTD::__rewrap_iter(__result,
----------------
Nit: please make sure to strip any trailing whitespace from the changed lines.


================
Comment at: libcxx/include/regex:2664-2672
     _LIBCPP_INLINE_VISIBILITY
-    basic_regex& operator=(const value_type* __p)
-        {return assign(__p);}
+    basic_regex& operator=(const value_type* __p) { assign(__p); return *this; }
 #ifndef _LIBCPP_CXX03_LANG
     _LIBCPP_INLINE_VISIBILITY
-    basic_regex& operator=(initializer_list<value_type> __il)
-        {return assign(__il);}
+    basic_regex& operator=(initializer_list<value_type> __il) { assign(__il); return *this; }
 #endif // _LIBCPP_CXX03_LANG
     template <class _ST, class _SA>
----------------
philnik wrote:
> Quuxplusone wrote:
> > What was clang-tidy complaining about here, specifically?
> > Don't these diffs change a tail-call into a non-tail-call, and isn't that generally a bad thing? (Of course in these cases the optimizer will DTRT anyway, but I bet it's a pessimization for `-O0` mode.)
> clang-tidy complains that `operator=()` does not return `*this` (it's another case of `misc-unconventional-assign-operator`). It does of course return `*this` through `assign`, but I think explicitly returning `*this` makes it look less weird. I thought it would compile to the same assembly, but that probably isn't the case in `-O0`. I'll revert the changes when I disable `misc-unconventional-assign-operator`. And what does DTRT stand for?
DTRT = Do The Right Thing (with a somewhat ironic connotation that the "right thing" is "obvious" and therefore can be handwaved).

(For the record, experimental evidence agrees that the right-hand side's assembly is the same at `-O1` and slightly worse at `-O0`: https://godbolt.org/z/x7rjTjz4a )


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114915/new/

https://reviews.llvm.org/D114915