================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:825
@@ -812,1 +824,3 @@
+ CGF.EmitCall(CGM.getTypes().arrangeCXXMethodCall(Args, FPT, required),
Callee,
+ ReturnValueSlot(), Args, D);
}
----------------
Timur Iskhodzhanov wrote:
> Hm, I don't like duplication code here.
> In my version of the patch, I've added something like a `bool
> ImplicitParameterFollowsExplicit` parameter to EmitCXXMemberCall.
>
> What do you think?
I'm not really happy with the proliferation of boolean parameters like
ForVirtualBase and Delegating below. I'd rather explicitly build up the
CallArgList in the MS ABI code.
================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:798
@@ -801,3 +797,3 @@
- llvm::Value *ImplicitParam = 0;
- QualType ImplicitParamTy;
+ // C++11 [class.mfct.non-static]p2:
+ // If a non-static member function of a class X is called for an object
that
----------------
Timur Iskhodzhanov wrote:
> I think this deserves a separate patch.
> Also, why is it MS ABI specific?
This comes from EmitCXXMemberCall.
================
Comment at: lib/CodeGen/CodeGenFunction.h:2495
@@ -2494,2 +2494,3 @@
+public:
/// EmitCallArgs - Emit call arguments for a function.
----------------
Timur Iskhodzhanov wrote:
> I don't like changing the public interface here, see my suggestion below.
Why not? It's only available in CodeGen anyway, and CGF is a pretty public
class that is tightly coupled with the CGCXXABI subclasses.
================
Comment at: lib/CodeGen/CodeGenFunction.cpp:697
@@ -696,3 +696,3 @@
CurGD = GD;
- const CXXMethodDecl *MD;
+ const CXXMethodDecl *MD = 0;
if ((MD = dyn_cast<CXXMethodDecl>(FD)) && MD->isInstance()) {
----------------
Timur Iskhodzhanov wrote:
> `= 0` is not needed here.
My thinking was that not having it is too subtle, and the store will be
trivially dead-store-eliminated. I'll raise the MD assignment out of the if
instead.
http://llvm-reviews.chandlerc.com/D2405
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
