Minor polishing of VTableBuilder

Anders Carlsson andersca at icloud.com
Mon May 13 07:33:23 PDT 2013


On May 8, 2013, at 1:14 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:

> 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.
> 

Hi Timur,

sorry for not getting back to you earlier. It's been a long time since I wrote this VTable code and I'd have to page everything in again. I'll try to look at your patch this weekend.

Thanks,
- Anders





More information about the cfe-commits mailing list