Workaround MSVC bug when using TrailingObjects from a template.

[jyknight]

Summary

MSVC appears to be getting confused as to whether OverloadToken is
supposed to be public or not.

Reported to microsoft by hughbe:
https://connect.microsoft.com/VisualStudio/feedback/details/3116517

[jyknight]

hughbe: I believe this is a better fix for the reported issue. Can you verify that it works for the real code in swift as well as the test-case?

[hughbe]

Nice - this is much cleaner than my attempt!

One nit: it seems like Clang (http://rextester.com/USGA76943) and GCC (http://rextester.com/PLDJ4987) can compile both conditional compilation blocks. Perhaps we can just get rid of the conditional compilation and use "template<typename T> using OverloadToken = typename ParentType::template OverloadToken<T>;"

I know it's uglier, but perhaps conditional compilation is ugly too!

I don't have access to a PC with VS right now (so can't fully test this with Swift).

However, I can confirm that MSVC compiles the original code that failed to compile with this change, according to an online compiler: http://rextester.com/LLI94126

[hughbe]

I'm sorry @jyknight

Swift still doesn't compile. The error is different this time thankfully:

#include "llvm/Support/TrailingObjects.h"

// Test the use of TrailingObjects with a template class. This
// previously failed to compile due to a bug in MSVC's member access
// control/lookup handling for OverloadToken.
template<typename Derived>
class Class5 : private llvm::TrailingObjects<Derived, float, int/*, void* */> {
  using TrailingObjects = typename llvm::TrailingObjects<Derived, float, int/*,  void* */>;
  friend TrailingObjects;

  size_t numTrailingObjects(
    typename TrailingObjects::template OverloadToken<float>) const {
    return 1;
  }

  size_t numTrailingObjects(
    typename TrailingObjects::template OverloadToken<int>) const {
    return 2;
  }
};

template class Class5<int>; // instantiate to trigger the error
// 1>consoleapplication1.cpp(17): error C2535: 'size_t Class5<Derived>::numTrailingObjects(TrailingObjects<BaseTy,TrailingTys...>::TrailingObjectsImpl<llvm::trailing_objects_internal::AlignmentCalcHelper<TrailingTys...>::Alignment,BaseTy,llvm::TrailingObjects<BaseTy,TrailingTys...>,BaseTy,TrailingTys...>::OverloadToken) const': member function already defined or declared
// 1>  consoleapplication1.cpp(11) : note: see declaration of 'Class5<Derived>::numTrailingObjects'
// 1>  consoleapplication1.cpp(20) : note: see reference to class template instantiation 'Class5<Derived>' being compiled

// Uncommenting either of the ", void*" generic parameters removes the errors.
// HOWEVER, uncommenting both restores the errors.

[hughbe]

Shoot, didn't mean to accept the revision, apologies

[jyknight]

I just noticed that the test case I wrote is passing a bogus first-argument to the template -- and so is your modified version in the comment above. So any errors resulting from that are probably not useful to examine. The testcase should've look like this instead:

I'm assuming the error coming from swift is actually from something else?

// Test the use of TrailingObjects with a template class. This                                                                                                                      
// previously failed to compile due to a bug in MSVC's member access                                                                                                                
// control/lookup handling for OverloadToken.                                                                                                                                       
template <typename Derived>
class Class5Tmpl : private llvm::TrailingObjects<Derived, float, int> {
  using TrailingObjects = typename llvm::TrailingObjects<Derived, float>;
  friend TrailingObjects;

  size_t numTrailingObjects(
      typename TrailingObjects::template OverloadToken<float>) const {
    return 1;
  }

  size_t numTrailingObjects(                                                                                                                                                              typename TrailingObjects::template OverloadToken<int>) const {
    return 2;
  }
};

class Class5 : public Class5Tmpl<Class5> {}

[hughbe]

Okay, considering this fixes the bug, I'm happy. Cheers, thanks for the fix! I'll make the necessary cherry-picking and upstream pulling to Swift

[hughbe]

Good to merge?

llvm/trunk/include/llvm/Support/TrailingObjects.h

#endif
   template <typename T>
   static size_t callNumTrailingObjects(const BaseTy *Obj,
                                        TrailingObjectsBase::OverloadToken<T>) {
     return Obj->numTrailingObjects(TrailingObjectsBase::OverloadToken<T>());
   }
 
 public:
   // Make this (privately inherited) member public.
+#ifndef _MSC_VER
   using ParentType::OverloadToken;
+#else
+  // MSVC bug prevents the above from working, at least up through CL
+  // 19.10.24629.
+  template <typename T>
+  using OverloadToken = typename ParentType::template OverloadToken<T>;
+#endif
 
   /// Returns a pointer to the trailing object array of the given type
   /// (which must be one of those specified in the class template). The
   /// array may have zero or more elements in it.
   template <typename T> const T *getTrailingObjects() const {
     verifyTrailingObjectsAssertions();
     // Forwards to an impl function with overloads, since member
     // function templates can't be specialized.

llvm/trunk/unittests/Support/TrailingObjectsTest.cpp

TEST(TrailingObjects, Realignment) {
   std::unique_ptr<char[]> P(new char[1000]);
   Class4 *C = reinterpret_cast<Class4 *>(P.get());
   EXPECT_EQ(C->getTrailingObjects<char>(), reinterpret_cast<char *>(C + 1));
   EXPECT_EQ(C->getTrailingObjects<long>(),
             reinterpret_cast<long *>(llvm::alignAddr(
                 reinterpret_cast<char *>(C + 1) + 1, alignof(long))));
 }
 }
+
+// Test the use of TrailingObjects with a template class. This
+// previously failed to compile due to a bug in MSVC's member access
+// control/lookup handling for OverloadToken.
+template <typename Derived>
+class Class5Tmpl : private llvm::TrailingObjects<Derived, float, int> {
+  using TrailingObjects = typename llvm::TrailingObjects<Derived, float>;
+  friend TrailingObjects;
+
+  size_t numTrailingObjects(
+      typename TrailingObjects::template OverloadToken<float>) const {
+    return 1;
+  }
+
+  size_t numTrailingObjects(
+      typename TrailingObjects::template OverloadToken<int>) const {
+    return 2;
+  }
+};
+
+class Class5 : public Class5Tmpl<Class5> {};