rnk added inline comments.
================
Comment at: lib/Sema/SemaType.cpp:5855-5856
@@ -5854,3 +5854,4 @@
CallingConv CurCC = FT->getCallConv();
- CallingConv FromCC =
+ CallingConv DefaultCC =
Context.getDefaultCallingConvention(IsVariadic, IsStatic);
+ CallingConv ToCC;
----------------
DefaultCC is only meaningful when we're dealing with non-structor member
functions. In the structor case, you're conflating it with ToCC. Let's sink the
DefaultCC definition down into the 'else' block, since it's only meaningful
there.
================
Comment at: lib/Sema/SemaType.cpp:5857
@@ -5856,5 +5856,3 @@
Context.getDefaultCallingConvention(IsVariadic, IsStatic);
- CallingConv ToCC = Context.getDefaultCallingConvention(IsVariadic,
!IsStatic);
- if (CurCC != FromCC || FromCC == ToCC)
- return;
+ CallingConv ToCC;
----------------
ToCC is always the same in both cases:
CallingConv ToCC = Context.getDefaultCallingConvention(IsVariadic, !IsStatic);
Let's just keep that here, and return early if CurrCC == ToCC, since the common
case is that we don't need to do anything.
================
Comment at: lib/Sema/SemaType.cpp:5864-5865
@@ +5863,4 @@
+ // bit MS ABI environment. This probably should be fixed.
+ if (Context.getTargetInfo().getTriple().isArch32Bit())
+ DefaultCC = CC_X86ThisCall;
+
----------------
This is reimplementing ASTContext::getDefaultCallingConv. Let's not duplicate
that logic.
================
Comment at: lib/Sema/SemaType.cpp:5867-5868
@@ +5866,4 @@
+
+ if (CurCC == DefaultCC)
+ return;
+
----------------
This can be covered by returning earlier if CurrCC == ToCC.
================
Comment at: lib/Sema/SemaType.cpp:5870-5872
@@ +5869,5 @@
+
+ // __declspec(dllexport) got transformed to CC_C and allowed.
+ if (CurCC == CC_C)
+ return;
+
----------------
This seems wrong, see this test case:
struct A {
__cdecl A(int x);
int f;
};
A::A(int x) : f(x) {}
32-bit MSVC turns it into thiscall.
I think what you're really seeing is that variadic constructors need to stay
cdecl. getDefaultCallingConvention already handles this case.
================
Comment at: lib/Sema/SemaType.cpp:5873-5877
@@ +5872,7 @@
+ return;
+
+ ToCC = DefaultCC;
+
+ // Issue a warning on ignored calling convention -- except of __stdcall.
+ // Again, this is what MS compiler does.
+ if (CurCC != CC_X86StdCall)
----------------
andreybokhanko wrote:
> I tried, but this leads to massive stability fails. I'm willing to
> investigate and fix them (if needed), but in a separate patch, OK?
I'd really rather get the *structor calling convention logic right on the first
try.
http://reviews.llvm.org/D12402
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
