dense-analysis / ale

Check syntax in Vim/Neovim asynchronously and fix files, with Language Server Protocol (LSP) support
BSD 2-Clause "Simplified" License
13.57k stars 1.44k forks source link

Fix C/C++ header issues #392

Closed w0rp closed 7 years ago

w0rp commented 7 years ago

There are a number of issues with C and C++ filetypes and header files which need to be resolved. This issue will capture all of them, so discussion doesn't get fragmented.

It is worth noting that it is not probably not possible to detect the location of header files for 100% of projects. Some manual configuration on the part of users will be needed. (See the most popular C projects on GitHub for examples.)

TODO

klement commented 7 years ago

Somehow detect the location of other header files, and use -I to include those. (See #379.)

I think this is not a job for ale. User should provide the necessary -I and -isystem switches to the appropriate ale_c_*_options/ale_cpp_*_options. Guessing would probably break horribly in more complicated projects, e.g. self-contained build systems, cross-compile scenarios etc.

As for #include "header" syntax - per C standard, this type of include is implementation-specific. This typically means that the header search starts in the directory containing the file with the #include statement. In this case - if the actual gcc/clang command is not executed in the source code directory (when ale runs the linter), I would add an implicit -I<c-source-directory> to the gcc/clang command line. Also, this might need an on/off behaviour (e.g. let g:ale_c_gcc_autoinclude_source_dir = 1 or similar), because if using a preprocessor which doesn't support #include "" syntax and treats it as #include <> instead (allowed by C standard), then this might break things..

w0rp commented 7 years ago

For the #include "foo" syntax, all I did was fix it so the directory the file is in will be included in the search path, which is how gcc and clang work, which are the only supported compilers for linting. If some other compilers are added, they can be implemented differently.

For -I paths, it would be very useful to parse a project's configuration files, or follow a common project structure, etc., and determine how to include headers automatically. If it doesn't break anything, or it can just be turned off, it will only be a benefit.

w0rp commented 7 years ago

I think for basically any functionality, you should be able to turn it off. Everyone wants different behaviour.

klement commented 7 years ago

For -I paths, it would be very useful to parse a project's configuration files, or follow a common project structure, etc., and determine how to include headers automatically. If it doesn't break anything, or it can just be turned off, it will only be a benefit.

Configuration files? Is this Makefiles? CMakefiles? Automake/autoconf stuff? Custom perl scripts (yes, I've seen those)? This is not easy without understanding the build system... Doing a guesstimate will only work for trivial projects, where the added value is small. I wouldn't spend time on this ...

The only automagic stuff I can think of which could work kinda universally is to

1.) write a wrapper for gcc calls 2.) ask the user to do a clean build 3.) wrapper records command line for each C file, stores it in a DB 4.) when invoking linter, do a DB lookup for the proper set of flags and use those

this still has issues, example:

a.) if editing a new file, what flags do you use? typically, files in same dir would have the same set, but what if they don't? b.) conflicts - files might be built multiple times in the same project (different feature sets, different architectures...)

w0rp commented 7 years ago

That's why I said this above.

It is worth noting that it is not probably not possible to detect the location of header files for 100% of projects.

w0rp commented 7 years ago

I would like to add some support for automatically detecting the location for header files, even if it's only for a minority of users.

esran commented 7 years ago

The use of compilation database (compile_commands.json) seems reasonably common. Support for that would probably be a good step.

w0rp commented 7 years ago

I didn't know about compile_commands.json. That sounds like a good idea. Here is a link to the documentation for the benefit of others: https://clang.llvm.org/docs/JSONCompilationDatabase.html

bennyyip commented 7 years ago

Maybe we can do it by reading a file like .clang_complete which is how https://github.com/Rip-Rip/clang_complete get the include path. And this project provide a script(https://github.com/Rip-Rip/clang_complete/blob/master/bin/cc_args.py) to generate .clang_complete from Makefile, though it's hard to write by hand.

