[PATCH] D12402: PR24595: clang-cl fails to compile vswriter.h header from Windows SDK 8.1 in 32 bit mode

[Reid Kleckner via cfe-commits]

rnk added inline comments.

================
Comment at: lib/Sema/SemaType.cpp:2272
@@ -2271,1 +2271,3 @@
   // calling convention.
+  bool is_ctor_or_dtor =
+      (Entity.getNameKind() == DeclarationName::CXXConstructorName) ||
----------------
s/is_ctor_or_dtor/IsCtorOrDtor/, it's the local variable convention.

================
Comment at: lib/Sema/SemaType.cpp:5857-5858
@@ -5860,3 +5856,4 @@
 
-  if (hasExplicitCallingConv(T))
-    return;
+  // MS compiler transforms __stdcall in ctors/dtors to __thiscall in 32 bit
+  // mode. We should do the same.
+  if ((CurCC == CC_X86StdCall) && IsCtorOrDtor &&
----------------
I think a more accurate statement would be that MSVC ignores explicit calling convention attributes on structors in all modes. Compile this example with MSVC for x64:
  struct HVA {
    double a, b, c, d;
  };
  struct A {
    __vectorcall A(HVA x);
    double f;
  };
  A::A(HVA x) : f(x.a + x.b + x.c + x.d) {}
Note the warning:
  warning C4166: illegal calling convention for constructor/destructor
And HVA is passed indirectly instead of in XMM0-3.

You should consider adding a warning like this when we adjust structor conventions from something other than `__stdcall`. MSVC doesn't appear to warn on `__stdcall` structors, probably because it is used accidentally in places like vswriter.h.

================
Comment at: lib/Sema/SemaType.cpp:5860
@@ +5859,3 @@
+  if ((CurCC == CC_X86StdCall) && IsCtorOrDtor &&
+      (Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment() &&
+       Context.getTargetInfo().getTriple().isArch32Bit()))
----------------
A better test for if we should treat structors specially would be:
  Context.getTargetInfo().getCXXABI().isMicrosoft()

It's really the MSVC C++ ABI that requires that structors have special conventions. I think it has to do with exception handling.

================
Comment at: lib/Sema/SemaType.cpp:5868
@@ +5867,3 @@
+    // __thiscall type to __cdecl for static methods.
+    CallingConv FromCC =
+        Context.getDefaultCallingConvention(IsVariadic, IsStatic);
----------------
In hindsight, a better name for this variable would be `DefaultCC`.

================
Comment at: lib/Sema/SemaType.cpp:5871-5875
@@ +5870,7 @@
+    ToCC = Context.getDefaultCallingConvention(IsVariadic, !IsStatic);
+    if (CurCC != FromCC || FromCC == ToCC)
+      return;
+
+    if (hasExplicitCallingConv(T))
+      return;
+  }
----------------
I think it would be better to wrap both ifs here in `if (!IsCtorOrDtor)`. We should always adjust ctors and dtors.


http://reviews.llvm.org/D12402