[cfe-commits] [PATCH] Refactoring of vtable and VTT builders

[Anders Carlsson]

On Jun 13, 2011, at 7:02 PM, Peter Collingbourne wrote:

> On Mon, Jun 13, 2011 at 03:19:11PM -0700, Anders Carlsson wrote:
>> 
>> On Jun 13, 2011, at 12:41 PM, Peter Collingbourne wrote:
>> 
>>> Hi,
>>> 
>>> This patch refactors the vtable and VTT builders to provide a
>>> backend-neutral interface to the builders in the AST library, which
>>> CodeGen uses to build vtables and VTTs in LLVM.  No functionality
>>> change intended.
>> 
>> While I'm not crazy about this idea, other people (Doug) are in favor of it, so I'm reluctantly fine with it :)
>> 
>> That being said, I don't think this patch should be committed in its current state. This should be done in incremental steps,
>> there's no reason to move both the VTT builder and VTable builder in a single commit.
> 
> OK, then.  Following your and Eli's suggestions I will try to
> break this patch up into the following steps:
> 
> 1) Move GlobalDecl to AST
> 2) Make VTTBuilder independent of CodeGen (broken into sub-steps)
> 3) Move VTTBuilder to AST
> 4) Make VTableBuilder independent of CodeGen (broken into sub-steps)
> 5) Move VTableBuilder to AST
> 
> As a start, I am attaching a (simple) patch for 1.  OK to commit?
> 

Sure, looks good!

>> Also, the files should be svn cp'ed to preserve history; being able to easily go back and see the rationale behind changes is
>> crucial to fixing bugs/regressions in this code.
> 
> Hmm, I'll see if I can figure out how to persuade git-svn to do that.

If you can't persuade git-svn to do it, you could always just svn cp the files as a separate step using, well, svn :) You could then hack on them further using git-svn.

- Anders