Minor polishing of VTableBuilder

Timur Iskhodzhanov timurrrr at google.com
Wed May 8 01:14:25 PDT 2013


2013/5/5 Peter Collingbourne <peter at pcc.me.uk>:
> On Fri, Apr 26, 2013 at 09:13:59PM +0400, Timur Iskhodzhanov wrote:
>> Hi Peter, Anders,
>>
>> I was reading the source of Clang VTableBuilder sources to get
>> familiar with the code before writing a vftable generator for
>> Microsoft ABI.
>>
>> I've decided to improve a couple of comments.
>> After improving one of the comments, fixing one of the FIXMEs in the
>> code became obvious, so I went ahead and just did it.
>> Please see the attached patch!
>
> The part of your patch that fixes the FIXME LGTM.

Thanks, r181396!
I went ahead and also committed the minor clarification of the comment
BaseOffset::VirtualBase, now that it's obvious from the code.

>> I also have a couple of questions, see the TODOs in the patch.
>> The most important is VTableBuilder::ComputeThisAdjustmentBaseOffset
>> where it's a bit unclear what's going on. It would be nice if we could
>> come up with a code which is easier to understand or at least a good
>> comment there.
>
> Sorry but I'm not familiar enough with the rest of the code to be
> able to comment on your changes.  Anders?

Anders,
I've updated the patch to skip the part already committed, please see attached.

> Thanks,
> --
> Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vtables_comments_2.patch
Type: application/octet-stream
Size: 2598 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130508/41d28f19/attachment.obj>


More information about the cfe-commits mailing list