[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] [ms-cxxabi] Emit non-virtual member function pointers
From:       Reid Kleckner <rnk () google ! com>
Date:       2013-04-30 23:44:42
Message-ID: ad8985f18083967d647f9c0b1f3c2386 () llvm-reviews ! chandlerc ! com
[Download RAW message or body]


  Thanks!  I'm curious if John has any thoughts on this, though.


================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:552
@@ +551,3 @@
+
+  // The rest of the fields are adjusted by conversions to a more derived class.
+  if (hasNonVirtualBaseAdjustmentField(IsMemberFunction, Inheritance))
----------------
João Matos wrote:
> Maybe we should leave a FIXME: with a link to the PR?
This code doesn't need to change, so I see no cause for a FIXME here.  We just need \
to implement ConvertMemberPointer().

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:604
@@ +603,3 @@
+MicrosoftCXXABI::EmitMemberPointer(const APValue &MP, QualType MPT) {
+  llvm_unreachable("const expr member pointer conversions not implemented in "
+                   "the MS ABI");
----------------
João Matos wrote:
> What exactly are "const expr member pointer conversions"?
This overload gets called to implement initializers like this:

struct A { int a; void foo(); };
struct B { int b; void bar(); };
struct C : A, B { };
void (C::*mp)() = &B::bar;

We have to do a compile-time conversion from 'void (B::*)()' to C, which means \
adjusting the non-virtual base offset.

If you look at the Itanium code, you can see how the conversions are implemented \
twice: once for Values and once for Constants.

We could probably avoid the duplication with a template, but I'm not sure it's worth \
it.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:142
@@ -141,1 +141,3 @@
 
+  /// EmitFullMemberPointer - Emits a full member pointer with the fields common
+  /// data and function member pointers.
----------------
João Matos wrote:
> with the fields common *to*
Fixed


http://llvm-reviews.chandlerc.com/D699
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread]