clangd, completion in header files

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

clangd, completion in header files

Eric Fiselier via cfe-dev
Hello list,

thank you for all the work on clangd. I tried it for the first time
with VSCode and I'm really impressed how useful it is just out of the
box.

I however encountered a problem with code completion in header files
and I would like to know if it's a bug in clangd or a problem in my
setup. I work on a C++ project which has headers with .h extension and
clangd incorrectly assumes it's a C code because it emits (besides
other things) "unknown type name 'namespace'" diagnostics message for
the namespace definition.

I run clangd -input-mirror-file for a while and I can see
textDocument/didOpen calls for the .h headers with "languageId" set to
"cpp". If I rename a header to .hpp, the problem disappears. I also
tried configuring custom file extension association for C++ in VSCode
and it worked as well. So I believe there must be something wrong just
with .h (i.e. clangd seems not to respect languageId for .h files).

Please, can you guide me how to diagnose the problem better so I can
eventually fill a bug report?

I also wonder how the completion is supposed to work in headers in
general. Is there some heuristics to guess the compilation flags
because the headers are not in the compilation database (at least when
generated by cmake)?

Regards,

Jan
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
+Sam and Ilya

On Mon, Mar 26, 2018 at 4:04 AM Jan Včelák via cfe-dev <[hidden email]> wrote:
Hello list,

thank you for all the work on clangd. I tried it for the first time
with VSCode and I'm really impressed how useful it is just out of the
box.

I however encountered a problem with code completion in header files
and I would like to know if it's a bug in clangd or a problem in my
setup. I work on a C++ project which has headers with .h extension and
clangd incorrectly assumes it's a C code because it emits (besides
other things) "unknown type name 'namespace'" diagnostics message for
the namespace definition.

I run clangd -input-mirror-file for a while and I can see
textDocument/didOpen calls for the .h headers with "languageId" set to
"cpp". If I rename a header to .hpp, the problem disappears. I also
tried configuring custom file extension association for C++ in VSCode
and it worked as well. So I believe there must be something wrong just
with .h (i.e. clangd seems not to respect languageId for .h files).

Please, can you guide me how to diagnose the problem better so I can
eventually fill a bug report?

I also wonder how the completion is supposed to work in headers in
general. Is there some heuristics to guess the compilation flags
because the headers are not in the compilation database (at least when
generated by cmake)?

Regards,

Jan
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev
Thanks for trying things out, and sorry for the bad header-file experience.

On Mon, Mar 26, 2018 at 4:05 AM Jan Včelák via cfe-dev <[hidden email]> wrote:
Hello list,

thank you for all the work on clangd. I tried it for the first time
with VSCode and I'm really impressed how useful it is just out of the
box.

I however encountered a problem with code completion in header files
and I would like to know if it's a bug in clangd or a problem in my
setup.
Yeah, this is a known missing feature - guessing the right compile flags for headers.
If you have a compilation database (e.g. compile_commands.json) that provides compile commands for headers, then clangd works as expected. However most build systems don't provide this information. (Bazel, for one, does).

When there's no compile command provided, we fall back to 'clang $filename'. Clang treats .h files as C.
But it's also missing a bunch of other information: include paths, defines etc that are likely required to get useful results.
So I don't think just diverging from clang here will actually help many projects (feel free to try this out by editing compile_commands.json - if this works for you it'd be good to know).

What can we do then? A few ideas:
 - we can preprocess all the files from compile_commands.json on startup (https://reviews.llvm.org/D41911 is the start of this). But we can't guarantee we get to the file you care about in time, so behavior will be erratic.
 - we can pick a compile command arbitrarily and take its flags, which will get include paths and defines right if they're uniform across the project
 - we can just refuse to provide any diagnostics/completions where we don't have a known-good set of flags.

I've dumped these thoughts into https://bugs.llvm.org/show_bug.cgi?id=36899 - I also want to solicit some thoughts on this problem at the BoF session at the next LLVM dev meeting.
I'd like to find time to work on this in the next quarter, but I'm not certain - if anyone is interested in attacking this problem, let me know!

I work on a C++ project which has headers with .h extension and
clangd incorrectly assumes it's a C code because it emits (besides
other things) "unknown type name 'namespace'" diagnostics message for
the namespace definition.

I run clangd -input-mirror-file for a while and I can see
textDocument/didOpen calls for the .h headers with "languageId" set to
"cpp". If I rename a header to .hpp, the problem disappears. I also
tried configuring custom file extension association for C++ in VSCode
and it worked as well. So I believe there must be something wrong just
with .h (i.e. clangd seems not to respect languageId for .h files).

Please, can you guide me how to diagnose the problem better so I can
eventually fill a bug report?
I think you've diagnosed it pretty well, but you can get a bit of extra information from the logs clangd writes to stderr. In VSCode, these are visible in the "output" pane, under "clang language server". This will include the command used to build each file.
 

I also wonder how the completion is supposed to work in headers in
general. Is there some heuristics to guess the compilation flags
because the headers are not in the compilation database (at least when
generated by cmake)?

Regards,

Jan

Cheers, Sam 
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
On Montag, 26. März 2018 10:22:05 CEST Sam McCall via cfe-dev wrote:

