Sarcasm / irony-mode

A C/C++ minor mode for Emacs powered by libclang
GNU General Public License v3.0
906 stars 99 forks source link

hpps files have wrong cdb with json from cmake #271

Open sezaru opened 8 years ago

sezaru commented 8 years ago

I have a project which have subprojects in it (each subproject have it own CMakeLists.txt)

For example, let's say I have this scenario: project ....|subproject1 ....|subproject2

project CMakeLists.txt only have add_subdirectory to subproject1 and 2.

Now, regarding irony-mode, when I open a cpp file, irony-mode will look in the json file cmake created to get the correct flags for it, and it works really great. But, when I open a hpp file, it looks like that irony-mode guesses what flags to use,.

For example, I have a file called NFSe.cpp which is part of subproject2 in directory project/subproject2/src, when I do irony-cdb-menu, the flags are right and the working directory is ../build/subproject2. But, when I open NFSe.hpp which is the header of NFSe.cpp, and is located in project/subproject2/includes, irony-mode stops working.

Running irony-cdb-mode, shows that the working directory for it is ../build/subproject1, and the flags to the compiler are wrong.

So, the problem is that the hpp files doesn't work since it thinks the project for this file is the wrong one and so gets the flags for the wrong project.

Sarcasm commented 8 years ago

I'm not sure to understand.

Did you customize irony-cdb-search-directory-list? What's its value? Where are located the compile_commands.json? Did you try: M-x irony-cdb-json-add-compile-commands-path RET?

sezaru commented 8 years ago

irony-cdb-search-directory-list is ("." "build")

