[cfe-commits] r146959 - in /cfe/trunk: include/clang/AST/ include/clang/Analysis/Analyses/ include/clang/Basic/ include/clang/CodeGen/ include/clang/Driver/ include/clang/Frontend/ include/clang/Lex/ include/clang/Sema/ include/clang/StaticAnalyzer/Core/ include/clang/StaticAnalyzer/Core/BugReporter/ include/clang/StaticAnalyzer/Core/PathSensitive/ lib/AST/ lib/Analysis/ lib/Basic/ lib/CodeGen/ lib/Driver/ lib/Frontend/ lib/Lex/ lib/Parse/ lib/Sema/ lib/StaticAnalyzer/Core/ utils/TableGen/

[Ted Kremenek]

On Dec 19, 2011, at 9:48 PM, Rafael Ávila de Espíndola wrote:

> On 19/12/11 11:33 PM, Ted Kremenek wrote:
>> Hi David,
>> 
>> I'm sorry to come down hard here, but I really don't like the idea of
>> adding a fake method just unweaken the vtables.  Can we just move the
>> offending virtual methods out-of-line?  Having weak vtables is one
>> thing; have a dead method permanently embedded in that vtable is
>> another.  We've now just increased the size of the clang executable
>> (and make the code less pretty) just to perform this build-time
>> optimization.
> 
> At least on ELF the impact of the patch is small, but mostly on the
> wrong direction. I tested reverting this patch in a Release build.
> 
> When being build with clang, reverting reduces the sum of the size of
> every .o from 73445656 bytes to 73420224 bytes and of the clang binary
> from 30553808 to 30513696.
> 
> When building with gcc 4.6, the sum of the size of every .o goes from
> 81926520 to 82165328 (so the patch saved a bit) and the clang binary
> goes from 36017976 to 35894528.

Beyond the file size, my main concern is why do we need an anchor() methods at all?  Can't we just move more methods to have out-of-line definitions in .cpp files and accomplish the same thing?  I'm completely willing to admit that I'm missing some key idea, but if so can someone please point it out to me?  No offense David (as I really appreciate you spear heading this effort), but they really look like turds in the codebase to me.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111219/a0645e4d/attachment.html>