> Thanks for trying things out, and sorry for the bad header-file experience.
>
> On Mon, Mar 26, 2018 at 4:05 AM Jan Včelák via cfe-dev <
>
> [hidden email]> wrote:
> > Hello list,
> >
> > thank you for all the work on clangd. I tried it for the first time
> > with VSCode and I'm really impressed how useful it is just out of the
> > box.
> >
> > I however encountered a problem with code completion in header files
> > and I would like to know if it's a bug in clangd or a problem in my
> > setup.
>
> Yeah, this is a known missing feature - guessing the right compile flags
> for headers.
> If you have a compilation database (e.g. compile_commands.json) that
> provides compile commands for headers, then clangd works as expected.
> However most build systems don't provide this information. (Bazel, for one,
> does).
>
> When there's no compile command provided, we fall back to 'clang
> $filename'. Clang treats .h files as C.
> But it's also missing a bunch of other information: include paths, defines
> etc that are likely required to get useful results.
> So I don't think just diverging from clang here will actually help many
> projects (feel free to try this out by editing compile_commands.json - if
> this works for you it'd be good to know).
>
> What can we do then? A few ideas:
>  - we can preprocess all the files from compile_commands.json on startup (
> https://reviews.llvm.org/D41911 is the start of this). But we can't
> guarantee we get to the file you care about in time, so behavior will be
> erratic.
>  - we can pick a compile command arbitrarily and take its flags, which will
> get include paths and defines right if they're uniform across the project
>  - we can just refuse to provide any diagnostics/completions where we don't
> have a known-good set of flags.
Another trick that we apply in KDevelop:

- Try to find the *.cpp file for the header based on some file naming
heuristics and use that instead.

This works well in the majority of cases, but fails for system headers,
header-only utilities, and headers you just started to write where the
accompanying *.cpp file is still empty.

Bye

--
Milian Wolff
[hidden email]
http://milianw.de
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
Hi,

In a YCM extension, we have the following heuristics to get compile
flags for a header:
- Try to find a TU for the header in the compile DB by using the
basename of the header.
- If still not found then try to browse the DB to find one TU which
uses the directory of our header as an include path (-I, -isystem).
- If still not found then we try to get the first sibling TU in the
directory of the header file.
- If still not found then fall back to the first entry in the compile
DB and use its flags.

In the last 3 years it has been working remarkable well for us (7
users). It handles nicely the cases when we add a new header.
It may be a bit slow on very large projects and it does not handle
multiplatform builds.
I hope you find some of these ideas helpful. The extension is available here:
https://github.com/martong/ycm_extra_conf.jsondb/blob/master/ycm_jsondb_core.py#L160

Cheers,
Gabor

On Mon, Mar 26, 2018 at 11:21 AM, Milian Wolff via cfe-dev
<[hidden email]> wrote:

> On Montag, 26. März 2018 10:22:05 CEST Sam McCall via cfe-dev wrote:
>> Thanks for trying things out, and sorry for the bad header-file experience.
>>
>> On Mon, Mar 26, 2018 at 4:05 AM Jan Včelák via cfe-dev <
>>
>> [hidden email]> wrote:
>> > Hello list,
>> >
>> > thank you for all the work on clangd. I tried it for the first time
>> > with VSCode and I'm really impressed how useful it is just out of the
>> > box.
>> >
>> > I however encountered a problem with code completion in header files
>> > and I would like to know if it's a bug in clangd or a problem in my
>> > setup.
>>
>> Yeah, this is a known missing feature - guessing the right compile flags
>> for headers.
>> If you have a compilation database (e.g. compile_commands.json) that
>> provides compile commands for headers, then clangd works as expected.
>> However most build systems don't provide this information. (Bazel, for one,
>> does).
>>
>> When there's no compile command provided, we fall back to 'clang
>> $filename'. Clang treats .h files as C.
>> But it's also missing a bunch of other information: include paths, defines
>> etc that are likely required to get useful results.
>> So I don't think just diverging from clang here will actually help many
>> projects (feel free to try this out by editing compile_commands.json - if
>> this works for you it'd be good to know).
>>
>> What can we do then? A few ideas:
>>  - we can preprocess all the files from compile_commands.json on startup (
>> https://reviews.llvm.org/D41911 is the start of this). But we can't
>> guarantee we get to the file you care about in time, so behavior will be
>> erratic.
>>  - we can pick a compile command arbitrarily and take its flags, which will
>> get include paths and defines right if they're uniform across the project
>>  - we can just refuse to provide any diagnostics/completions where we don't
>> have a known-good set of flags.
>
> Another trick that we apply in KDevelop:
>
> - Try to find the *.cpp file for the header based on some file naming
> heuristics and use that instead.
>
> This works well in the majority of cases, but fails for system headers,
> header-only utilities, and headers you just started to write where the
> accompanying *.cpp file is still empty.
>
> Bye
>
> --
> Milian Wolff
> [hidden email]
> http://milianw.de
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
Thanks Milian and Gábor,
I'd been a bit reluctant about using filename heuristics but both your experience and concrete suggestions are compelling.
This has the side benefit that it could reasonably fit inside the CompilationDatabase abstraction, and if we had a high quality implementation we could even make this behavior the default for compile_commands.json processing in all tools. (Headers etc shouldn't be *enumerated* of course, but if a tool explicitly asks how to compile one...)
I'll try this idea out and see how it works, unless someone gets to it first.
Cheers, Sam

On Mon, Mar 26, 2018 at 9:49 PM Gábor Márton <[hidden email]> wrote:
Hi,

In a YCM extension, we have the following heuristics to get compile
flags for a header:
- Try to find a TU for the header in the compile DB by using the
basename of the header.
- If still not found then try to browse the DB to find one TU which
uses the directory of our header as an include path (-I, -isystem).
- If still not found then we try to get the first sibling TU in the
directory of the header file.
- If still not found then fall back to the first entry in the compile
DB and use its flags.

In the last 3 years it has been working remarkable well for us (7
users). It handles nicely the cases when we add a new header.
It may be a bit slow on very large projects and it does not handle
multiplatform builds.
I hope you find some of these ideas helpful. The extension is available here:
https://github.com/martong/ycm_extra_conf.jsondb/blob/master/ycm_jsondb_core.py#L160

Cheers,
Gabor

On Mon, Mar 26, 2018 at 11:21 AM, Milian Wolff via cfe-dev
<[hidden email]> wrote:
> On Montag, 26. März 2018 10:22:05 CEST Sam McCall via cfe-dev wrote:
>> Thanks for trying things out, and sorry for the bad header-file experience.
>>
>> On Mon, Mar 26, 2018 at 4:05 AM Jan Včelák via cfe-dev <
>>
>> [hidden email]> wrote:
>> > Hello list,
>> >
>> > thank you for all the work on clangd. I tried it for the first time
>> > with VSCode and I'm really impressed how useful it is just out of the
>> > box.
>> >
>> > I however encountered a problem with code completion in header files
>> > and I would like to know if it's a bug in clangd or a problem in my
>> > setup.
>>
>> Yeah, this is a known missing feature - guessing the right compile flags
>> for headers.
>> If you have a compilation database (e.g. compile_commands.json) that
>> provides compile commands for headers, then clangd works as expected.
>> However most build systems don't provide this information. (Bazel, for one,
>> does).
>>
>> When there's no compile command provided, we fall back to 'clang
>> $filename'. Clang treats .h files as C.
>> But it's also missing a bunch of other information: include paths, defines
>> etc that are likely required to get useful results.
>> So I don't think just diverging from clang here will actually help many
>> projects (feel free to try this out by editing compile_commands.json - if
>> this works for you it'd be good to know).
>>
>> What can we do then? A few ideas:
>>  - we can preprocess all the files from compile_commands.json on startup (
>> https://reviews.llvm.org/D41911 is the start of this). But we can't
>> guarantee we get to the file you care about in time, so behavior will be
>> erratic.
>>  - we can pick a compile command arbitrarily and take its flags, which will
>> get include paths and defines right if they're uniform across the project
>>  - we can just refuse to provide any diagnostics/completions where we don't
>> have a known-good set of flags.
>
> Another trick that we apply in KDevelop:
>
> - Try to find the *.cpp file for the header based on some file naming
> heuristics and use that instead.
>
> This works well in the majority of cases, but fails for system headers,
> header-only utilities, and headers you just started to write where the
> accompanying *.cpp file is still empty.
>
> Bye
>
> --
> Milian Wolff
> [hidden email]
> http://milianw.de
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev
Hi,

I recently tried to use clangd for Chromium in vscode and hit the same issue. 
I experimented a bit with generating a compile_commands.json file with valid rules for headers and came to this solution: https://gist.github.com/xchrdw/bfd2b3a5f765f4195a55d6351daf1b48
I sorted all .cc filenames and then looked up the index of the closest match for each header using binary search. As there were some edge cases like the first or last file in a directory, I additionally compared which file at the index has the largest prefix with the header.
I found a few issues with system headers and some generated protobuf headers but otherwise it works well.
Native support from clangd would be amazing :)

Thanks,
Christian

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
Are there any interesting codebases I should be trying this stuff out on? Ideally - builds on linux without too much fuss, uses cmake and generates compile_commands.json. Preferably not incredibly huge (though I will try chromium).


I'm playing with heuristics; and have a rough tool to validate them.
This seems to work pretty well:
- 2 points if basenames match (without extension). 1 point for substrings, in either direction.
- for /a/b/c/d/foo.h, 1 point each for "/c/" and "/d/" in the candidate path, and 1 point if it starts with "/a/b"
- break ties by longest common prefix

I've been testing using the LLVM codebase, which has some tricky cases. (nontrivial directory layout, lots of headers without matching cc files)

Out of ~3000 headers (i.e. any file transitively #included) in my llvm checkout, working accuracy seems to be extremely good:
PERFECT (~2200): we pick a TU that transitively included the header. (When there's a corresponding cc file, it's usually that on.)
GOOD (~650): we pick a TU whose args "obviously" work (it contains every arg that was defined by *every* transitively including TU)
MAYBE_BAD (150):
 - about half of these are *.def and *.inc which I haven't studied, they're not relevant to clangd
 - most of the remainder are false alarms: the flags are fine, but my heuristic can't tell
 - I haven't found any that would give wrong results, though some results are confused (clang/Parse/ParseDiagnostic.h gets associated with clang/lib/Basic/Diagnostic.cpp rather than e.g. clang/lib/Parse/ParseAST.cpp).

On Wed, Mar 28, 2018 at 4:12 PM Christian Dullweber via cfe-dev <[hidden email]> wrote:
Hi,

I recently tried to use clangd for Chromium in vscode and hit the same issue. 
I experimented a bit with generating a compile_commands.json file with valid rules for headers and came to this solution: https://gist.github.com/xchrdw/bfd2b3a5f765f4195a55d6351daf1b48
I sorted all .cc filenames and then looked up the index of the closest match for each header using binary search. As there were some edge cases like the first or last file in a directory, I additionally compared which file at the index has the largest prefix with the header.
I found a few issues with system headers and some generated protobuf headers but otherwise it works well. 
Native support from clangd would be amazing :)

Thanks,
Christian
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev
Another heuristic solution:

Enumerate all compilation database entries and calculate the matching score for each entry with the target file
Bonus for common leading components
Penalty for diverged path components
Bonus for common trailing characters sans filename extension (e.g. Match.cc Match.h are the same sans extension)

This heuristic is used in cquery.

Add another point for textDocument/didOpen on an unseen filename:
The inferred command line options are not authoritative, they should be overriden by other translation units when later it turns out the header file is included by some entry in the compilation database.


On Wed, Mar 28, 2018 at 7:12 AM Christian Dullweber via cfe-dev <[hidden email]> wrote:
Hi,

I recently tried to use clangd for Chromium in vscode and hit the same issue. 
I experimented a bit with generating a compile_commands.json file with valid rules for headers and came to this solution: https://gist.github.com/xchrdw/bfd2b3a5f765f4195a55d6351daf1b48
I sorted all .cc filenames and then looked up the index of the closest match for each header using binary search. As there were some edge cases like the first or last file in a directory, I additionally compared which file at the index has the largest prefix with the header.
I found a few issues with system headers and some generated protobuf headers but otherwise it works well.
Native support from clangd would be amazing :)

Thanks,
Christian
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


--
宋方睿

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev
There is another place where such heauristic may be related.

A project may build multiple executables but there is no way to tell
which file a global function call (or variable).
(Linking options are not specified in JSON compilation database and
I can imagine specifying it will be infeasible.)

I personally prefer listing all definitions sharing the same Clang USR,
but there can be heuristics to find the most possible file that defines
the symbol, and make it rank top in the textDocument/definition response list.

On 2018-03-28, Sam McCall via cfe-dev wrote:

>Thanks Milian and Gábor,
>I'd been a bit reluctant about using filename heuristics but both your
>experience and concrete suggestions are compelling.
>This has the side benefit that it could reasonably fit inside the
>CompilationDatabase abstraction, and if we had a high quality implementation we
>could even make this behavior the default for compile_commands.json processing
>in all tools. (Headers etc shouldn't be *enumerated* of course, but if a tool
>explicitly asks how to compile one...)
>I'll try this idea out and see how it works, unless someone gets to it first.
>Cheers, Sam
>
>On Mon, Mar 26, 2018 at 9:49 PM Gábor Márton <[hidden email]> wrote:
>
>    Hi,
>
>    In a YCM extension, we have the following heuristics to get compile
>    flags for a header:
>    - Try to find a TU for the header in the compile DB by using the
>    basename of the header.
>    - If still not found then try to browse the DB to find one TU which
>    uses the directory of our header as an include path (-I, -isystem).
>    - If still not found then we try to get the first sibling TU in the
>    directory of the header file.
>    - If still not found then fall back to the first entry in the compile
>    DB and use its flags.
>
>    In the last 3 years it has been working remarkable well for us (7
>    users). It handles nicely the cases when we add a new header.
>    It may be a bit slow on very large projects and it does not handle
>    multiplatform builds.
>    I hope you find some of these ideas helpful. The extension is available
>    here:
>    https://github.com/martong/ycm_extra_conf.jsondb/blob/master/
>    ycm_jsondb_core.py#L160
>
>    Cheers,
>    Gabor
>
>    On Mon, Mar 26, 2018 at 11:21 AM, Milian Wolff via cfe-dev
>    <[hidden email]> wrote:
>    > On Montag, 26. März 2018 10:22:05 CEST Sam McCall via cfe-dev wrote:
>    >> Thanks for trying things out, and sorry for the bad header-file
>    experience.
>    >>
>    >> On Mon, Mar 26, 2018 at 4:05 AM Jan Včelák via cfe-dev <
>    >>
>    >> [hidden email]> wrote:
>    >> > Hello list,
>    >> >
>    >> > thank you for all the work on clangd. I tried it for the first time
>    >> > with VSCode and I'm really impressed how useful it is just out of the
>    >> > box.
>    >> >
>    >> > I however encountered a problem with code completion in header files
>    >> > and I would like to know if it's a bug in clangd or a problem in my
>    >> > setup.
>    >>
>    >> Yeah, this is a known missing feature - guessing the right compile flags
>    >> for headers.
>    >> If you have a compilation database (e.g. compile_commands.json) that
>    >> provides compile commands for headers, then clangd works as expected.
>    >> However most build systems don't provide this information. (Bazel, for
>    one,
>    >> does).
>    >>
>    >> When there's no compile command provided, we fall back to 'clang
>    >> $filename'. Clang treats .h files as C.
>    >> But it's also missing a bunch of other information: include paths,
>    defines
>    >> etc that are likely required to get useful results.
>    >> So I don't think just diverging from clang here will actually help many
>    >> projects (feel free to try this out by editing compile_commands.json -
>    if
>    >> this works for you it'd be good to know).
>    >>
>    >> What can we do then? A few ideas:
>    >>  - we can preprocess all the files from compile_commands.json on startup
>    (
>    >> https://reviews.llvm.org/D41911 is the start of this). But we can't
>    >> guarantee we get to the file you care about in time, so behavior will be
>    >> erratic.
>    >>  - we can pick a compile command arbitrarily and take its flags, which
>    will
>    >> get include paths and defines right if they're uniform across the
>    project
>    >>  - we can just refuse to provide any diagnostics/completions where we
>    don't
>    >> have a known-good set of flags.
>    >
>    > Another trick that we apply in KDevelop:
>    >
>    > - Try to find the *.cpp file for the header based on some file naming
>    > heuristics and use that instead.
>    >
>    > This works well in the majority of cases, but fails for system headers,
>    > header-only utilities, and headers you just started to write where the
>    > accompanying *.cpp file is still empty.
>    >
>    > Bye
>    >
>    > --
>    > Milian Wolff
>    > [hidden email]
>    > http://milianw.de
>    > _______________________________________________
>    > cfe-dev mailing list
>    > [hidden email]
>    > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>    >
>

>_______________________________________________
>cfe-dev mailing list
>[hidden email]
>http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


--
宋方睿
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev
Looks like our mail crossed :)
Cquery's success with this is another point in favor.

I have a prototype in https://reviews.llvm.org/D45006 (+ 2-line patch to enable in clangd: https://reviews.llvm.org/D45007).
It seems to work pretty well at first glance, but it needs rigorous testing. If anyone's feeling this pain, feel free to try it out!

It does a little bit of indexing to avoid traversing all the entries every time, not sure if that's worth it.

On Wed, Mar 28, 2018 at 9:58 PM Fāng-ruì Sòng via cfe-dev <[hidden email]> wrote:
Another heuristic solution:

Enumerate all compilation database entries and calculate the matching score for each entry with the target file
Bonus for common leading components
Penalty for diverged path components
Bonus for common trailing characters sans filename extension (e.g. Match.cc Match.h are the same sans extension)

This heuristic is used in cquery.

Add another point for textDocument/didOpen on an unseen filename:
The inferred command line options are not authoritative, they should be overriden by other translation units when later it turns out the header file is included by some entry in the compilation database.


On Wed, Mar 28, 2018 at 7:12 AM Christian Dullweber via cfe-dev <[hidden email]> wrote:
Hi,

I recently tried to use clangd for Chromium in vscode and hit the same issue. 
I experimented a bit with generating a compile_commands.json file with valid rules for headers and came to this solution: https://gist.github.com/xchrdw/bfd2b3a5f765f4195a55d6351daf1b48
I sorted all .cc filenames and then looked up the index of the closest match for each header using binary search. As there were some edge cases like the first or last file in a directory, I additionally compared which file at the index has the largest prefix with the header.
I found a few issues with system headers and some generated protobuf headers but otherwise it works well.
Native support from clangd would be amazing :)

Thanks,
Christian
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


--
宋方睿
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
Sound great, thanks! 

One interesting case in Chromium might be generated jni headers. I would be interested what the heuristic finds for these files:
src/out/Android/gen/chrome/browser/jni_headers/chrome/jni/BrowsingDataBridge_jni.h

the implementation is in:
src/chrome/browser/android/browsing_data/browsing_data_bridge.cc
I don't think that will get matched.

I don't really care if code completion works in generated files but at least there shouldn't be incorrect error messages in vscode. Is it possible to exclude a folder from being analyzed by clangd?



On Thu, Mar 29, 2018 at 1:50 AM, Sam McCall <[hidden email]> wrote:
Looks like our mail crossed :)
Cquery's success with this is another point in favor.

I have a prototype in https://reviews.llvm.org/D45006 (+ 2-line patch to enable in clangd: https://reviews.llvm.org/D45007).
It seems to work pretty well at first glance, but it needs rigorous testing. If anyone's feeling this pain, feel free to try it out!

It does a little bit of indexing to avoid traversing all the entries every time, not sure if that's worth it.

On Wed, Mar 28, 2018 at 9:58 PM Fāng-ruì Sòng via cfe-dev <[hidden email]> wrote:
Another heuristic solution:

Enumerate all compilation database entries and calculate the matching score for each entry with the target file
Bonus for common leading components
Penalty for diverged path components
Bonus for common trailing characters sans filename extension (e.g. Match.cc Match.h are the same sans extension)

This heuristic is used in cquery.

Add another point for textDocument/didOpen on an unseen filename:
The inferred command line options are not authoritative, they should be overriden by other translation units when later it turns out the header file is included by some entry in the compilation database.


On Wed, Mar 28, 2018 at 7:12 AM Christian Dullweber via cfe-dev <[hidden email]> wrote:
Hi,

I recently tried to use clangd for Chromium in vscode and hit the same issue. 
I experimented a bit with generating a compile_commands.json file with valid rules for headers and came to this solution: https://gist.github.com/xchrdw/bfd2b3a5f765f4195a55d6351daf1b48
I sorted all .cc filenames and then looked up the index of the closest match for each header using binary search. As there were some edge cases like the first or last file in a directory, I additionally compared which file at the index has the largest prefix with the header.
I found a few issues with system headers and some generated protobuf headers but otherwise it works well.
Native support from clangd would be amazing :)

