[PATCH] D96134: [ADT] Allow SmallPtrSet to be used with a std::insert_iterator

Aaron Ballman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 5 13:04:03 PST 2021


aaron.ballman added inline comments.


================
Comment at: llvm/unittests/ADT/SmallPtrSetTest.cpp:407
+  // Ensure that we can use SmallPtrSet with std::inserter().
+  std::copy(std::begin(Buf), std::end(Buf), std::inserter(Set, Set.begin()));
+
----------------
dblaikie wrote:
> I'd probably/prefer to make a direct call to the API, rather than testing it through a bunch of other container machinery. More isolated testing, clearer failures (not layers of template goo inside the standard library), etc.
> 
>  (& my minor preference is towards using member begin/end unless the fully generic ADL call is needed (in which case you need llvm::adl_* functions or the raw using+unqualified call))
Does the new test look better to you?


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

https://reviews.llvm.org/D96134



More information about the llvm-commits mailing list