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