Thanks,
Christian
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


--
宋方睿
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev

On Thu, Mar 29, 2018 at 11:34 AM Christian Dullweber <[hidden email]> wrote:
Sound great, thanks! 

One interesting case in Chromium might be generated jni headers. I would be interested what the heuristic finds for these files:
src/out/Android/gen/chrome/browser/jni_headers/chrome/jni/BrowsingDataBridge_jni.h
Indeed, it doesn't do well here, but still compiles fine - this seems to be the case when the exact flags used don't matter much as long as the general project setup is right. Probably quite common in generated files - I guess these tend to have relatively few predictable dependencies, and might prefer unwieldy fully-qualified include paths where a human would add a -I entry. This one includes <jni.h> and "../../../../../../../../base/android/jni_generator/jni_generator_helper.h".

The heuristic fares poorly here because the filename is unique, the last few path components (chrome, jni, jni_headers) are unhelpful in locating related cc files, and the "nearby in the tree" logic doesn't handle the separate root for genfiles well.
The right file turns out to be e.g. "chrome/browser/android/browsing_data/browsing_data_bridge.cc". The only useful hints here are chrome/browser (not very specific) and BrowsingDataBridge if we used fuzzy word matching to account for case differences. I'm inclined not to try to solve this case without more data.

The only diagnostics produced are a bunch of "internal linkage... but not defined" where the header forward declares static functions whose implementations are meant to be provided by the including CC file.
This is an artifact of assuming the .h file is standalone, and it seems like a form of non-modularity, but this is a minor issue we can deal with in a few ways and unrelated to the choice of flags.

