[PATCH] Get rid of VTableContext::ComputeMethodVTableIndices() and VTableContext::getNumVirtualFunctionPointers()
Timur Iskhodzhanov
timurrrr at google.com
Mon May 13 07:26:14 PDT 2013
Hi pcc, anders1,
Hi Anders, Peter,
While reading the VTableBuilder.cpp I realized that the VTableContext::ComputeMethodVTableIndices() duplicates substantial parts of the VTableBuilder code.
Also I believe VTableContext is a wrong place to calculate the indices of methods in a given vtable as VTableContext should eventually work with any ABI, whilst VTableBuilder(s) should be ABI-specific.
I've moved the index map calculation code to VTableBuilder.
Not only it allowed me to shave off about 100 LOC, the ComputeMethodVTableIndices() code was recursive and rather complex and I was able to replace it with a simple non-recursive loop.
I've also noticed the vtable indices calculation is hardly covered by tests, so I've added some.
Please note that the trunk version passes these tests just fine, i.e. the code change is pure refactoring, no output change*.
[* - I actually did change one llvm::format line to use the same indentation when printing out different tables (see the other occurences of %4 in the file).]
I removed VTableContext::getNumVirtualFunctionPointers() as it's no longer used.
Can you please review this patch?
Please note that I don't do any special handling for implicit destructors in my code.
The Test16::D seems to work well without this handling.
Is it OK or am I missing some special cases?
--
Thanks,
Timur
http://llvm-reviews.chandlerc.com/D785
Files:
test/CodeGenCXX/vtable-layout.cpp
include/clang/AST/VTableBuilder.h
lib/AST/VTableBuilder.cpp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D785.1.patch
Type: text/x-patch
Size: 27821 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130513/7c34d709/attachment.bin>
More information about the cfe-commits
mailing list