[Lldb-commits] [PATCH] D18530: Move some functions from ClangASTContext to ClangUtil

[Enrico Granata via lldb-commits]

> On Mar 30, 2016, at 12:31 PM, Tamas Berghammer via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> 
> So in general if we get some actual advantage from moving the code (e.g. cleaner API, better testing options, etc...) then I have no issue around it but if the only gain is the file size reduction and the removal of the cyclic dependencies then I think this causes more problem then the benefit of the change (same reason why we don't run clang-format over the full code base).
> 
> In this concrete case I don't see the benefit neither in the API nor for testing as all functions were accessible before in the same way and the tests can call the functions from the ClangASTContext instead of the ClangUtils with the same functionality.


+1 to this

I have seen this kind of pattern (splitting the semantics of a class across multiple implementation files) in other languages (e.g. I remember seeing some C# code using it), but I don’t see much of an advantage to it in C++
Actually, in this case it looks like the multiple implementation files all need to #include some common headers, so what you end up with is even more code being compiled, which negates any advantage of having smaller on-disk files.

Thanks,
- Enrico
📩 egranata@.com ☎️ 27683

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160330/8ee97d04/attachment-0001.html>