Bug 386421 - C++ include path completion misses system include path completion
Summary: C++ include path completion misses system include path completion
Status: RESOLVED FIXED
Alias: None
Product: kdevelop
Classification: Applications
Component: Language Support: CPP (Clang-based) (show other bugs)
Version: 5.2.0
Platform: Neon Linux
: NOR normal
Target Milestone: ---
Assignee: kdevelop-bugs-null
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-11-01 08:59 UTC by Venca B Spam
Modified: 2017-11-18 22:38 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 5.2.1


Attachments
Bug report illustration (1.19 MB, video/mp4)
2017-11-01 08:59 UTC, Venca B Spam
Details
Sample project required to reproduce reported behavior. (2.45 KB, application/zip)
2017-11-01 08:59 UTC, Venca B Spam
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Venca B Spam 2017-11-01 08:59:04 UTC
Created attachment 108661 [details]
Bug report illustration

KDevelop C++ code completion does not respect user defined system-wide include directories in CMake projects. Please see attached illustration video.

*How to reproduce?*
1. Open attached sample project.
2. Build it.
3. Open file "SampleLibraryStuff.cpp"
4. On top of that file try to type "#include <My"
5. Press Ctrl+Space to force invoke auto completion.

*What happens?*
KDevelop just proposes operating system wide files/folders along with automatic word completion.

*Expected behavior?*
KDevelop should propose "MyNamespace" directory (user defined system wide include) first along with other matches.

*Notes:*
- Please note the angle bracket "<" which is supposed to be for system wide includes. The local project related includes works well.

- The user defined system wide include directory is specified in CMakeLists.txxt by target_include_directory command argument SYSTEM PUBLIC - for details please see attached sample project.

- My system is using Clang 5.x, CMake 3.8 with server support, KDevelop 5.1.80.
Comment 1 Venca B Spam 2017-11-01 08:59:48 UTC
Created attachment 108662 [details]
Sample project required to reproduce reported behavior.
Comment 2 Venca B Spam 2017-11-17 18:05:52 UTC
I tested KDevelop 5.2.0 from Neon packages today and it still do not work properly.
Comment 3 Sven Brauch 2017-11-17 18:15:30 UTC
Hm, actually I don't think we have a concept of adding system includes from projects at all, so this is not super simple to fix. I see how it's a minor problem in some specific situations, though.
Comment 4 Venca B Spam 2017-11-17 18:29:58 UTC
I did not dig into details (yet) however I was thinking that as long as KDevelop is using Clang as backend for C++ language support (code completion & friends), it does not play a role. 

I mean, if the Clang call for particular project/file is using proper flags/parameters, then KDevelop should receive proper expansion/evaluation of the completion query. 

Or am I completely wrong?
Comment 5 Sven Brauch 2017-11-17 18:39:34 UTC
It's not that simple, we can retrieve information from the build system but we assemble the compiler command line ourselves. We also add your hand-configured includes etc. and that code, from a quick look, simply doesn't have a way to add system includes.
Comment 6 Venca B Spam 2017-11-17 18:49:56 UTC
Ok, I got it.

I have observation that in some cases when the include is already written e.g.

```code
#include <MyNamespace/MyInclude.hpp>
```

The included file can be "clicked-through" (Ctrl+LMB on the included file) which will open the file.

Is this evaluated by different process/principle?

This observation gives me still some home.. :-)
Comment 7 Sven Brauch 2017-11-17 18:56:52 UTC
I think the thing is that clang simply doesn't suggest local includes (specified with -I) when typing #include < even though the resulting code would still be compiling. Since we specify all your includes with -I (I think) the analyzer doesn't complain, but doesn't suggest the completions either.
Comment 8 Venca B Spam 2017-11-18 09:01:05 UTC
Is there a way how I can help? 

I cloned KDevelop just for fun and learning, trying to find out how is designed. Later after some difficulties also compiled it.

Are there any design diagrams (sysml/uml/or just ideas on paper)?
Comment 9 Sven Brauch 2017-11-18 10:10:17 UTC
If you want to fix this, I think you need to do three things:

 1) add an API function systemIncludes() to IDefinesAndIncludesManager
 2) in MakeFileResolver::processOutput, differentiate -I and -isystem (like it differentiates -I and -iframework already)
 3) Refactor how appendPaths() works in ClangParsingEnvironment, because it currently assumes "everything in the project dir is a non-system include".

All this is just from a quick look and could be wrong ... but it would be very nice if you want to have a look at this!
Comment 10 Milian Wolff 2017-11-18 15:20:21 UTC
Uhm sorry to interrupt, but the gif shows an issue that has nothing do with clang. It's our custom include path code completion. seems like that is not querying for (all) the include paths and thus misses one? Should be easy to fix, since it doesn't involve clang ;-)
Comment 11 Sven Brauch 2017-11-18 15:43:08 UTC
Oh, sorry, I thought clang was completing this. Thanks for the interruption.

I still think IDAIM not making this differentation is something that should be fixed eventually.
Comment 12 Milian Wolff 2017-11-18 16:45:23 UTC
@sven: Yes, but that's unrelated to the bug shown here. Its impact would only be marginal anyways, as it only affects the diagnostics and nothing else (i.e. some diagnostics are ignored in system headers).
Comment 13 Milian Wolff 2017-11-18 17:05:05 UTC
ok, the bug really is trivial to fix - we currently ignore system paths for local completion (#include "...") and vice versa ignore project paths for global completion (#include <...>). That is plain wrong.
Comment 14 Milian Wolff 2017-11-18 18:07:58 UTC
Git commit 015141e38b795b6c8ff9fe616e87cf0a55065406 by Milian Wolff.
Committed on 18/11/2017 at 18:07.
Pushed by mwolff into branch '5.2'.

kdev-clang: Offer all include paths for code completion

We used to only offer code completion of project paths for local
code completion in `#include "..."` contexts. And for vice versa,
we only offered system paths for global code completion in
`#include <...>` contexts. This is wrong, as the include style
only changes the order in which a compiler iterates through these
paths to find an include file. For code completion purposes, this
is not important.

Now we offer code completion in both path lists always. To show the
user the right file/dir being included, we don't sort and unify the
search path list anymore as that potentially changes the final result.
Rather, we use a hash set to ensure we don't encounter paths multiple
times and iterate over the search path lists in their original order.

M  +18   -9    plugins/clang/codecompletion/includepathcompletioncontext.cpp
M  +1    -1    plugins/clang/tests/test_codecompletion.cpp

https://commits.kde.org/kdevelop/015141e38b795b6c8ff9fe616e87cf0a55065406
Comment 15 Venca B Spam 2017-11-18 20:43:40 UTC
I did compile the commit 015141e38b795b6c8ff9fe616e87cf0a55065406 and I can confirm that now it works. Even with the simplification described in the commit message, it works for my everyday work.

In the meantime I did started learning how it works while trying to fix it. It helped me understand some part of KDevelop.

Thank you all!