MSVC Ownership.h patch

Attached is a potential patch for fixing a move semantics bug with Visual Studio 2008 Express. It needs a little attention before applying, not least a correct version check for the compiler. However, I am uncomfortable about a couple of things.

i/ so far, no-one but me has reported the issue - is it something else with my setup?
ii/ ASTMultiPtr is not the only type using this idiom in this way, but is so far the only one needing patching.
iii/ Solving by casting could simply be hiding a deeper problem that the two types with the same name really are distinct to MSVC for some reason.
iv/ I don't like being the one to set a precedent of introducing compiler-specific fixes.
v/ This code is way too ugly to be used as mainline code, and kind of demands a version-specific check or dropping support for the compiler!

In the meantime, I propose to use this patch on my own system as it allows me to continue testing.
I'll take advice on whether/how it is worth cleaning up and applying to the trunk.

AlisdairM

msvc_ownership.patch (1.53 KB)

i/ so far, no-one but me has reported the issue - is it something else with my setup?
  
I have the same problem with VS2008 (full edition), so it is not only you.

Cédric

Well, we can't drop support for Visual C++ over a little bug like this. I'm perfectly happy to put your patch into Clang once it has a proper version check. This stuff will clean itself up once rvalue references become more prevalent.

  - Doug

Well in that case I'm going to have to ask the list what the best way to identify this compiler is?

As Doug knows I'm more familiar with a different brand of Windows compilers, and I know lots of compilers on Windows try to look like MSVC to get a clean run at the Windows headers.

Is there a precedent for this elsewhere in Clang/LLVM?

AlisdairM

We usually use "#ifdef _MSC_VER" to detect microsoft compilers. If you want to detect the windows platform, use #ifdef LLVM_ON_WIN32

-Chris

Thanks.

Updated patch attached that unconditionally tests for Microsoft compilers.
I have taken the lead from Olaf to move the cast logic into a separate
function, but retain the compiler checks.

AlisdairM

msvc_move_semantic.patch (1.51 KB)

Patch committed. Thanks, Alisdair!

  - Doug