Add workaround for MSVC bug

[hughbe]

See swiftlang/swift#3759 (comment)

The problem is that MSVC doesn't recognise the following syntax, reporting multiple errors here:

  size_t numTrailingObjects(
      typename TrailingObjects::template OverloadToken<Identifier>) const {
    return asDerived().getNumArguments();
  }

For some reason, MSVC ignores the fact that we gave friend access to llvm::TrailingObjects here.

Note that the Swift source code also has to be modified, as this is not enough. This is the new source code, that fully compiles after this change!

  // Work around MSVC bug: can't infer llvm::trailing_objects_internal, even though we granted friend access to it
	size_t numTrailingObjects(
#if defined(_MSC_VER)
		llvm::trailing_objects_internal::TrailingObjectsBase::OverloadToken<Identifier>) const {
#else
		typename TrailingObjects::template OverloadToken<Identifier>) const {
#endif
    return asDerived().getNumArguments();
  }

@DougGregor

[jrose-apple]

Changes to LLVM really shouldn't happen here; they should go to llvm.org. The fact that they haven't made this change suggests to me that this isn't the only thing necessary to support, um, whatever version of MSVC you're on.

[hughbe]

Hi Jordan, thanks for taking a look. I'm using the very latest version of MSVC:
Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24215.1 for x86

I considered sending this PR upstream to the LLVM folks, but this exposes something as protected that really shouldn't be. Since this is a blocker for Swift only, I thought our fork would be a better fit for this fix.

The fact that they haven't made this change

Perhaps this error is only hit in the Swift source code, or MSVC only hits this bug in the Swift source code.

By the way, I've taken a look at the source code for where I'm hitting this bug - its probably too much to ask but could you take a look, just verifying that @DougGregor has written valid C++ (I'm sure you have) :D

[hyp]

@hughbe I think I saw this kind f error before, can you point me to where it's located? I think the class forgot to friend the TrailingObjects, which is what is typically done in clang.

[hughbe]

@hyp swiftlang/swift#3759 (diff)

Thanks!!

[hughbe]

Unfortunately, Doug did friend TrailingObjects: https://github.com/apple/swift/pull/3759/files#diff-323bdb4c6d842e3b3cd3b40e628c9771R633

[hyp]

Oh yeah, I see, then it probably is a MSVC issue indeed.

[hyp]

@hughbe I'm not sure if that will fix anything, but you might wanna try adding friend TrailingObjects to all classes that derive from TrailingCallArguments and see if that helps.

[DougGregor]

Clang uses TrailingObjects quite a bit... is there a canonical workaround that has been figured out in that code base?

[hyp]

@DougGregor AFAIK all classes that use TrailingObjects are final, so they don't have this problem. I just found one known MSVC issue that's related to TrailingObjects (https://github.com/llvm-mirror/clang/blob/a896e9a7d27788e7573f3ca13066cc5fc6d91da6/include/clang/CodeGen/CGFunctionInfo.h#L519), but that class is still final.

[hughbe]

Hi all, sorry for the delay.

I tried out your suggest @hyp. This is what I got from MSVC:

I also got 84 errors of:

error C2248: 'swift::TrailingCallArguments<swift::UnresolvedMemberExpr>::TrailingObjects': cannot access private typedef declared in class 'swift::TrailingCallArguments<swift::UnresolvedMemberExpr>'

Let me know what you guys think - MSVC really doesn't seem to like TrailingObjects!

[hughbe]

Hi all: what shall we do about this one? Whilst this deviates this fork from the actual LLVM source, it is necessary to build Swift using MSVC (so far, no workarounds work!)

[jrose-apple]

This still bothers me. I don't want to maintain this change out of LLVM tree, but that means taking it upstream and figuring out what to do there, or at the very least adding a test to their unit tests to make sure they don't break it in the future.

[hughbe]

Okay - I've made a connect bug for this: https://connect.microsoft.com/VisualStudio/feedback/details/3116517

If anyone's curious, they can try coming up with a better workaround by compiling MSVC code online here: http://rextester.com/XIJ43194

Thanks!

[jrose-apple]

Thank you for filing this (these).

[hughbe]

Hopefully someone might tinker with the example and get a prettier workaround - I've genuinely spent hours trying but I assume someone with a fresh mind could help!

[hughbe]

Closing, this is targeted to the wrong branch (stable) instead of swift-4.0-branch