[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