[llvm] r279312 - Reapply "ADT: Tidy up ilist_traits static asserts, NFC"

[Duncan P. N. Exon Smith via llvm-commits]

Author: dexonsmith
Date: Fri Aug 19 15:17:23 2016
New Revision: 279312

URL: http://llvm.org/viewvc/llvm-project?rev=279312&view=rev
Log:
Reapply "ADT: Tidy up ilist_traits static asserts, NFC"

This spiritually reapplies r279012 (reverted in r279052) without the
r278974 parts.  The differences:

  - Only the HasGetNext trait exists here, so I've only cleaned up (and
    tested) it.  I still added HasObsoleteCustomization since I know
    this will be expanding when r278974 is reapplied.

  - I changed the unit tests to use static_assert to catch problems
    earlier in the build.

  - I added negative tests for the type traits.

Original commit message follows.

----

Change the ilist traits to use decltype instead of sizeof, and add
HasObsoleteCustomization so that additions to this list don't
need to be added in two places.

I suspect this will now work with MSVC, since the trait tested in
r278991 seems to work.  If for some reason it continues to fail on
Windows I'll follow up by adding back the #ifndef _MSC_VER.

Modified:
    llvm/trunk/include/llvm/ADT/ilist.h
    llvm/trunk/unittests/ADT/ilistTest.cpp

Modified: llvm/trunk/include/llvm/ADT/ilist.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ilist.h?rev=279312&r1=279311&r2=279312&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/ilist.h (original)
+++ llvm/trunk/include/llvm/ADT/ilist.h Fri Aug 19 15:17:23 2016
@@ -85,12 +85,15 @@ template <class TraitsT, class NodeT> st
   template <size_t N> struct SFINAE {};
 
   template <class U>
-  static Yes &hasGetNext(
-      SFINAE<sizeof(static_cast<NodeT *>(make<U>().getNext(&make<NodeT>())))>
-          * = 0);
-  template <class U> static No &hasGetNext(...);
+  static Yes &test(U *I, decltype(I->getNext(&make<NodeT>())) * = 0);
+  template <class> static No &test(...);
 
-  static const bool value = sizeof(hasGetNext<TraitsT>(nullptr)) == sizeof(Yes);
+public:
+  static const bool value = sizeof(test<TraitsT>(nullptr)) == sizeof(Yes);
+};
+
+template <class TraitsT, class NodeT> struct HasObsoleteCustomization {
+  static const bool value = HasGetNext<TraitsT, NodeT>::value;
 };
 
 } // end namespace ilist_detail
@@ -378,12 +381,11 @@ template<typename NodeTy> struct simplif
 ///
 template <typename NodeTy, typename Traits = ilist_traits<NodeTy>>
 class iplist : public Traits, ilist_node_access {
-#if !defined(_MSC_VER)
-  // FIXME: This fails in MSVC, but it's worth keeping around to help
-  // non-Windows users root out bugs in their ilist_traits.
-  static_assert(!ilist_detail::HasGetNext<Traits, NodeTy>::value,
-                "ilist next and prev links are not customizable!");
-#endif
+  // TODO: Drop this assertion and the transitive type traits anytime after
+  // v4.0 is branched (i.e,. keep them for one release to help out-of-tree code
+  // update).
+  static_assert(!ilist_detail::HasObsoleteCustomization<Traits, NodeTy>::value,
+                "ilist customization points have changed!");
 
   mutable NodeTy *Head;
 

Modified: llvm/trunk/unittests/ADT/ilistTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/ilistTest.cpp?rev=279312&r1=279311&r2=279312&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/ilistTest.cpp (original)
+++ llvm/trunk/unittests/ADT/ilistTest.cpp Fri Aug 19 15:17:23 2016
@@ -128,4 +128,25 @@ TEST(ilistTest, UnsafeClear) {
   EXPECT_EQ(6, List.back().Value);
 }
 
+struct Empty {};
+TEST(ilistTest, HasObsoleteCustomizationTrait) {
+  // Negative test for HasObsoleteCustomization.
+  static_assert(!ilist_detail::HasObsoleteCustomization<Empty, Node>::value,
+                "Empty has no customizations");
 }
+
+struct GetNext {
+  Node *getNext(Node *);
+};
+TEST(ilistTest, HasGetNextTrait) {
+  static_assert(ilist_detail::HasGetNext<GetNext, Node>::value,
+                "GetNext has a getNext(Node*)");
+  static_assert(ilist_detail::HasObsoleteCustomization<GetNext, Node>::value,
+                "Empty should be obsolete because of getNext()");
+
+  // Negative test for HasGetNext.
+  static_assert(!ilist_detail::HasGetNext<Empty, Node>::value,
+                "Empty does not have a getNext(Node*)");
+}
+
+} // end namespace