Relevant logs are:
Chose /usr/local/google/home/sammccall/src/chromium/src/out/Android/gen/chrome/browser/android/proto/client_discourse_context.pb.cc as proxy for /usr/local/google/home/sammccall/src/chromium/src/out/Android/gen/chrome/browser/jni_headers/chrome/jni/BrowsingDataBridge_jni.h

Rebuilding file /usr/local/google/home/sammccall/src/chromium/src/out/Android/gen/chrome/browser/jni_headers/chrome/jni/BrowsingDataBridge_jni.h with command [/usr/local/google/home/sammccall/src/chromium/src/out/Android] ../../third_party/llvm-build/Release+Asserts/bin/clang++ -x c++ -MMD -MF obj/chrome/browser/client_discourse_context_proto/client_discourse_context.pb.o.d -D V8_DEPRECATION_WARNINGS -D NO_TCMALLOC -D SAFE_BROWSING_DB_REMOTE -D CHROMIUM_BUILD -D FIELDTRIAL_TESTING_ENABLED -D ANDROID -D HAVE_SYS_UIO_H -D ANDROID_NDK_VERSION_ROLL=r16_1 -D CR_CLANG_REVISION="328575-1" -D __STDC_CONSTANT_MACROS -D __STDC_FORMAT_MACROS -D COMPONENT_BUILD -D __GNU_SOURCE=1 -D CHROMIUM_CXX_TWEAK_INLINES -D _DEBUG -D DYNAMIC_ANNOTATIONS_ENABLED=1 -D WTF_USE_DYNAMIC_ANNOTATIONS=1 -D GOOGLE_PROTOBUF_NO_RTTI -D GOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -D HAVE_PTHREAD -D PROTOBUF_USE_DLLS -I ../.. -I gen -I ../../third_party/protobuf/src -I gen/protoc_out -I ../../third_party/protobuf/src -fno-strict-aliasing --param ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D __DATE__= -D __TIME__= -D __TIMESTAMP__= -funwind-tables -fPIC -pipe -fcolor-diagnostics -no-canonical-prefixes -ffunction-sections -fno-short-enums --target=arm-linux-androideabi -isystem ../../third_party/android_ndk/sysroot/usr/include/arm-linux-androideabi -D __ANDROID_API__=16 -D __NDK_FPABI__= -D HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC=1 -march=armv7-a -mfloat-abi=softfp -mtune=generic-armv7-a -mfpu=neon -mthumb -Wall -Werror -Wextra -Wimplicit-fallthrough -Wthread-safety -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wno-unused-lambda-capture -Wno-user-defined-warnings -Wno-enum-compare-switch -Wno-null-pointer-arithmetic -Wno-ignored-pragma-optimize -Oz -fno-ident -fdata-sections -ffunction-sections -fomit-frame-pointer -gdwarf-3 -g2 -ggnu-pubnames -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-ipc -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Wno-undefined-bool-conversion -Wno-tautological-undefined-compare -std=gnu++14 -fno-exceptions -fno-rtti -isystem ../../third_party/android_ndk/sources/cxx-stl/llvm-libc++/include -isystem ../../third_party/android_ndk/sources/cxx-stl/llvm-libc++abi/include -isystem ../../third_party/android_ndk/sources/android/support/include --sysroot=../../third_party/android_ndk/sysroot -fvisibility-inlines-hidden -c /usr/local/google/home/sammccall/src/chromium/src/out/Android/gen/chrome/browser/jni_headers/chrome/jni/BrowsingDataBridge_jni.h -resource-dir=/usr/local/google/home/sammccall/llvmbuild/bin/../lib/clang/7.0.0

