[PATCH] Fix PR17738 - add support for vtordisp thunks when using -cxx-abi microsoft

[Reid Kleckner]

On Tue, Nov 5, 2013 at 11:10 AM, Timur Iskhodzhanov <timurrrr at google.com>wrote:

> 2013/11/5 Reid Kleckner <rnk at google.com>:
> >
> >   LGTM
> >
> >
> > ================
> > Comment at: lib/AST/MicrosoftMangle.cpp:1849
> > @@ -1848,2 +1848,3 @@
> >                                        MicrosoftCXXNameMangler &Mangler,
> >                                        raw_ostream &Out) {
> > +  if (!Adjustment.Virtual.isEmpty()) {
> > ----------------
> > Timur Iskhodzhanov wrote:
> >> Reid Kleckner wrote:
> >> > This is complicated enough that it deserves it's own BNF comment with
> portions of the mangleFunctionClass() BNF diagram.  The G, O, W and A, I, Q
> access codes clearly correspond to it.
> >> Agreed. I'm not a BNF master, so formatting/content suggestions are
> welcome!
> >>
> >> It has actually reminded me we might sometimes have far thunks... I'm
> not sure when are they needed (hey, why would you need more than +-2^31 to
> adjust this inside a record type?) so I've added a FIXME. Anyways, the
> mangleFunctionClass didn't ever handle "far" functions either.
> > Looks good.
> >
> > "far" isn't related to displacement, it's related to segments and 16-bit
> Windows.  Thankfully it's dead and we don't support it.  :)
>
> Ah, that's right.
> Shall I just remove the BNF lines with "far" then?


I like having documentation for what we believe the other half of the
character codes are used for.


> > ================
> > Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:998
> > @@ +997,3 @@
> > +
> > +    if (TA.Virtual.Microsoft.VBPtrOffset) {
> > +      assert(TA.Virtual.Microsoft.VBPtrOffset > 0);
> > ----------------
> > This corresponds to vtordispex thunks, right?  Can you put in some
> comments about that, or name give names to the IR nodes?
>
> Sure, will add a comment.
>
> > http://llvm-reviews.chandlerc.com/D2079
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131105/8f9abaf7/attachment.html>