more about the script: https://github.com/Rip-Rip/clang_complete/blob/e536ca69f275a1c615550fc4aada5e320802262a/doc/clang_complete.txt#L314

cmlaverdiere commented 7 years ago

The way https://github.com/Rip-Rip/clang_complete handles compile_commands.json works perfectly for me. No need for external scripts since cmake outputs the json automatically.

caedn commented 7 years ago

I like the way YouCompleMe handles this: https://github.com/Valloric/YouCompleteMe#option-1-use-a-compilation-database and you can also manually provide flags on a per project basis: https://github.com/Valloric/YouCompleteMe#option-2-provide-the-flags-manually

crapp commented 7 years ago

As somebody already mentioned YouCompleteMe I'll ask my question here. YouCompleteMe deactivates Syntastic for C/C++ projects and provides it's own syntax checking. Which is also asynchronous and very good. Do you know if YouCompleteMe and ALE can work together the same way?

klement commented 7 years ago

I'm using YCM for tab completion and ALE for syntax checking without any issues. ..

On Sat, Apr 1, 2017, 09:57 Christian Rapp notifications@github.com wrote:

As somebody already mentioned YouCompleteMe I'll ask my question here. YouCompleteMe deactivates Syntastic for C/C++ projects and provides it's own syntax checking. Which is also asynchronous and very good. Do you know if YouCompleteMe and ALE can work together the same way?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/w0rp/ale/issues/392#issuecomment-290904021, or mute the thread https://github.com/notifications/unsubscribe-auth/ABn33c55YOiwZEmretCERjoQQETl1gtAks5rrgN4gaJpZM4MbBk5 .

crapp commented 7 years ago

@klement thanks for the information I'll definitely try ALE this weekend...

gagbo commented 7 years ago

I messed around a little bit with the commands, here's what I found out :

I still have 2 problems concerning this : 1) clang-check gives errors for all the files included for the compilation of the file in buffer, and using the gcc handler took all line/column locations like they were in the buffered file. This leads to having a warning popping up in fileA.cpp (in active window) at line 30 col 25 even if the actual warning was in fileB.cpp (or a header file, it just needs to be included somehow to compilation of fileA.cpp) at line 30 col 25. 2) Currently clang-tidy checker has "--" in its command, which basically cancels the -p flag. Since the main point of clang-tidy checker is to use compilation databases, I think this should go, but I am really not sure about this point and am eager to learn more

w0rp commented 7 years ago

I suppose we have two options for these linters, and we can only pick one.

  1. Check the files on disk, and check for semantic errors requiring information files.
  2. Check for syntax errors only, ignoring other files, as you type.

The good news is that the two can be combined. Two linters can be written for supporting both at the same time, with only one checking for problems while you type. Combining linters in this way is supported already, so nothing complicated will need to be done there.

I think a I'll recommend first changing pretty much every current C linter so they only check for what errors can be checked without knowledge of other header files, and then separately adding linter definitions which look at the file on disk, to check for more problems.

That is, unless there's some way to check for even semantic errors while you type. I'm relying on those with knowledge of these compilers to tell me how to do that. I don't really use them myself.

Having some example open source projects to test with would be very helpful. I can't really do anything without some code to test against.

w0rp commented 7 years ago

I'm of two minds really. Ideally, I would like to check for all problems while you type, including semantic errors. The more error information, the more frequently, the better. If it turns out to be impossible, then it's impossible. If I knew more about C, C++, GCC, Clang, and so on, this might already have been implemented.

w0rp commented 7 years ago

What confuses me about the clang-tidy linter is that it is set up like this.

let g:ale_cpp_clangtidy_options =
\   get(g:, 'ale_cpp_clangtidy_options', '-std=c++14 -Wall')

function! ale_linters#cpp#clangtidy#GetCommand(buffer) abort
    return 'clang-tidy %t -- ' . g:ale_cpp_clangtidy_options