(Yes, the -M flags should really be filtered)
 

the implementation is in:
src/chrome/browser/android/browsing_data/browsing_data_bridge.cc
I don't think that will get matched.

I don't really care if code completion works in generated files but at least there shouldn't be incorrect error messages in vscode. Is it possible to exclude a folder from being analyzed by clangd?
Not currently, it'll look at anything with a supported extension.
It's not obvious to me how to manage either heuristically or via configuration in a way that's easy to use, so unless people have ideas I'd punt on this and try to improve the results instead.
 



On Thu, Mar 29, 2018 at 1:50 AM, Sam McCall <[hidden email]> wrote:
Looks like our mail crossed :)
Cquery's success with this is another point in favor.

I have a prototype in https://reviews.llvm.org/D45006 (+ 2-line patch to enable in clangd: https://reviews.llvm.org/D45007).
It seems to work pretty well at first glance, but it needs rigorous testing. If anyone's feeling this pain, feel free to try it out!

It does a little bit of indexing to avoid traversing all the entries every time, not sure if that's worth it.

On Wed, Mar 28, 2018 at 9:58 PM Fāng-ruì Sòng via cfe-dev <[hidden email]> wrote:
Another heuristic solution:

Enumerate all compilation database entries and calculate the matching score for each entry with the target file
Bonus for common leading components
Penalty for diverged path components
Bonus for common trailing characters sans filename extension (e.g. Match.cc Match.h are the same sans extension)

This heuristic is used in cquery.

Add another point for textDocument/didOpen on an unseen filename:
The inferred command line options are not authoritative, they should be overriden by other translation units when later it turns out the header file is included by some entry in the compilation database.


On Wed, Mar 28, 2018 at 7:12 AM Christian Dullweber via cfe-dev <[hidden email]> wrote:
Hi,

I recently tried to use clangd for Chromium in vscode and hit the same issue. 
I experimented a bit with generating a compile_commands.json file with valid rules for headers and came to this solution: https://gist.github.com/xchrdw/bfd2b3a5f765f4195a55d6351daf1b48
I sorted all .cc filenames and then looked up the index of the closest match for each header using binary search. As there were some edge cases like the first or last file in a directory, I additionally compared which file at the index has the largest prefix with the header.
I found a few issues with system headers and some generated protobuf headers but otherwise it works well.
Native support from clangd would be amazing :)

Thanks,
Christian
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


--
宋方睿
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev

> On Mar 26, 2018, at 01:22, Sam McCall via cfe-dev <[hidden email]> wrote:
>
> When there's no compile command provided, we fall back to 'clang $filename'. Clang treats .h files as C.

I wonder if, as a fallback, `clang -x objective-c++ $filename` would be more generally useful... it puts Clang into "accept almost everything" mode.  This is the mode LLDB uses for expression evaluation IIRC.
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
That's a good point. Somehow I forgot obj-c++ was a thing... We should switch this.

One silly downside though - I find just "-x c++" won't actually give useful results on real projects.
Does this match others' experience?
The "Unknown type name 'namespace'" error I tend to get from the current behavior is at least predictable and recognizable. If we were parsing as obj-c++ but missing other flags, problems could be more subtle.

But we could of course do even better here - if the CDB could report when a command is fallback/inferred, clangd could insert a warning at the first line of the file. "No compile flags found, using 'clang -x objective-c++'" or so.

What do you think?

On Fri, Mar 30, 2018, 18:55 Duncan P. N. Exon Smith <[hidden email]> wrote:

> On Mar 26, 2018, at 01:22, Sam McCall via cfe-dev <[hidden email]> wrote:
>
> When there's no compile command provided, we fall back to 'clang $filename'. Clang treats .h files as C.

I wonder if, as a fallback, `clang -x objective-c++ $filename` would be more generally useful... it puts Clang into "accept almost everything" mode.  This is the mode LLDB uses for expression evaluation IIRC.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
I think warning that we don't know the right compile flags is a great idea.

On Mar 30, 2018, at 11:47, Sam McCall <[hidden email]> wrote:

That's a good point. Somehow I forgot obj-c++ was a thing... We should switch this.

One silly downside though - I find just "-x c++" won't actually give useful results on real projects.
Does this match others' experience?
The "Unknown type name 'namespace'" error I tend to get from the current behavior is at least predictable and recognizable. If we were parsing as obj-c++ but missing other flags, problems could be more subtle.

But we could of course do even better here - if the CDB could report when a command is fallback/inferred, clangd could insert a warning at the first line of the file. "No compile flags found, using 'clang -x objective-c++'" or so.

What do you think?

On Fri, Mar 30, 2018, 18:55 Duncan P. N. Exon Smith <[hidden email]> wrote:

> On Mar 26, 2018, at 01:22, Sam McCall via cfe-dev <[hidden email]> wrote:
>
> When there's no compile command provided, we fall back to 'clang $filename'. Clang treats .h files as C.

