<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Nov 5, 2013 at 11:10 AM, Timur Iskhodzhanov <span dir="ltr"><<a href="mailto:timurrrr@google.com" target="_blank">timurrrr@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">2013/11/5 Reid Kleckner <<a href="mailto:rnk@google.com">rnk@google.com</a>>:<br>
<div class="im">><br>
>   LGTM<br>
><br>
><br>
> ================<br>
> Comment at: lib/AST/MicrosoftMangle.cpp:1849<br>
> @@ -1848,2 +1848,3 @@<br>
>                                        MicrosoftCXXNameMangler &Mangler,<br>
>                                        raw_ostream &Out) {<br>
> +  if (!Adjustment.Virtual.isEmpty()) {<br>
> ----------------<br>
> Timur Iskhodzhanov wrote:<br>
>> Reid Kleckner wrote:<br>
>> > 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.<br>
>> Agreed. I'm not a BNF master, so formatting/content suggestions are welcome!<br>
>><br>
>> 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.<br>

> Looks good.<br>
><br>
> "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.  :)<br>
<br>
</div>Ah, that's right.<br>
Shall I just remove the BNF lines with "far" then?</blockquote><div><br></div><div>I like having documentation for what we believe the other half of the character codes are used for.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">
> ================<br>
> Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:998<br>
> @@ +997,3 @@<br>
> +<br>
> +    if (TA.Virtual.Microsoft.VBPtrOffset) {<br>
> +      assert(TA.Virtual.Microsoft.VBPtrOffset > 0);<br>
> ----------------<br>
> This corresponds to vtordispex thunks, right?  Can you put in some comments about that, or name give names to the IR nodes?<br>
<br>
</div>Sure, will add a comment.<br>
<br>
> <a href="http://llvm-reviews.chandlerc.com/D2079" target="_blank">http://llvm-reviews.chandlerc.com/D2079</a><br>
</blockquote></div><br></div></div>