endfunction

I was told that -- was needed to pass compiler flags, but I'm also hearing now that -- breaks clang-tidy so it doesn't pick up on the compilation database automatically. What should be done here?

Is there some additional argument which could be passed to have clang-tidy pick up on the compilation database? We could find the file ourselves and pass it to clang-tidy ourselves.

Should the options for clang-tidy be removed? How will you then configure your compiler flags? Is that something clang-tidy derives from the compilation database files? (I don't know much about it myself.)

w0rp commented 7 years ago

I'll go do some reading later, as I'll have some time this evening, and see if I can find any example projects to test with, and some documentation on how all of this is supposed to work.

gagbo commented 7 years ago

Compilation databases are json objects that contain (at least) 3 keys : "directory", "file" (specific to lookup the file in the src folder) and "command". Command contains the actual command run to compile the file, include directives and compilation flags. That means there is no need to pass compile flags to clangTools linters if they're given compilation databases (Flags are written in the json by CMake or bear...)

Personally, I think all tools from Clang tools, since they derive from LibTooling, are meant to be used with compilation databases. So in my opinion clang-tidy and clang-check linter should check files on disk and check for semantic errors, even if the linter is active only on write operations ; whereas clang linter can be used for local syntax errors only.

My 2 cents on the question "What should we do ?" is to put clang-tidy and clang-check in category 1, and clang in category 2. So clang-tidy/check could use %s and pass "-p build" as default flags (or find build subfolder in the path). And as a first step, we can dismiss all the warnings clang-* tools give for included files.

w0rp commented 7 years ago

Cool, thanks for all of the information.

I'll also see about adding C and C++ tools to the Docker image, if possible, with some tests for those tools.

w0rp commented 7 years ago

Okay, after a bit of tinkering around, the Docker image should now include every C and C++ tool, except from cppcheck, for now. (I couldn't easily install that one using the package manager.) Now it should be possible to write some tests which actually execute the C tools, for covering header issues with tests.

w0rp commented 7 years ago

I have just pushed a commit now which might initially cause a couple of issues, but should hopefully be a start on fixing some header issues.

Now if there are any problems inside of code in included headers, those problems will be reported on the header #include line with the string 'Problems were found in the header (See :ALEDetail)'. Using :ALEDetail will just print all of the raw lines of output from GCC.

For example, for this output...

In file included from a.h:1:0,
                 from test.c:3:
b.h:1:1: error: unknown type name ‘bad_type’
 bad_type x;
 ^
b.h:2:1: error: unknown type name ‘other_bad_type’
 other_bad_type y;
 ^

:ALEDetail will print the following lines:

b.h:1:1: error: unknown type name ‘bad_type’
 bad_type x;
 ^
b.h:2:1: error: unknown type name ‘other_bad_type’
 other_bad_type y;
 ^

Similar adjustments will probably be needed for clang, some regex will probably need to be adjusted, etc.

w0rp commented 7 years ago

I have also added some integration tests which execute GCC and check the errors that come out the other end.

w0rp commented 7 years ago

The next step is to start parsing the compilation database files for at least the clang programs.

w0rp commented 7 years ago

The same should be supported for Clang now too.

w0rp commented 7 years ago

I'm moving the rest of these issues to the next milestone. I've fixed a variety of C and C++ header issues, but there's still a lot more that can be done.

gagbo commented 7 years ago

I still do think that the Libtooling provided by clang should be used if you want to detect the other included headers. clang-check does the exact same analysis as clang -fsyntax-only as far as my understanding goes (probably not so far), so implementing clang-check linter to ale should help a lot with figuring out where header files are.

I'm working on a quick implementation right now, Pull request should fall this afternoon (GMT). Also, I would like to add the option of giving the build path to clang-tidy linter, since these tools are made to work with a compilation database, and the path to find it is not always easy. In the end I really want to just use autocommands like this in my vimrc

w0rp commented 7 years ago

The detection of include directories should be good enough for now. Support for using comple_commands.json and other similar things for more tools can be added later.

esran commented 7 years ago

I've struggled a bit with the defaults. If I enable cppcheck it seems to check the whole project rather than just a single file. This is possibly a cppcheck issue though as running the command myself does the same.

For the other checkers, the projects I'm working with need a lot more compile options, which vary based on the sub-directory or individual file. I've hacked together a change for the c_gcc check to get the options from my compile_commands.json. Its pretty bodgy vim script though being my first attempt.

What's the best way to share this?

gagbo commented 7 years ago

I don't know how gcc linter works with compile_commands.json, but now there is a function to that finds compilation command databases for c-language projects. See autoload/ale/c.vim

If you want to see an example of how it works, you can look at the clangcheck linter I added. clang-check wants the folder with compile_commands to be passed with the -p flag, and the doc for global cpp options to see how to interact with ale#c#FindCompileCommands.

The best way to share this is to fork the repo, create a branch with your changes there, and ask for a pull request.

w0rp commented 7 years ago

I recommend creating a pull request for it. If there's some way to parse options from a file and pass them on to GCC which works reliably, then we could support that. From now on I'm tracking individual issues with C and C++ compilers, now I've fixed a large number of simple issues, like finding header files in simple projects automatically.

esran commented 7 years ago

Yup. I've got as far as forking. I think I need to branch too? And then tidy what I've done up and make it a bit more correct in its parsing of the compilation database. And see if I can create tests for it as well. Probably not until the weekend at the earliest though.

w0rp commented 7 years ago

Cool. You can create a branch, or you can commit your changes to master on your fork. Whatever works best for you.

unphased commented 7 years ago

I'd like to get a reasonable workflow for using ALE to lint my C++ when only gcc is available on the system, it doesnt seem like a compilation database is needed (nor does gcc know what to do with one), most if not all of the failure is from being unable to find headers. So, I just need some quick non-global way to inform ALE about what -I flags to set.

In YCM this would be to make custom .ycm_extra_confs that define those flags explicitly, which is fine, if extremely ugly due to the mass of boilerplate in there. The key here is for each little project I want to add just the specific header search paths and not others. For the time being I can dump them all into my .vimrc to make ALE always pick them up when linting any C++ but i'd like it to try to fetch this from a local dotfile like .ale_cpp_conf in the directory of the source file (or above it).

There may even be a builtin vim functionality that could do this, e.g. for a cpp buffer to get it to load something, and that vimscript can just set g:ale_cpp_gcc_options.

w0rp commented 7 years ago

See :help g:ale_pattern_options. You can set up options for each buffer based on a regular expression pattern for filenames.

unphased commented 7 years ago

OK, that's useful, but i guess I'd prefer checking the local filesystem for project specific compile path config rather than polluting my global vim config with project-specific compile path config. Isn't that sensible?

This is an improvement on just having the same set of include paths for ALL source files though. so there's that.

This would also be a valid feature to use if there indeed is a way to get vim to auto load some vimscript in the same directory when opening a file from that directory.

skyegecko commented 7 years ago

You can use a plugin such as https://github.com/krisajenkins/vim-projectlocal to define settings based on a particular directory. I use this to set the CPLUS_INCLUDE_PATH environment variable, which is exported to calls to the compiler by vim.

unphased commented 7 years ago

That's very useful. Do you know how it is different from builtin set exrc?

skyegecko commented 7 years ago

exrc only looks in the cwd I believe. Projectlocal will identify the root of the project based on version control markers and automatically includes multiple configuration files from parent directories if present. The two don't differ too much though, the plugin is only about 30 LOC.

unphased commented 7 years ago

Yeah, okay. I think I can get by with exrc as my workflow often just has me leave my central vim's cwd in a particular place. But that does seem quite useful.

unphased commented 7 years ago

@dutchgecko there are moderately serious security concerns about the use of exrc or that plugin you linked. I'm going to rip that out of my vimrc and try to see if i can get away with some regexes as @w0rp suggests. putting that all inside my vimrc as opposed to littered around the filesystem isn't something that bothers me just yet.

partounian commented 7 years ago

Sorry I'm a bit confused. One issue I have with C++ and Ale is I get many false positives for includes from libraries that come from the cmake. Is there any solution to this? I also noticed in these files it will only show the first error, if there an error lower in the file that will not be marked.

gagbo commented 7 years ago

What do you mean by false positives ?

Edit : It may work if you give all the arguments to the linters manually, but I mean it doesn't work out of the box like the linting for implementation files

unphased commented 6 years ago

So far in the past few months, g:ale_pattern_options has worked extremely well, and all I do is iterate through running :ALEInfo to get to the bottom of all the (mostly header path related) errors.

Today I have also figured out how to get clang_complete functioning very well with my projects.

My next step will be to unify the two systems by implementing in my vimrc some code (probably will just use python) that will implement the walking up the filesystem searching for .clang_complete and procedurally assemble a corresponding g:ale_pattern_options from that. This should give me that holy grail where I fully control the linting and completer flags, both use clang, and I have full clang powered completion and jump-to-definition together with live in-editor signs (complete with line highlighting). Full featured Vim IDE by this point, plus, extra points because it works on both macOS and Linux.

I think it would make sense to try to make ALE smart and leverage .clang_completes if it finds them. Is that within scope for this plugin? If not, I will just leave it in my vimrc, but if it would be, id be happy to try to contribute it once I get things up and running.

gagbo commented 6 years ago

I think .clang_complete should just be in your vim's path option, and then you can just use a built-in vimscript function to look for the .clang_complete file and be done with it. Also, it's probably already done in this case by this function : can you tell if it's good enough or if you have corner cases where it doesn't work ?

The only issue we had with these "Find smartly this file I need" is for the compilation database made by CMake for example, since the file is actually nested in a different subfolder and therefore is not easily findable by just setting the path correctly. To correct this we added a function to specifically look for compilation databases in out-of-source builds : It's there

I have not looked the code for a while, but if you make a translator from .clang_complete to ale_pattern_options, it's probably going to be needed in the plugin here in a location to confirm later. But it's also possible that it's been done.

unphased commented 6 years ago

Thanks for schooling me on path. It does seem to be part of Vim's original design toward use in projects, stuff like this (and e.g. various ctags integration) sometimes get left on the wayside in the search for more "modern" tools but should not get overlooked. I can't say that ctags has much use in the presence of the glory that is clang, though 😄

I am not certain that using path makes the most sense here. I've never configured it in my vimrc, it seems like it is set to .,/usr/include,, which is sensible though rather minimal. I would not want to extend this with anything beyond the typical system header locations, though. As for .clang_complete flag files... those are going to get stashed within my projects' directories. I really don't want to put compiler flag configuration (which is really specific to a projects' dependency implementation detail) in a centralized place, which is where it has been (in my vimrc in g:ale_pattern_options) so far.

So it does feel like the logic I want (and hopefully to implement in pure vimscript) is to iteratively go from current working dir (and from file's containing dir) upward toward filesystem root looking for .clang_complete.

laoshaw commented 4 years ago

fast forward to 2020, ale complains not finding local header files, what is the right way to "fix" it? I do have compile_commands.json generated but they're not used by ALE it seems, do I need add the -I flags to ALE's gcc/clang flag variable manually for each project?

w0rp commented 4 years ago

There are two settings that might help. g:ale_c_parse_compile_commands and g:ale_c_parse_makefile. Both are off by default as they aren't perfect.

I think the problem is with C itself. There's no standard for where headers should go, so there's no reliable way to detect them. You can guess there is an include directory. The best option I think you have is to parse Makefile results or compile_commands.json files, which those two options are for.