I wonder if, as a fallback, `clang -x objective-c++ $filename` would be more generally useful... it puts Clang into "accept almost everything" mode.  This is the mode LLDB uses for expression evaluation IIRC.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev
Thank you all for the responses.

On Mon, Mar 26, 2018 at 10:22 AM, Sam McCall wrote:
> Yeah, this is a known missing feature - guessing the right compile flags for
> headers.
> If you have a compilation database (e.g. compile_commands.json) that
> provides compile commands for headers, then clangd works as expected.
> However most build systems don't provide this information. (Bazel, for one,
> does).

I see. That makes perfect sense. We use cmake which doesn't export
compile commands for headers unfortunately.

> When there's no compile command provided, we fall back to 'clang $filename'.
> Clang treats .h files as C.
> But it's also missing a bunch of other information: include paths, defines
> etc that are likely required to get useful results.
> So I don't think just diverging from clang here will actually help many
> projects (feel free to try this out by editing compile_commands.json - if
> this works for you it'd be good to know).

I have tried adding a compile_commands.json entry for a header with
flags matching the cpp file. The headers were recognized as C++
because the former errors on keywords like namespace were gone but
something was still wrong. Some STL declarations were not found -
std::shared_ptr for instance. Running the compiler command from
terminal showed 'clang-6.0: warning: treating 'c-header' input as
'c++-header' when in C++ mode, this behavior is deprecated
[-Wdeprecated]' which made me try adding '-x c++'. That fixed the
remaining problem but it's strange as the code was already recognized
as C++ and the command contains explicit -std=c++14 option.

> What can we do then? A few ideas:

There are some good suggestions. I'm sure you will find some good
default and I'm looking forward to try it out. I can just confirm that
the heuristic used by YouCompleteMe is quite nice because that's what
I've been using so far. It's not optimal but it works reasonably well.

Cheers,

Jan
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
Just landed r329582 which has does some guessing when there's no compile command available.
Please try it out!

Related changes I'd still like to make:
 - parse *.h as obj-c++ when there's no compilation database at all (out for review)
 - inject a diagnostic when we're using a fallback/guessed command for diagnostics

On Tue, Apr 3, 2018 at 12:03 AM Jan Včelák <[hidden email]> wrote:
Thank you all for the responses.

On Mon, Mar 26, 2018 at 10:22 AM, Sam McCall wrote:
> Yeah, this is a known missing feature - guessing the right compile flags for
> headers.
> If you have a compilation database (e.g. compile_commands.json) that
> provides compile commands for headers, then clangd works as expected.
> However most build systems don't provide this information. (Bazel, for one,
> does).

I see. That makes perfect sense. We use cmake which doesn't export
compile commands for headers unfortunately.

> When there's no compile command provided, we fall back to 'clang $filename'.
> Clang treats .h files as C.
> But it's also missing a bunch of other information: include paths, defines
> etc that are likely required to get useful results.
> So I don't think just diverging from clang here will actually help many
> projects (feel free to try this out by editing compile_commands.json - if
> this works for you it'd be good to know).

I have tried adding a compile_commands.json entry for a header with
flags matching the cpp file. The headers were recognized as C++
because the former errors on keywords like namespace were gone but
something was still wrong. Some STL declarations were not found -
std::shared_ptr for instance. Running the compiler command from
terminal showed 'clang-6.0: warning: treating 'c-header' input as
'c++-header' when in C++ mode, this behavior is deprecated
[-Wdeprecated]' which made me try adding '-x c++'. That fixed the
remaining problem but it's strange as the code was already recognized
as C++ and the command contains explicit -std=c++14 option.

> What can we do then? A few ideas:

There are some good suggestions. I'm sure you will find some good
default and I'm looking forward to try it out. I can just confirm that
the heuristic used by YouCompleteMe is quite nice because that's what
I've been using so far. It's not optimal but it works reasonably well.

Cheers,

Jan

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
In reply to this post by Eric Fiselier via cfe-dev
Hi,
thanks for implementing the heuristics. Header files that have a cc file with the same name are working great now :)

I found a few issues that I can't really explain. It looks like some header-only files are matched with a basically random file:
base/no_destructor.h, base/macros.h and base/callback_forward.h are compiled with commands for third_party/freetype/freetype_source/ftbase.c. 
(according to the "-MF obj/third_party/freetype/freetype_source/ftbase.o.d" parameter)
components/content_settings/core/browser/user_modifiable_provider.h is compiled with commands for components/autofill/core/browser/browser/address.cc

There are much better matches such as base/callback_helpers.cc or components/content_settings/core/browser/website_settings_info.cc.

There are also headers where good commands are choosen, e.g. components/browsing_data/core/clear_browsing_data_tab.h with components/browsing_data/core/core/browsing_data_utils.cc, so I wonder what is going wrong with the others files?

I'm using the clangd build 20180424.RC0-1


On Thu, Mar 29, 2018 at 4:59 PM Sam McCall <[hidden email]> wrote:

On Thu, Mar 29, 2018 at 11:34 AM Christian Dullweber <[hidden email]> wrote:
Sound great, thanks! 

One interesting case in Chromium might be generated jni headers. I would be interested what the heuristic finds for these files:
src/out/Android/gen/chrome/browser/jni_headers/chrome/jni/BrowsingDataBridge_jni.h
Indeed, it doesn't do well here, but still compiles fine - this seems to be the case when the exact flags used don't matter much as long as the general project setup is right. Probably quite common in generated files - I guess these tend to have relatively few predictable dependencies, and might prefer unwieldy fully-qualified include paths where a human would add a -I entry. This one includes <jni.h> and "../../../../../../../../base/android/jni_generator/jni_generator_helper.h".

The heuristic fares poorly here because the filename is unique, the last few path components (chrome, jni, jni_headers) are unhelpful in locating related cc files, and the "nearby in the tree" logic doesn't handle the separate root for genfiles well.
The right file turns out to be e.g. "chrome/browser/android/browsing_data/browsing_data_bridge.cc". The only useful hints here are chrome/browser (not very specific) and BrowsingDataBridge if we used fuzzy word matching to account for case differences. I'm inclined not to try to solve this case without more data.

The only diagnostics produced are a bunch of "internal linkage... but not defined" where the header forward declares static functions whose implementations are meant to be provided by the including CC file.
This is an artifact of assuming the .h file is standalone, and it seems like a form of non-modularity, but this is a minor issue we can deal with in a few ways and unrelated to the choice of flags.

Relevant logs are:
Chose /usr/local/google/home/sammccall/src/chromium/src/out/Android/gen/chrome/browser/android/proto/client_discourse_context.pb.cc as proxy for /usr/local/google/home/sammccall/src/chromium/src/out/Android/gen/chrome/browser/jni_headers/chrome/jni/BrowsingDataBridge_jni.h