And irony-mode finds the json file just fine, the problem is with hpp files (which compile_commands.json doesn't have any information about).

I made a example project to better show what I mean, I have my test root project with it CMakelists.txt

cmake_minimum_required(VERSION 3.2.0)
project(Test)

add_subdirectory(proj1)
add_subdirectory(proj2)

file (GLOB_RECURSE sources ${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp)
add_executable(${PROJECT_NAME} ${sources})

Inside test/src I have a single file with a main.

Now, I have 2 subprojects, proj1 and proj2, both added with the add_subdirectory as shown above. Each subproject contains 1 cpp file and the CMakeLists.txt for then looks like this:

cmake_minimum_required(VERSION 3.2.0)
project(project1)

file (GLOB_RECURSE sources ${CMAKE_CURRENT_SOURCE_DIR}/src/*.cpp)
add_library(${PROJECT_NAME} SHARED ${sources})

In the case of proj2, it's the same CMakeLists.txt but the project name is project2.

So, after running cmake with -DCMAKE_EXPORT_COMPILE_COMMANDS=ON, I got this compile_commands.json:

[
{
  "directory": "/home/devel/test/build",
  "command": "/usr/bin/c++      -o CMakeFiles/Utils.dir/src/dir1/dir2/file.cpp.o -c /home/devel/test/src/dir1/dir2/file.cpp",
  "file": "/home/devel/test/src/dir1/dir2/file.cpp"
},
{
  "directory": "/home/devel/test/build/proj1",
  "command": "/usr/bin/c++   -Dproject1_EXPORTS -fPIC   -o CMakeFiles/project1.dir/src/dir1/file2.cpp.o -c /home/devel/test/proj1/src/dir1/file2.cpp",
  "file": "/home/devel/test/proj1/src/dir1/file2.cpp"
},
{
  "directory": "/home/devel/test/build/proj2",
  "command": "/usr/bin/c++   -Dproject2_EXPORTS -fPIC   -o CMakeFiles/project2.dir/src/dir1/file2.cpp.o -c /home/devel/test/proj2/src/dir1/file2.cpp",
  "file": "/home/devel/test/proj2/src/dir1/file2.cpp"
}
]

Now, if I open the file "/home/devel/test/src/dir1/dir2/file.cpp" irony will correctly use the flags setted in compile_commands.json, and irony-cdb-json will show that the working directory is /home/deve/test/build.

The same happens if I open "/home/devel/test/proj1/src/dir1/file2.cpp", working directory will be /home/devel/test/build/proj1 and the flags will be from project1.

The problem lies whenever I open a hpp file, let's say I have a hpp file in /home/devel/test/proj1/include/file.hpp, this file is inside the proj1 directory, so it should contain the working directory /home/deve/test/build/proj1 and the flags of this subproject.

But what happens is that the working directory is /home/devel/test/build/proj2 with it's flags..

Actually any hpp in all my project and subprojects will be seen by irony as being part of proj2. It seems to me that irony-mode just pick one of my projects (or subproject) and use it to all header files.

This solution (if that is the case), would be to irony map the subprojects of the root project and based on that find to what project the header file belongs.

For example, in my test project, irony would map 3 projects (based in the directory parameters of compile_commands.json):

 root: "/home/devel/test/build"
 proj1: "/home/devel/test/build/proj1"
 proj2: "/home/devel/test/build/proj2"

and then, when a hpp file is opened, it would look for it path, if the file is in /home/devel/test/proj1/include/, then it belongs to proj1 and so on.

if it still not very clear, I can upload somewhere my test project so you can reproduce.

Thanks

Sarcasm commented 8 years ago

Okay, I understand the issue better now, I did not realize there was only one compilation database for the multiple projects.

if it still not very clear, I can upload somewhere my test project so you can reproduce.

Seems clear but the sample project would facilitate my work, could you upload it somewhere (gist?)?

sezaru commented 8 years ago

There you go https://www.dropbox.com/s/fe4gwcd7mfne2f3/test.tar.bz2?dl=0 I uploaded to dropbox instead of gist since it is multiple files in different directories.

Sarcasm commented 8 years ago

thanks!

sezaru commented 8 years ago

Hey Sarcasm,

I've made changes to my local version of your irony-server to address this issue.

What I've done is change the function getCompileOptions in irony.cpp to check if the compileCommands variable is null (it is whenever I open a hpp file since the file is not presented in the json file the clang db loaded).

If the variable is null, I get all the compile commands from clang db, and split it to the project modules, then I check what module the file belongs and use the compile commands for it.

Actually I do more work, but this is the essential parts of it.

And so far, it is working perfect, I never had a single file which have incorrect (or empty) cdb again.

I will test a little more and if it works great in all the projects I have I will make a pull request.

But this post is actually to ask why do you load the clang db every time a file is opened (since it will call the getCompileCommands which load the json db) is that somewhat cached somewhere by clang? My code will make the split process every time the function is called so I wanted to cache it too.

In my small projects I do not have any problem with performance regarding this issue, but I guess in bigger projects with bigger json this can be a problem.

Maybe a solution like rtags where the server is running in the background and all the db is already loaded in memory would be a better solution? Thanks

Sarcasm commented 8 years ago

But this post is actually to ask why do you load the clang db every time a file is opened (since it will call the getCompileCommands which load the json db) is that somewhat cached somewhere by clang? My code will make the split process every time the function is called so I wanted to cache it too.

I did not worked on caching because I would like to be able to be on the command line, grep something, then open the file in emacs, and irony should load the database fast even if not cached. I did not work on making anything fast but that's what I would want.

Maybe a solution like rtags where the server is running in the background and all the db is already loaded in memory would be a better solution?

I don't think so, loading a compile_commands.json should not be slow, it's just one file, I think it can be made pretty fast. Maybe one should compile it to other other form to make loading fast too but I don't want the fast loading to require to manage some processes in the background, I think there are simpler solutions.

Sarcasm commented 8 years ago

I will test a little more and if it works great in all the projects I have I will make a pull request.

Yes please do.

I have neglected the compilation database for some time now even though I think it is an essential component of irony. On the other side @Hylen has made some work in this regarde that may have some kind of overlap with you (#230, #232, #270).

sezaru commented 8 years ago

I don't think I like Hylen approach to guessing the hpp cflags (by trying to find a cpp file related to it). It is the approach used in a lot of projects, actually (CEDET comes to mind), but for me, it looks like more of a hack that will work in small projects but not on larger ones, or at least it will not work 100% of the time.

One case that it will probably not work that I can think now is a pure template class, this class will only have a header file , so Hylen algorithms (as far as I understand it) will fail to find the correct cflags to it.

My approach on the other hand, assumes that a file in a cmake project have the same cflags as any other file in the same project, and so far I haven't found a example that this is not the case, since every file in the project will have the same includes, libraries, etc.

This change if you have subprojects, but my approach deals with this situation too.

The only way that, maybe, there will be a problem, is if a cmake project, creates, for example, 2 binaries, with different files in it and linked libraries, that's the case I plan to test before making the pull request.

I think qtcreator reads the CMakeLists.txt (or some other generated cmake file) to know what files belongs to what binaries and add the correct cflags to it, maybe that is a solution to the problem above, but I'm not sure if it is really needed.

sezaru commented 8 years ago

Hey Sarcasm, let me ask you something.

I think I figured out a way to make hpps files have the correct cflags 100% of the time. But, this approach needs the ninja build system installed to work.

The idea is to parse the project compile_commands.json with the information ninja -t deps gives to figure out the hpps cflags.

I've made some manual testing and it seems like it will work in all circumstances.

My question is if it is acceptable to add ninja as a dependency to irony-mode.

Another thing is the json parser to the compile_commands.json, in irony-mode you use the db command from libclang to parse it, I can use it too in this approach, but I think it would be faster to parse it directly.

Given that, I ask if adding some json parser as a dependency is acceptable too, for example, Hylen used rapidjson which is a header only json parser.

Since it is header only, it would be really easy to add to the irony repository as a external project without needed to modify the cmakelists.txt of irony or the compilation method. Is this acceptable too?

Thanks

Sarcasm commented 8 years ago

My question is if it is acceptable to add ninja as a dependency to irony-mode.

Not as a mandatory dependency but we could imagine being more accurate when ninja is used.

Given that, I ask if adding some json parser as a dependency is acceptable too, for example, Hylen used rapidjson which is a header only json parser.

I told Hylen to remove the JSON parser if he is using Boost to avoid having 2 dependencies providing a JSON parser.

Hylen commented 8 years ago

My approach on the other hand, assumes that a file in a cmake project have the same cflags as any other file in the same project, and so far I haven't found a example that this is not the case, since every file in the project will have the same includes, libraries, etc.

My method searches for cpp files close to the hpp file in the directory structure, so it should work as well as this. Besides, many projects have different compile commands for different cpp files, as an example you could look at LLVM itself.

Using ninja in the case when it's installed could work, but I think the easiest way of improving accuracy for the method I'm using would be to parse the source files and see what header files they include. This could probably be done using libclang itself, but would require parsing all the files in a project every time the database gets updated, which could be costly.

I suggest you start by trying out my method, and point out a case of where it gives the wrong result and we can discuss it from there.