[PATCH] D62369: [ADT] Enable set_difference() to be used on StringSet

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/unittests/ADT/SetOperationsTest.cpp:1-29
+//===- llvm/unittest/ADT/StringMapMap.cpp - StringMap unit tests ----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
----------------
This test probably should go where other StringSet tests are (presumably unittests/ADT/StringSetTest) - and I would /generally/ be in favor of not 'integration testing' that set_difference works with StringSet, but that StringSet exposes the appropriate kind of iterators/operations that set_difference requires in general?

Oh, I see, this llvm::set_difference doesn't just wrap std::set_difference, so it has different interface requirements. I'd /probably/ still test the surface area of StringSet directly - and certainly feel like code changes to StringSet would be tested in StringSet's tests.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62369