Rebuilding file /usr/local/google/home/sammccall/src/chromium/src/out/Android/gen/chrome/browser/jni_headers/chrome/jni/BrowsingDataBridge_jni.h with command [/usr/local/google/home/sammccall/src/chromium/src/out/Android] ../../third_party/llvm-build/Release+Asserts/bin/clang++ -x c++ -MMD -MF obj/chrome/browser/client_discourse_context_proto/client_discourse_context.pb.o.d -D V8_DEPRECATION_WARNINGS -D NO_TCMALLOC -D SAFE_BROWSING_DB_REMOTE -D CHROMIUM_BUILD -D FIELDTRIAL_TESTING_ENABLED -D ANDROID -D HAVE_SYS_UIO_H -D ANDROID_NDK_VERSION_ROLL=r16_1 -D CR_CLANG_REVISION="328575-1" -D __STDC_CONSTANT_MACROS -D __STDC_FORMAT_MACROS -D COMPONENT_BUILD -D __GNU_SOURCE=1 -D CHROMIUM_CXX_TWEAK_INLINES -D _DEBUG -D DYNAMIC_ANNOTATIONS_ENABLED=1 -D WTF_USE_DYNAMIC_ANNOTATIONS=1 -D GOOGLE_PROTOBUF_NO_RTTI -D GOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -D HAVE_PTHREAD -D PROTOBUF_USE_DLLS -I ../.. -I gen -I ../../third_party/protobuf/src -I gen/protoc_out -I ../../third_party/protobuf/src -fno-strict-aliasing --param ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D __DATE__= -D __TIME__= -D __TIMESTAMP__= -funwind-tables -fPIC -pipe -fcolor-diagnostics -no-canonical-prefixes -ffunction-sections -fno-short-enums --target=arm-linux-androideabi -isystem ../../third_party/android_ndk/sysroot/usr/include/arm-linux-androideabi -D __ANDROID_API__=16 -D __NDK_FPABI__= -D HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC=1 -march=armv7-a -mfloat-abi=softfp -mtune=generic-armv7-a -mfpu=neon -mthumb -Wall -Werror -Wextra -Wimplicit-fallthrough -Wthread-safety -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wno-unused-lambda-capture -Wno-user-defined-warnings -Wno-enum-compare-switch -Wno-null-pointer-arithmetic -Wno-ignored-pragma-optimize -Oz -fno-ident -fdata-sections -ffunction-sections -fomit-frame-pointer -gdwarf-3 -g2 -ggnu-pubnames -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-ipc -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Wno-undefined-bool-conversion -Wno-tautological-undefined-compare -std=gnu++14 -fno-exceptions -fno-rtti -isystem ../../third_party/android_ndk/sources/cxx-stl/llvm-libc++/include -isystem ../../third_party/android_ndk/sources/cxx-stl/llvm-libc++abi/include -isystem ../../third_party/android_ndk/sources/android/support/include --sysroot=../../third_party/android_ndk/sysroot -fvisibility-inlines-hidden -c /usr/local/google/home/sammccall/src/chromium/src/out/Android/gen/chrome/browser/jni_headers/chrome/jni/BrowsingDataBridge_jni.h -resource-dir=/usr/local/google/home/sammccall/llvmbuild/bin/../lib/clang/7.0.0

(Yes, the -M flags should really be filtered)
 

the implementation is in:
src/chrome/browser/android/browsing_data/browsing_data_bridge.cc
I don't think that will get matched.

I don't really care if code completion works in generated files but at least there shouldn't be incorrect error messages in vscode. Is it possible to exclude a folder from being analyzed by clangd?
Not currently, it'll look at anything with a supported extension.
It's not obvious to me how to manage either heuristically or via configuration in a way that's easy to use, so unless people have ideas I'd punt on this and try to improve the results instead.
 



On Thu, Mar 29, 2018 at 1:50 AM, Sam McCall <[hidden email]> wrote:
Looks like our mail crossed :)
Cquery's success with this is another point in favor.

I have a prototype in https://reviews.llvm.org/D45006 (+ 2-line patch to enable in clangd: https://reviews.llvm.org/D45007).
It seems to work pretty well at first glance, but it needs rigorous testing. If anyone's feeling this pain, feel free to try it out!

It does a little bit of indexing to avoid traversing all the entries every time, not sure if that's worth it.

On Wed, Mar 28, 2018 at 9:58 PM Fāng-ruì Sòng via cfe-dev <[hidden email]> wrote:
Another heuristic solution:

Enumerate all compilation database entries and calculate the matching score for each entry with the target file
Bonus for common leading components
Penalty for diverged path components
Bonus for common trailing characters sans filename extension (e.g. Match.cc Match.h are the same sans extension)

This heuristic is used in cquery.

Add another point for textDocument/didOpen on an unseen filename:
The inferred command line options are not authoritative, they should be overriden by other translation units when later it turns out the header file is included by some entry in the compilation database.


On Wed, Mar 28, 2018 at 7:12 AM Christian Dullweber via cfe-dev <[hidden email]> wrote:
Hi,

I recently tried to use clangd for Chromium in vscode and hit the same issue. 
I experimented a bit with generating a compile_commands.json file with valid rules for headers and came to this solution: https://gist.github.com/xchrdw/bfd2b3a5f765f4195a55d6351daf1b48
I sorted all .cc filenames and then looked up the index of the closest match for each header using binary search. As there were some edge cases like the first or last file in a directory, I additionally compared which file at the index has the largest prefix with the header.
I found a few issues with system headers and some generated protobuf headers but otherwise it works well.
Native support from clangd would be amazing :)

Thanks,
Christian
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


--
宋方睿
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: clangd, completion in header files

Eric Fiselier via cfe-dev
Ah, this is a fun case... logs from a debug clangd with -debug-only=interpolate:

interpolate: chose ../../third_party/freetype/src/src/base/ftbase.c as proxy for /usr/local/google/home/sammccall/src/chromium/src/base/no_destructor.h preferring none score=3

The trick is the good match between src/src/base and src/base yields 3 points (should probably be two) - clangd doesn't know that "src" is semantically meaningless, and "base" is at least ambiguous.

This yields at least a few few possible approaches:
1. just fix the scorer to award 2 points instead of 3, which will make this rarer (should happen in any case)
2. blacklist words like "src" from getting points
3. dynamically detect which words are significant at startup time based on frequency
4. more substantially change the approach to path matching
...

I quite like the idea of 3, we could index the N rarest directory segments for each command, rather than the N last.

On Wed, Apr 25, 2018 at 4:34 PM Christian Dullweber <[hidden email]> wrote:
Hi,
thanks for implementing the heuristics. Header files that have a cc file with the same name are working great now :)

I found a few issues that I can't really explain. It looks like some header-only files are matched with a basically random file:
base/no_destructor.h, base/macros.h and base/callback_forward.h are compiled with commands for third_party/freetype/freetype_source/ftbase.c. 
(according to the "-MF obj/third_party/freetype/freetype_source/ftbase.o.d" parameter)
components/content_settings/core/browser/user_modifiable_provider.h is compiled with commands for components/autofill/core/browser/browser/address.cc

There are much better matches such as base/callback_helpers.cc or components/content_settings/core/browser/website_settings_info.cc.

There are also headers where good commands are choosen, e.g. components/browsing_data/core/clear_browsing_data_tab.h with components/browsing_data/core/core/browsing_data_utils.cc, so I wonder what is going wrong with the others files?

I'm using the clangd build 20180424.RC0-1


On Thu, Mar 29, 2018 at 4:59 PM Sam McCall <[hidden email]> wrote:

On Thu, Mar 29, 2018 at 11:34 AM Christian Dullweber <[hidden email]> wrote:
Sound great, thanks! 

