[PATCH] D51881: [ADT] Made numerous methods of ImmutableList const

Vedant Kumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 10 13:25:55 PDT 2018


vsk added inline comments.


================
Comment at: unittests/ADT/ImmutableListTest.cpp:83
 
+struct NoCopyMoveAssignMoveAssign {
+  NoCopyMoveAssignMoveAssign() = default;
----------------
It might be simpler to use a shorter name here (maybe 'Unmodifiable'?) and to have a comment explaining that the struct has no move/copy constructors/assignment operators.


================
Comment at: unittests/ADT/ImmutableListTest.cpp:89
+  NoCopyMoveAssignMoveAssign *operator=(const NoCopyMoveAssignMoveAssign &) = delete;
+  NoCopyMoveAssignMoveAssign *operator=(const NoCopyMoveAssignMoveAssign &&) = delete;
+
----------------
Why do the *-assign operators not return references?


================
Comment at: unittests/ADT/ImmutableListTest.cpp:103
+  ImmutableList<const NoCopyMoveAssignMoveAssign &> L = f.create(N);
+  for (ImmutableList<const NoCopyMoveAssignMoveAssign &>::iterator It = L.begin(), E = L.end(); It != E; ++It) {
+    It->doNothing();
----------------
Nit, please clang-format diffs.


Repository:
  rL LLVM

https://reviews.llvm.org/D51881





More information about the llvm-commits mailing list