Review Request 123963: First bits of refactorings. Skeleton of interface for KDevelop. Build system. CompilationDatabase for CMake projects and ClangTool.

[Maciej Poleski]

> On Cze 15, 2015, 3:19 po południu, Milian Wolff wrote:
> > refactoring/DocumentCache.h, line 54
> > <https://git.reviewboard.kde.org/r/123963/diff/3/?file=379860#file379860line54>
> >
> >     why store it in pointers?
> >     
> >     also: style, prefix members with `m_`, not just `_`.

Because when container is resized, objects may be relocated (in memory). This should not happen because in such case objects should be moved (and memory allocated for std::string data should be reused). But i didn't found guarantee.
ClangTool will take address of string data (not std::string, but rather something like (std::string::data(),std::string::size()).


> On Cze 15, 2015, 3:19 po południu, Milian Wolff wrote:
> > refactoring/DocumentCache.cpp, line 67
> > <https://git.reviewboard.kde.org/r/123963/diff/3/?file=379861#file379861line67>
> >
> >     this is the only place where you need the std::strings, right? Maybe use QByteArray before and only convert to std::string here on-demand. that simplifies the rest of the code thanks to implicit sharing, and you could simply use a QHash<QByteArray, QByteArray> for the map.

But if we have implicit sharing, what about multi-threading? Also i'm not sure what would happen with encoding (we have to convert from QString UTF-16 to unknown (probably UTF-8 or "local-8-bit")). QHash could use QString directly, but then the above problem... Converting on demand is also imposible due to the above problem (lifetime of data must extend lifetime of ClangTool, see http://clang.llvm.org/doxygen/classclang_1_1tooling_1_1ClangTool.html#a2fef9bcf06819ffb9418560996ff71c7).
Is Qt guaranteeing, that (memory) location of data in QByteArray remains unchanged, when object is copied/move due to QHash rehasing?


> On Cze 15, 2015, 3:19 po południu, Milian Wolff wrote:
> > refactoring/utils.cpp, line 121
> > <https://git.reviewboard.kde.org/r/123963/diff/3/?file=379865#file379865line121>
> >
> >     remove

Why exceptions are disabled? I'm unable to handle this error in this place, and returning also error coude would make implementation and use of this stack of methods fairly more complicated.
Siliently ignoring errors is also not the best idea. (logging != handling)


- Maciej


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123963/#review81480
-----------------------------------------------------------


On Cze 14, 2015, 9:24 po południu, Maciej Poleski wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123963/
> -----------------------------------------------------------
> 
> (Updated Cze 14, 2015, 9:24 po południu)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-clang
> 
> 
> Description
> -------
> 
> First bits of refactorings.
> Skeleton of interface for KDevelop.
> Build system (really nasty, linked DSO has 18MiB).
> CompilationDatabase for CMake projects and ClangTool (with cache, but without cache updates).
> Interface is not frozen for now.
> 
> Currently building of refactorings is enabled only if Clang 3.6.x is found. In other case build system behaves exactly as current master.
> 
> Refactorings require some contextual informations about projects. I made draft of interface with should feature:
>  - Stable ABI - dlopen (or equivalent) a DSO and if succeed (binary is still compatible with Clang libraries ABI), dlsym (or equivalent) some functions which are to provide all refactorings features
>  - Prototypes (with some implementation) of functions constituting API between KDevelop and refactorings backend
>  - It may be considered to use Qt plugin system to provide implementation of this (or equivalent) interface. This would be more expensive, but also more portable
>  - In worst case this library (static version) can be used with additional driver to make stand-alone executable providing all refactorings features (but it would require additional marshaling and hamper cooperation with user (how to provide UI in such a case?))
> 
> Handling dependencies between Clang/LLVM libraries is a nightmare. I extended FindClang.cmake to find much more libraries and used them to link with first pieces of my code. This is huge, requires additional libraries like zlib and even ordering of dependencies can cause linker errors.
> 
> Interface is not finished
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 875172a8407f4bd9faf330f032a280fa66c2b16f 
>   cmake/FindClang.cmake 6c9bd6ef0242319122dcc7e6fd6cea8d9f0cbfbb 
>   refactoring/CMakeLists.txt PRE-CREATION 
>   refactoring/DocumentCache.h PRE-CREATION 
>   refactoring/DocumentCache.cpp PRE-CREATION 
>   refactoring/interface.h PRE-CREATION 
>   refactoring/interface.cpp PRE-CREATION 
>   refactoring/utils.h PRE-CREATION 
>   refactoring/utils.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/123963/diff/
> 
> 
> Testing
> -------
> 
> Compiles on my Gentoo system. It makes sense to build this only on system with Clang 3.6.
> 
> 
> Thanks,
> 
> Maciej Poleski
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150615/f47728be/attachment.html>