One interesting case in Chromium might be generated jni headers. I would be interested what the heuristic finds for these files:
src/out/Android/gen/chrome/browser/jni_headers/chrome/jni/BrowsingDataBridge_jni.h
Indeed, it doesn't do well here, but still compiles fine - this seems to be the case when the exact flags used don't matter much as long as the general project setup is right. Probably quite common in generated files - I guess these tend to have relatively few predictable dependencies, and might prefer unwieldy fully-qualified include paths where a human would add a -I entry. This one includes <jni.h> and "../../../../../../../../base/android/jni_generator/jni_generator_helper.h".

The heuristic fares poorly here because the filename is unique, the last few path components (chrome, jni, jni_headers) are unhelpful in locating related cc files, and the "nearby in the tree" logic doesn't handle the separate root for genfiles well.
The right file turns out to be e.g. "chrome/browser/android/browsing_data/browsing_data_bridge.cc". The only useful hints here are chrome/browser (not very specific) and BrowsingDataBridge if we used fuzzy word matching to account for case differences. I'm inclined not to try to solve this case without more data.

The only diagnostics produced are a bunch of "internal linkage... but not defined" where the header forward declares static functions whose implementations are meant to be provided by the including CC file.
This is an artifact of assuming the .h file is standalone, and it seems like a form of non-modularity, but this is a minor issue we can deal with in a few ways and unrelated to the choice of flags.

Relevant logs are:
Chose /usr/local/google/home/sammccall/src/chromium/src/out/Android/gen/chrome/browser/android/proto/client_discourse_context.pb.cc as proxy for /usr/local/google/home/sammccall/src/chromium/src/out/Android/gen/chrome/browser/jni_headers/chrome/jni/BrowsingDataBridge_jni.h

Rebuilding file /usr/local/google/home/sammccall/src/chromium/src/out/Android/gen/chrome/browser/jni_headers/chrome/jni/BrowsingDataBridge_jni.h with command [/usr/local/google/home/sammccall/src/chromium/src/out/Android] ../../third_party/llvm-build/Release+Asserts/bin/clang++ -x c++ -MMD -MF obj/chrome/browser/client_discourse_context_proto/client_discourse_context.pb.o.d -D V8_DEPRECATION_WARNINGS -D NO_TCMALLOC -D SAFE_BROWSING_DB_REMOTE -D CHROMIUM_BUILD -D FIELDTRIAL_TESTING_ENABLED -D ANDROID -D HAVE_SYS_UIO_H -D ANDROID_NDK_VERSION_ROLL=r16_1 -D CR_CLANG_REVISION="328575-1" -D __STDC_CONSTANT_MACROS -D __STDC_FORMAT_MACROS -D COMPONENT_BUILD -D __GNU_SOURCE=1 -D CHROMIUM_CXX_TWEAK_INLINES -D _DEBUG -D DYNAMIC_ANNOTATIONS_ENABLED=1 -D WTF_USE_DYNAMIC_ANNOTATIONS=1 -D GOOGLE_PROTOBUF_NO_RTTI -D GOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -D HAVE_PTHREAD -D PROTOBUF_USE_DLLS -I ../.. -I gen -I ../../third_party/protobuf/src -I gen/protoc_out -I ../../third_party/protobuf/src -fno-strict-aliasing --param ssp-buffer-size=4 -fstack-protector -Wno-builtin-macro-redefined -D __DATE__= -D __TIME__= -D __TIMESTAMP__= -funwind-tables -fPIC -pipe -fcolor-diagnostics -no-canonical-prefixes -ffunction-sections -fno-short-enums --target=arm-linux-androideabi -isystem ../../third_party/android_ndk/sysroot/usr/include/arm-linux-androideabi -D __ANDROID_API__=16 -D __NDK_FPABI__= -D HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC=1 -march=armv7-a -mfloat-abi=softfp -mtune=generic-armv7-a -mfpu=neon -mthumb -Wall -Werror -Wextra -Wimplicit-fallthrough -Wthread-safety -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wno-unused-lambda-capture -Wno-user-defined-warnings -Wno-enum-compare-switch -Wno-null-pointer-arithmetic -Wno-ignored-pragma-optimize -Oz -fno-ident -fdata-sections -ffunction-sections -fomit-frame-pointer -gdwarf-3 -g2 -ggnu-pubnames -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.so -Xclang -add-plugin -Xclang find-bad-constructs -Xclang -plugin-arg-find-bad-constructs -Xclang check-ipc -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Wno-undefined-bool-conversion -Wno-tautological-undefined-compare -std=gnu++14 -fno-exceptions -fno-rtti -isystem ../../third_party/android_ndk/sources/cxx-stl/llvm-libc++/include -isystem ../../third_party/android_ndk/sources/cxx-stl/llvm-libc++abi/include -isystem ../../third_party/android_ndk/sources/android/support/include --sysroot=../../third_party/android_ndk/sysroot -fvisibility-inlines-hidden -c /usr/local/google/home/sammccall/src/chromium/src/out/Android/gen/chrome/browser/jni_headers/chrome/jni/BrowsingDataBridge_jni.h -resource-dir=/usr/local/google/home/sammccall/llvmbuild/bin/../lib/clang/7.0.0

(Yes, the -M flags should really be filtered)
 

the implementation is in:
src/chrome/browser/android/browsing_data/browsing_data_bridge.cc
I don't think that will get matched.

I don't really care if code completion works in generated files but at least there shouldn't be incorrect error messages in vscode. Is it possible to exclude a folder from being analyzed by clangd?
Not currently, it'll look at anything with a supported extension.
It's not obvious to me how to manage either heuristically or via configuration in a way that's easy to use, so unless people have ideas I'd punt on this and try to improve the results instead.
 



On Thu, Mar 29, 2018 at 1:50 AM, Sam McCall <[hidden email]> wrote:
Looks like our mail crossed :)
Cquery's success with this is another point in favor.

I have a prototype in https://reviews.llvm.org/D45006 (+ 2-line patch to enable in clangd: https://reviews.llvm.org/D45007).
It seems to work pretty well at first glance, but it needs rigorous testing. If anyone's feeling this pain, feel free to try it out!

It does a little bit of indexing to avoid traversing all the entries every time, not sure if that's worth it.

On Wed, Mar 28, 2018 at 9:58 PM Fāng-ruì Sòng via cfe-dev <[hidden email]> wrote:
Another heuristic solution:

Enumerate all compilation database entries and calculate the matching score for each entry with the target file
Bonus for common leading components
Penalty for diverged path components
Bonus for common trailing characters sans filename extension (e.g. Match.cc Match.h are the same sans extension)

This heuristic is used in cquery.

Add another point for textDocument/didOpen on an unseen filename:
The inferred command line options are not authoritative, they should be overriden by other translation units when later it turns out the header file is included by some entry in the compilation database.


On Wed, Mar 28, 2018 at 7:12 AM Christian Dullweber via cfe-dev <[hidden email]> wrote:
Hi,

I recently tried to use clangd for Chromium in vscode and hit the same issue. 
I experimented a bit with generating a compile_commands.json file with valid rules for headers and came to this solution: https://gist.github.com/xchrdw/bfd2b3a5f765f4195a55d6351daf1b48
I sorted all .cc filenames and then looked up the index of the closest match for each header using binary search. As there were some edge cases like the first or last file in a directory, I additionally compared which file at the index has the largest prefix with the header.
I found a few issues with system headers and some generated protobuf headers but otherwise it works well.
Native support from clangd would be amazing :)

Thanks,
Christian
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


--
宋方睿
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev