[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