Hello!
I have been trying to get clangd working properly with tree_nocb.h. clangd
trips quite badly when trying to build tree_nocb.h to generate ASTs.
I get something like this in the clangd logs showing it 'infers' how to build
tree_nocb.h because it could not find a command in compile_commands.json:
ASTWorker building file [..]/tree_nocb.h version 9 with command inferred from
[..]/kernel/rcu/tree.c
This leads to all hell breaking lose with complaints about missing rcu_data
struct definition and so forth.
So far I came up with a workaround as follows, but is there a better way?
1. Open compile_commands.json and add a new entry as follows, with a
definition "-DNOCB_H_CLANGD_PARSE". Otherwise the entry is indentical to how
tree.c is built.
{
"arguments": [
"/usr/bin/clang",
"-Wp,-MMD,kernel/rcu/.treenocb.o.d",
"-nostdinc",
"-I./arch/x86/include",
"-I./arch/x86/include/generated",
"-I./include",
"-I./arch/x86/include/uapi",
[...]
"-Wformat-zero-length",
"-Wnonnull",
"-Wformat-insufficient-args",
"-Wno-sign-compare",
"-Wno-pointer-to-enum-cast",
"-Wno-tautological-constant-out-of-range-compare",
"-Wno-unaligned-access",
"-DKBUILD_MODFILE=\"kernel/rcu/tree\"",
"-DKBUILD_BASENAME=\"tree\"",
"-DKBUILD_MODNAME=\"tree\"",
"-D__KBUILD_MODNAME=kmod_tree",
"-DNOCB_H_CLANGD_PARSE",
"-c",
"-I",
"/s/",
"-I",
"/s/",
"-o",
"kernel/rcu/tree_nocb.h.o",
"kernel/rcu/tree_nocb.h"
],
"directory": "/usr/local/google/home/joelaf/repo/linux-master",
"file": "/usr/local/google/home/joelaf/repo/linux-master/kernel/rcu/tree_nocb.h",
"output": "/usr/local/google/home/joelaf/repo/linux-master/kernel/rcu/tree_nocb.h.o"
},
2.
Then in kernel/tree/tree_nocb.h, I do the following right in the beginning.
(Thanks to paulmck@ for this idea).
#ifdef NOCB_H_CLANGD_PARSE
#include "tree.c"
#endif
3. To prevent the above inclusion of tree.c from recursively including
tree_nocb.h, I do the following at the end of tree.c
+#ifndef NOCB_H_CLANGD_PARSE
#include "tree_nocb.h"
-#include "tree_plugin.h"
+#endif
+#include "tree_plugin.h"
With that it works, but if I ever generate compile_commands.json again, then
I'll have to again modify compile_commands.json manually to make my editor
work again with clangd.
So I guess my questions are:
1. Is there a 'standard' procedure to solve something like this?
2. How do we fix this the right way?
One way would be for scripts/clang-tools/gen_compile_commands.py to parse
header files and generate suitable compile_commands.json based on
meta-data in the header file.
3. How do we fix this for other header files in general? Do we have to make hacks like
above (sad face) or can we come up with a standard way to make it work for kernel
sources?
Thank you!
- Joel
Hello,
One way to fix this could be to add this to the beginning of tree_nocb.h
/* Make clangd understand tree_nocb.h */
+#ifdef CLANGD_PARSER_ACTIVE
+#define TREE_NOCB_H_CLANGD
+#include "tree.c"
+#endif
And then at the end of tree.c, do this to prevent recursion:
+#ifndef TREE_NOCB_H_CLANGD
#include "tree_nocb.h"
-#include "tree_plugin.h"
+#endif
+#include "tree_plugin.h"
Then in scripts/clang-tools/gen_compile_commands.py, we can just make
it add "-DCLANGD_PARSER_ACTIVE" to all compile command entries in the
JSON file.
Would that be an acceptable solution? Let me know your opinion (both
Paul and clangd people :)) so I can work on a patch to do this.
- Joel
On Thu, Apr 13, 2023 at 8:53 PM Joel Fernandes <[email protected]> wrote:
>
> Hello!
>
> I have been trying to get clangd working properly with tree_nocb.h. clangd
> trips quite badly when trying to build tree_nocb.h to generate ASTs.
>
> I get something like this in the clangd logs showing it 'infers' how to build
> tree_nocb.h because it could not find a command in compile_commands.json:
>
> ASTWorker building file [..]/tree_nocb.h version 9 with command inferred from
> [..]/kernel/rcu/tree.c
>
> This leads to all hell breaking lose with complaints about missing rcu_data
> struct definition and so forth.
>
> So far I came up with a workaround as follows, but is there a better way?
>
> 1. Open compile_commands.json and add a new entry as follows, with a
> definition "-DNOCB_H_CLANGD_PARSE". Otherwise the entry is indentical to how
> tree.c is built.
>
> {
> "arguments": [
> "/usr/bin/clang",
> "-Wp,-MMD,kernel/rcu/.treenocb.o.d",
> "-nostdinc",
> "-I./arch/x86/include",
> "-I./arch/x86/include/generated",
> "-I./include",
> "-I./arch/x86/include/uapi",
> [...]
> "-Wformat-zero-length",
> "-Wnonnull",
> "-Wformat-insufficient-args",
> "-Wno-sign-compare",
> "-Wno-pointer-to-enum-cast",
> "-Wno-tautological-constant-out-of-range-compare",
> "-Wno-unaligned-access",
> "-DKBUILD_MODFILE=\"kernel/rcu/tree\"",
> "-DKBUILD_BASENAME=\"tree\"",
> "-DKBUILD_MODNAME=\"tree\"",
> "-D__KBUILD_MODNAME=kmod_tree",
> "-DNOCB_H_CLANGD_PARSE",
> "-c",
> "-I",
> "/s/",
> "-I",
> "/s/",
> "-o",
> "kernel/rcu/tree_nocb.h.o",
> "kernel/rcu/tree_nocb.h"
> ],
> "directory": "/usr/local/google/home/joelaf/repo/linux-master",
> "file": "/usr/local/google/home/joelaf/repo/linux-master/kernel/rcu/tree_nocb.h",
> "output": "/usr/local/google/home/joelaf/repo/linux-master/kernel/rcu/tree_nocb.h.o"
> },
>
> 2.
> Then in kernel/tree/tree_nocb.h, I do the following right in the beginning.
> (Thanks to paulmck@ for this idea).
>
> #ifdef NOCB_H_CLANGD_PARSE
> #include "tree.c"
> #endif
>
> 3. To prevent the above inclusion of tree.c from recursively including
> tree_nocb.h, I do the following at the end of tree.c
>
> +#ifndef NOCB_H_CLANGD_PARSE
> #include "tree_nocb.h"
> -#include "tree_plugin.h"
> +#endif
> +#include "tree_plugin.h"
>
> With that it works, but if I ever generate compile_commands.json again, then
> I'll have to again modify compile_commands.json manually to make my editor
> work again with clangd.
>
> So I guess my questions are:
>
> 1. Is there a 'standard' procedure to solve something like this?
>
> 2. How do we fix this the right way?
> One way would be for scripts/clang-tools/gen_compile_commands.py to parse
> header files and generate suitable compile_commands.json based on
> meta-data in the header file.
>
> 3. How do we fix this for other header files in general? Do we have to make hacks like
> above (sad face) or can we come up with a standard way to make it work for kernel
> sources?
>
> Thank you!
>
> - Joel
>
>
>
>
>
>
On Thu, Apr 13, 2023 at 9:11 PM Joel Fernandes <[email protected]> wrote:
>
> Hello,
>
> One way to fix this could be to add this to the beginning of tree_nocb.h
>
> /* Make clangd understand tree_nocb.h */
> +#ifdef CLANGD_PARSER_ACTIVE
> +#define TREE_NOCB_H_CLANGD
> +#include "tree.c"
> +#endif
>
> And then at the end of tree.c, do this to prevent recursion:
> +#ifndef TREE_NOCB_H_CLANGD
> #include "tree_nocb.h"
> -#include "tree_plugin.h"
> +#endif
> +#include "tree_plugin.h"
>
> Then in scripts/clang-tools/gen_compile_commands.py, we can just make
> it add "-DCLANGD_PARSER_ACTIVE" to all compile command entries in the
> JSON file.
>
Ah and even that will not fully work, because there is no entry for
tree_nocb.h in compile_commands.json to begin with :-(. However, that
also can be fixed. Here's how:
We can just add metadata to tree_nocb.h , something like:
/* clangd_infer_from: tree.c */
Then we make scripts/clang-tools/gen_compile_commands.py duplicate
the compile command JSON entry for tree.c but for tree_nocb.h.
Lastly, we add -DDCLANGD_PARSER_ACTIVE to all entries (including the duplicate).
- Joel
On Thu, Apr 13, 2023 at 5:53 PM Joel Fernandes <[email protected]> wrote:
>
> Hello!
>
> I have been trying to get clangd working properly with tree_nocb.h. clangd
> trips quite badly when trying to build tree_nocb.h to generate ASTs.
Hi Joel,
Thanks for the report. What are you using clangd for? I'll bet
something interesting.
I've never used it myself, so I don't know where to even begin with
how to reproduce the issue.
It might be worth filing a bug upstream at
https://github.com/llvm/llvm-project/issues
or internally under the component
Language Platforms > C++ > Clang > Tools > Clangd
with detailed steps to reproduce (and what the observed error actually
is). Feel free to cc me, though I don't know the first thing about
clangd.
>
> I get something like this in the clangd logs showing it 'infers' how to build
> tree_nocb.h because it could not find a command in compile_commands.json:
>
> ASTWorker building file [..]/tree_nocb.h version 9 with command inferred from
> [..]/kernel/rcu/tree.c
>
> This leads to all hell breaking lose with complaints about missing rcu_data
> struct definition and so forth.
>
> So far I came up with a workaround as follows, but is there a better way?
>
> 1. Open compile_commands.json and add a new entry as follows, with a
> definition "-DNOCB_H_CLANGD_PARSE". Otherwise the entry is indentical to how
> tree.c is built.
>
> {
> "arguments": [
> "/usr/bin/clang",
> "-Wp,-MMD,kernel/rcu/.treenocb.o.d",
> "-nostdinc",
> "-I./arch/x86/include",
> "-I./arch/x86/include/generated",
> "-I./include",
> "-I./arch/x86/include/uapi",
> [...]
> "-Wformat-zero-length",
> "-Wnonnull",
> "-Wformat-insufficient-args",
> "-Wno-sign-compare",
> "-Wno-pointer-to-enum-cast",
> "-Wno-tautological-constant-out-of-range-compare",
> "-Wno-unaligned-access",
> "-DKBUILD_MODFILE=\"kernel/rcu/tree\"",
> "-DKBUILD_BASENAME=\"tree\"",
> "-DKBUILD_MODNAME=\"tree\"",
> "-D__KBUILD_MODNAME=kmod_tree",
> "-DNOCB_H_CLANGD_PARSE",
> "-c",
> "-I",
> "/s/",
> "-I",
> "/s/",
> "-o",
> "kernel/rcu/tree_nocb.h.o",
> "kernel/rcu/tree_nocb.h"
> ],
> "directory": "/usr/local/google/home/joelaf/repo/linux-master",
> "file": "/usr/local/google/home/joelaf/repo/linux-master/kernel/rcu/tree_nocb.h",
> "output": "/usr/local/google/home/joelaf/repo/linux-master/kernel/rcu/tree_nocb.h.o"
> },
>
> 2.
> Then in kernel/tree/tree_nocb.h, I do the following right in the beginning.
> (Thanks to paulmck@ for this idea).
>
> #ifdef NOCB_H_CLANGD_PARSE
> #include "tree.c"
> #endif
>
> 3. To prevent the above inclusion of tree.c from recursively including
> tree_nocb.h, I do the following at the end of tree.c
>
> +#ifndef NOCB_H_CLANGD_PARSE
> #include "tree_nocb.h"
> -#include "tree_plugin.h"
> +#endif
> +#include "tree_plugin.h"
>
> With that it works, but if I ever generate compile_commands.json again, then
> I'll have to again modify compile_commands.json manually to make my editor
> work again with clangd.
>
> So I guess my questions are:
>
> 1. Is there a 'standard' procedure to solve something like this?
>
> 2. How do we fix this the right way?
> One way would be for scripts/clang-tools/gen_compile_commands.py to parse
> header files and generate suitable compile_commands.json based on
> meta-data in the header file.
>
> 3. How do we fix this for other header files in general? Do we have to make hacks like
> above (sad face) or can we come up with a standard way to make it work for kernel
> sources?
>
> Thank you!
>
> - Joel
>
>
>
>
>
>
--
Thanks,
~Nick Desaulniers
+ Florent
Joel, Florent is doing some cool stuff with clangd and kernels (check
out the demo at go/linux-kernel-vscode). I'm pushing Florent to
publish some of the cool stuff he's been working on externally because
it is awesome. ;)
Florent, have you seen any such issues such as what Joel reported below?
On Fri, Apr 14, 2023 at 3:47 PM Nick Desaulniers
<[email protected]> wrote:
>
> On Thu, Apr 13, 2023 at 5:53 PM Joel Fernandes <[email protected]> wrote:
> >
> > Hello!
> >
> > I have been trying to get clangd working properly with tree_nocb.h. clangd
> > trips quite badly when trying to build tree_nocb.h to generate ASTs.
>
> Hi Joel,
> Thanks for the report. What are you using clangd for? I'll bet
> something interesting.
>
> I've never used it myself, so I don't know where to even begin with
> how to reproduce the issue.
>
> It might be worth filing a bug upstream at
> https://github.com/llvm/llvm-project/issues
> or internally under the component
> Language Platforms > C++ > Clang > Tools > Clangd
> with detailed steps to reproduce (and what the observed error actually
> is). Feel free to cc me, though I don't know the first thing about
> clangd.
>
> >
> > I get something like this in the clangd logs showing it 'infers' how to build
> > tree_nocb.h because it could not find a command in compile_commands.json:
> >
> > ASTWorker building file [..]/tree_nocb.h version 9 with command inferred from
> > [..]/kernel/rcu/tree.c
> >
> > This leads to all hell breaking lose with complaints about missing rcu_data
> > struct definition and so forth.
> >
> > So far I came up with a workaround as follows, but is there a better way?
> >
> > 1. Open compile_commands.json and add a new entry as follows, with a
> > definition "-DNOCB_H_CLANGD_PARSE". Otherwise the entry is indentical to how
> > tree.c is built.
> >
> > {
> > "arguments": [
> > "/usr/bin/clang",
> > "-Wp,-MMD,kernel/rcu/.treenocb.o.d",
> > "-nostdinc",
> > "-I./arch/x86/include",
> > "-I./arch/x86/include/generated",
> > "-I./include",
> > "-I./arch/x86/include/uapi",
> > [...]
> > "-Wformat-zero-length",
> > "-Wnonnull",
> > "-Wformat-insufficient-args",
> > "-Wno-sign-compare",
> > "-Wno-pointer-to-enum-cast",
> > "-Wno-tautological-constant-out-of-range-compare",
> > "-Wno-unaligned-access",
> > "-DKBUILD_MODFILE=\"kernel/rcu/tree\"",
> > "-DKBUILD_BASENAME=\"tree\"",
> > "-DKBUILD_MODNAME=\"tree\"",
> > "-D__KBUILD_MODNAME=kmod_tree",
> > "-DNOCB_H_CLANGD_PARSE",
> > "-c",
> > "-I",
> > "/s/",
> > "-I",
> > "/s/",
> > "-o",
> > "kernel/rcu/tree_nocb.h.o",
> > "kernel/rcu/tree_nocb.h"
> > ],
> > "directory": "/usr/local/google/home/joelaf/repo/linux-master",
> > "file": "/usr/local/google/home/joelaf/repo/linux-master/kernel/rcu/tree_nocb.h",
> > "output": "/usr/local/google/home/joelaf/repo/linux-master/kernel/rcu/tree_nocb.h.o"
> > },
> >
> > 2.
> > Then in kernel/tree/tree_nocb.h, I do the following right in the beginning.
> > (Thanks to paulmck@ for this idea).
> >
> > #ifdef NOCB_H_CLANGD_PARSE
> > #include "tree.c"
> > #endif
> >
> > 3. To prevent the above inclusion of tree.c from recursively including
> > tree_nocb.h, I do the following at the end of tree.c
> >
> > +#ifndef NOCB_H_CLANGD_PARSE
> > #include "tree_nocb.h"
> > -#include "tree_plugin.h"
> > +#endif
> > +#include "tree_plugin.h"
> >
> > With that it works, but if I ever generate compile_commands.json again, then
> > I'll have to again modify compile_commands.json manually to make my editor
> > work again with clangd.
> >
> > So I guess my questions are:
> >
> > 1. Is there a 'standard' procedure to solve something like this?
> >
> > 2. How do we fix this the right way?
> > One way would be for scripts/clang-tools/gen_compile_commands.py to parse
> > header files and generate suitable compile_commands.json based on
> > meta-data in the header file.
> >
> > 3. How do we fix this for other header files in general? Do we have to make hacks like
> > above (sad face) or can we come up with a standard way to make it work for kernel
> > sources?
> >
> > Thank you!
> >
> > - Joel
> >
> >
> >
> >
> >
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers
On Fri, Apr 14, 2023 at 03:47:51PM -0700, Nick Desaulniers wrote:
> On Thu, Apr 13, 2023 at 5:53 PM Joel Fernandes <[email protected]> wrote:
> >
> > Hello!
> >
> > I have been trying to get clangd working properly with tree_nocb.h. clangd
> > trips quite badly when trying to build tree_nocb.h to generate ASTs.
>
> Hi Joel,
> Thanks for the report. What are you using clangd for? I'll bet
> something interesting.
Thanks for the response and sorry for the late reply. I am at the OSPM
conference. I use vim and vscode with clangd. In vim, YCM uses it to
highlight compiler errors live while editing, I am pretty happy with it so
far and has been a huge time saver. Enough that now I want to use it for
everything...
I first came across clangd when developing Chrome userspace code which is C++
:). In Chrome, ninja builds can be made to output compile_commands.json.
However, now I noticed the support in the kernel and was like, wow I need to
try it. Further, YCM seems to work much better with it than without :)
> I've never used it myself, so I don't know where to even begin with
> how to reproduce the issue.
Ah ok. :). When I ran get_maintainer on the script, your name popped up and
someone also suggested that you're the goto person for clang on the kernel
(which I kind of already knew ;)
> It might be worth filing a bug upstream at
> https://github.com/llvm/llvm-project/issues
> or internally under the component
> Language Platforms > C++ > Clang > Tools > Clangd
> with detailed steps to reproduce (and what the observed error actually
> is). Feel free to cc me, though I don't know the first thing about
> clangd.
Ok I will consider doing this if needed. One thing I do observe is lack of
good support for header files and it is a known clangd issue [1].
However, the fixes I was proposing can purely be done in the kernel itself
since all it'd require is generating compile_compands.json with the -D<macro>
and editing files to keep clangd happy. I guess one question is, how welcome
would such changes to header files be since they're for tooling and isn't
code that will be compiled outside of clangd.
(Linked issue may not directly related to what I'm saying)
[1] https://discourse.llvm.org/t/header-file-heuristics-issue/1749
Thanks.
On Tue, Apr 18, 2023 at 1:36 PM Joel Fernandes <[email protected]> wrote:
>
> On Fri, Apr 14, 2023 at 03:47:51PM -0700, Nick Desaulniers wrote:
> > On Thu, Apr 13, 2023 at 5:53 PM Joel Fernandes <[email protected]> wrote:
> > >
> > > Hello!
> > >
> > > I have been trying to get clangd working properly with tree_nocb.h. clangd
> > > trips quite badly when trying to build tree_nocb.h to generate ASTs.
> >
> > Hi Joel,
> > Thanks for the report. What are you using clangd for? I'll bet
> > something interesting.
>
> Thanks for the response and sorry for the late reply. I am at the OSPM
> conference. I use vim and vscode with clangd. In vim, YCM uses it to
> highlight compiler errors live while editing, I am pretty happy with it so
> far and has been a huge time saver. Enough that now I want to use it for
> everything...
Cool! I use vim, can you share more info about your set up for this?
I'll have to try it.
>
> I first came across clangd when developing Chrome userspace code which is C++
> :). In Chrome, ninja builds can be made to output compile_commands.json.
> However, now I noticed the support in the kernel and was like, wow I need to
> try it. Further, YCM seems to work much better with it than without :)
>
> > I've never used it myself, so I don't know where to even begin with
> > how to reproduce the issue.
>
> Ah ok. :). When I ran get_maintainer on the script, your name popped up and
> someone also suggested that you're the goto person for clang on the kernel
> (which I kind of already knew ;)
You've cc'ed the right set of folks. We might not have the expertise
related to clangd specifically, but we can point you in the right
direction.
>
> > It might be worth filing a bug upstream at
> > https://github.com/llvm/llvm-project/issues
> > or internally under the component
> > Language Platforms > C++ > Clang > Tools > Clangd
> > with detailed steps to reproduce (and what the observed error actually
> > is). Feel free to cc me, though I don't know the first thing about
> > clangd.
>
> Ok I will consider doing this if needed. One thing I do observe is lack of
> good support for header files and it is a known clangd issue [1].
>
> However, the fixes I was proposing can purely be done in the kernel itself
> since all it'd require is generating compile_compands.json with the -D<macro>
> and editing files to keep clangd happy. I guess one question is, how welcome
> would such changes to header files be since they're for tooling and isn't
> code that will be compiled outside of clangd.
Specifically your patch sites some log print that doesn't look
indicative of a failure:
https://github.com/llvm/llvm-project/blob/53430bfd5c9d0074dd6de06dccea366e1d40bce4/clang-tools-extra/clangd/TUScheduler.cpp#L903-L906
so something else is going on here. Just trying to make sure we root cause this.
>
> (Linked issue may not directly related to what I'm saying)
> [1] https://discourse.llvm.org/t/header-file-heuristics-issue/1749
>
> Thanks.
>
--
Thanks,
~Nick Desaulniers
Hi Nick,
On Tue, Apr 18, 2023 at 01:46:40PM -0700, Nick Desaulniers wrote:
> On Tue, Apr 18, 2023 at 1:36 PM Joel Fernandes <[email protected]> wrote:
> >
> > On Fri, Apr 14, 2023 at 03:47:51PM -0700, Nick Desaulniers wrote:
> > > On Thu, Apr 13, 2023 at 5:53 PM Joel Fernandes <[email protected]> wrote:
> > > >
> > > > Hello!
> > > >
> > > > I have been trying to get clangd working properly with tree_nocb.h. clangd
> > > > trips quite badly when trying to build tree_nocb.h to generate ASTs.
> > >
> > > Hi Joel,
> > > Thanks for the report. What are you using clangd for? I'll bet
> > > something interesting.
> >
> > Thanks for the response and sorry for the late reply. I am at the OSPM
> > conference. I use vim and vscode with clangd. In vim, YCM uses it to
> > highlight compiler errors live while editing, I am pretty happy with it so
> > far and has been a huge time saver. Enough that now I want to use it for
> > everything...
>
> Cool! I use vim, can you share more info about your set up for this?
> I'll have to try it.
This is how I installed YCM:
# Install YouCompleteMe for vim
# cd ~/.vim/bundle
# git clone https://github.com/Valloric/YouCompleteMe.git
# cd YouCompleteMe/
# git submodule update --init --recursive
# python3 install.py --clang-completer
Then install and run bear in the kernel sources to generate
compile_compands.json:
bear -- make -j99 CC=clang
However, there's also the script:
scripts/clang-tools/gen_compile_commands.py
This generates the .json from an existing build. Thank God because we can
probably make this generate better .json files which may make clangd better.
You don't need YCM to reproduce the issue though if you just use vscode with
the clangd plugin.
> > I first came across clangd when developing Chrome userspace code which is C++
> > :). In Chrome, ninja builds can be made to output compile_commands.json.
> > However, now I noticed the support in the kernel and was like, wow I need to
> > try it. Further, YCM seems to work much better with it than without :)
> >
> > > I've never used it myself, so I don't know where to even begin with
> > > how to reproduce the issue.
> >
> > Ah ok. :). When I ran get_maintainer on the script, your name popped up and
> > someone also suggested that you're the goto person for clang on the kernel
> > (which I kind of already knew ;)
>
> You've cc'ed the right set of folks. We might not have the expertise
> related to clangd specifically, but we can point you in the right
> direction.
Sure, thanks! And thanks for CC'ing the right folks.
> > > It might be worth filing a bug upstream at
> > > https://github.com/llvm/llvm-project/issues
> > > or internally under the component
> > > Language Platforms > C++ > Clang > Tools > Clangd
> > > with detailed steps to reproduce (and what the observed error actually
> > > is). Feel free to cc me, though I don't know the first thing about
> > > clangd.
> >
> > Ok I will consider doing this if needed. One thing I do observe is lack of
> > good support for header files and it is a known clangd issue [1].
> >
> > However, the fixes I was proposing can purely be done in the kernel itself
> > since all it'd require is generating compile_compands.json with the -D<macro>
> > and editing files to keep clangd happy. I guess one question is, how welcome
> > would such changes to header files be since they're for tooling and isn't
> > code that will be compiled outside of clangd.
>
> Specifically your patch sites some log print that doesn't look
> indicative of a failure:
> https://github.com/llvm/llvm-project/blob/53430bfd5c9d0074dd6de06dccea366e1d40bce4/clang-tools-extra/clangd/TUScheduler.cpp#L903-L906
> so something else is going on here. Just trying to make sure we root cause this.
>
Right, so clangd's log does not show failure, the failure is when it displays
in the code editor that your code has compiler errors when in fact it does
not.
So if you open up tree_nocb.h in vscode, for example, you'll see squiggles
saying 'undefined reference to rcu_data', etc. That makes clangd stop working
at the error. Sorry to not make the failure mode clear earlier.
thanks,
- Joel
Adding Florent as well since Nick added him to the other thread.
Thanks.
> On Apr 18, 2023, at 11:03 PM, Joel Fernandes <[email protected]> wrote:
>
> Hi Nick,
>
>> On Tue, Apr 18, 2023 at 01:46:40PM -0700, Nick Desaulniers wrote:
>>> On Tue, Apr 18, 2023 at 1:36 PM Joel Fernandes <[email protected]> wrote:
>>>
>>> On Fri, Apr 14, 2023 at 03:47:51PM -0700, Nick Desaulniers wrote:
>>>> On Thu, Apr 13, 2023 at 5:53 PM Joel Fernandes <[email protected]> wrote:
>>>>>
>>>>> Hello!
>>>>>
>>>>> I have been trying to get clangd working properly with tree_nocb.h. clangd
>>>>> trips quite badly when trying to build tree_nocb.h to generate ASTs.
>>>>
>>>> Hi Joel,
>>>> Thanks for the report. What are you using clangd for? I'll bet
>>>> something interesting.
>>>
>>> Thanks for the response and sorry for the late reply. I am at the OSPM
>>> conference. I use vim and vscode with clangd. In vim, YCM uses it to
>>> highlight compiler errors live while editing, I am pretty happy with it so
>>> far and has been a huge time saver. Enough that now I want to use it for
>>> everything...
>>
>> Cool! I use vim, can you share more info about your set up for this?
>> I'll have to try it.
>
> This is how I installed YCM:
>
> # Install YouCompleteMe for vim
> # cd ~/.vim/bundle
> # git clone https://github.com/Valloric/YouCompleteMe.git
> # cd YouCompleteMe/
> # git submodule update --init --recursive
> # python3 install.py --clang-completer
>
> Then install and run bear in the kernel sources to generate
> compile_compands.json:
> bear -- make -j99 CC=clang
>
> However, there's also the script:
> scripts/clang-tools/gen_compile_commands.py
>
> This generates the .json from an existing build. Thank God because we can
> probably make this generate better .json files which may make clangd better.
>
> You don't need YCM to reproduce the issue though if you just use vscode with
> the clangd plugin.
>
>>> I first came across clangd when developing Chrome userspace code which is C++
>>> :). In Chrome, ninja builds can be made to output compile_commands.json.
>>> However, now I noticed the support in the kernel and was like, wow I need to
>>> try it. Further, YCM seems to work much better with it than without :)
>>>
>>>> I've never used it myself, so I don't know where to even begin with
>>>> how to reproduce the issue.
>>>
>>> Ah ok. :). When I ran get_maintainer on the script, your name popped up and
>>> someone also suggested that you're the goto person for clang on the kernel
>>> (which I kind of already knew ;)
>>
>> You've cc'ed the right set of folks. We might not have the expertise
>> related to clangd specifically, but we can point you in the right
>> direction.
>
> Sure, thanks! And thanks for CC'ing the right folks.
>
>>>> It might be worth filing a bug upstream at
>>>> https://github.com/llvm/llvm-project/issues
>>>> or internally under the component
>>>> Language Platforms > C++ > Clang > Tools > Clangd
>>>> with detailed steps to reproduce (and what the observed error actually
>>>> is). Feel free to cc me, though I don't know the first thing about
>>>> clangd.
>>>
>>> Ok I will consider doing this if needed. One thing I do observe is lack of
>>> good support for header files and it is a known clangd issue [1].
>>>
>>> However, the fixes I was proposing can purely be done in the kernel itself
>>> since all it'd require is generating compile_compands.json with the -D<macro>
>>> and editing files to keep clangd happy. I guess one question is, how welcome
>>> would such changes to header files be since they're for tooling and isn't
>>> code that will be compiled outside of clangd.
>>
>> Specifically your patch sites some log print that doesn't look
>> indicative of a failure:
>> https://github.com/llvm/llvm-project/blob/53430bfd5c9d0074dd6de06dccea366e1d40bce4/clang-tools-extra/clangd/TUScheduler.cpp#L903-L906
>> so something else is going on here. Just trying to make sure we root cause this.
>>
>
> Right, so clangd's log does not show failure, the failure is when it displays
> in the code editor that your code has compiler errors when in fact it does
> not.
>
> So if you open up tree_nocb.h in vscode, for example, you'll see squiggles
> saying 'undefined reference to rcu_data', etc. That makes clangd stop working
> at the error. Sorry to not make the failure mode clear earlier.
>
> thanks,
>
> - Joel
>
On Tue, Apr 18, 2023 at 6:28 PM Nick Desaulniers
<[email protected]> wrote:
>
> + Florent
Hi there!
> Joel, Florent is doing some cool stuff with clangd and kernels (check
> out the demo at go/linux-kernel-vscode). I'm pushing Florent to
Apologies for folks outside Google, this is an internal link to a
kernel dev setup I originally created for myself, then for my team and
apparently more and more people are starting to use it internally. :)
If there's enough appetite for it externally too, I'll try to
open-source it someday. Anyway, in the context of this conversation,
it's just something that uses clangd. :)
> publish some of the cool stuff he's been working on externally because
> it is awesome. ;)
>
> Florent, have you seen any such issues such as what Joel reported below?
Yes, I've seen this problem a bunch of times. Afaiu, Clangd operates
under the assumption that every source file is a valid compilation
unit. My understanding is that it's generally a good practice to keep
things that way and I wouldn't be surprised if the userspace Chrome
code-base Joel saw enforces this (iirc, it's a rule for
Google-internal C++ too, all headers must be interpretable
independently).
However, from the perspective of the C spec, anything can be included
anywhere and a C file can only make sense if it's included
after/before certain other things are defined/included. Spontaneously,
I would call these ".inc" rather than ".h" or ".c" because I would
expect a source file to be always valid and this suffix makes it
clearer they depend on their context, but as a matter of fact source
files that don't compile when interpreted individually are quite
common in the kernel tree. Other examples that have been reported to
me include a lot of kernel/sched/*, since many of these files
(including .c files) are included from kernel/sched/build_policy.c in
a specific order to form one big compilation unit.
Unfortunately, I don't know of any solution. :( This feels like a
limit of C or compile_commands.json to me. "compile commands" can not
be enough to interpret any file, clangd would need a way to express
"interpret this file as if it were included in that spot of that
compilation unit" and maybe even need a bunch of heuristics to choose
one such include spot.
I don't know if clangd has any plan to address this and so far I've
just lived with these error squiggles.
On Wed, Apr 19, 2023 at 6:00 AM Florent Revest <[email protected]> wrote:
>
> On Tue, Apr 18, 2023 at 6:28 PM Nick Desaulniers
> <[email protected]> wrote:
> >
> > + Florent
>
> Hi there!
>
> > Joel, Florent is doing some cool stuff with clangd and kernels (check
> > out the demo at go/linux-kernel-vscode). I'm pushing Florent to
>
> Apologies for folks outside Google, this is an internal link to a
> kernel dev setup I originally created for myself, then for my team and
> apparently more and more people are starting to use it internally. :)
> If there's enough appetite for it externally too, I'll try to
> open-source it someday. Anyway, in the context of this conversation,
> it's just something that uses clangd. :)
>
> > publish some of the cool stuff he's been working on externally because
> > it is awesome. ;)
> >
> > Florent, have you seen any such issues such as what Joel reported below?
>
> Yes, I've seen this problem a bunch of times. Afaiu, Clangd operates
> under the assumption that every source file is a valid compilation
> unit. My understanding is that it's generally a good practice to keep
> things that way and I wouldn't be surprised if the userspace Chrome
> code-base Joel saw enforces this (iirc, it's a rule for
> Google-internal C++ too, all headers must be interpretable
> independently).
>
> However, from the perspective of the C spec, anything can be included
> anywhere and a C file can only make sense if it's included
> after/before certain other things are defined/included. Spontaneously,
> I would call these ".inc" rather than ".h" or ".c" because I would
> expect a source file to be always valid and this suffix makes it
> clearer they depend on their context, but as a matter of fact source
> files that don't compile when interpreted individually are quite
> common in the kernel tree. Other examples that have been reported to
> me include a lot of kernel/sched/*, since many of these files
> (including .c files) are included from kernel/sched/build_policy.c in
> a specific order to form one big compilation unit.
>
> Unfortunately, I don't know of any solution. :( This feels like a
> limit of C or compile_commands.json to me. "compile commands" can not
> be enough to interpret any file, clangd would need a way to express
> "interpret this file as if it were included in that spot of that
> compilation unit" and maybe even need a bunch of heuristics to choose
> one such include spot.
>
> I don't know if clangd has any plan to address this and so far I've
> just lived with these error squiggles.
>
Some information about Clang based language servers
It's good practice to ensure that a header file is self-contained. If
not, for example, if a.c includes a.h, which is not self-contained,
a.h is generally compiled on its own (as if using clang [options] a.h)
to confine diagnostics/completion results to a.h and not to other
headers included by a.c.
However, this design choice may cause language servers to emit
diagnostics that you don't see when building the project.
For my tool ccls, the index file for an included file (e.g. a.h) is
computed when compiling the main source file (a.c). Therefore, if the
project builds successfully, the index is always accurate.
Language servers usually have the limitation that only one
configuration of a main source file is recorded, e.g., you only get
index data for either clang -DA=1 a.c or clang -DA=2 a.c, not both.
This limitation exists due to technical challenges and practicality.
If a main source file has 10 configurations, when you edit the file,
do you compile it 10 times to get all indexes?
---
My setup is like this. I have been doing it since about 2019, though I
rarely read Linux kernel code :(
Ensure you have the following Clang and LLVM tools. If you build them
by yourself, create a llvm-project build directory, say, /tmp/Rel
```
ninja -C /tmp/Rel clang lld
llvm-{ar,nm,objcopy,objdump,ranlib,readelf,strings,strip}
```
In a Linux kernel source directory,
(https://github.com/MaskRay/ccls/wiki/Example-Projects)
```
PATH=/tmp/Rel/bin:$PATH make O=/tmp/out/x86_64 ARCH=x86_64 LLVM=1 defconfig
PATH=/tmp/Rel/bin:$PATH make O=/tmp/out/x86_64 ARCH=x86_64 LLVM=1 -k
bzImage modules # generate .<target>.cmd files
scripts/clang-tools/gen_compile_commands.py -d /tmp/out/x86_64
```
emacs -nw fs/binfmt_elf.c # check out lsp-mode and eglot
For neovim, use https://github.com/neoclide/coc.nvim or the built-in
client. I prefer coc.nvim.
--
宋方睿
On Tue, Apr 18, 2023 at 2:03 PM Joel Fernandes <[email protected]> wrote:
>
> Hi Nick,
>
> On Tue, Apr 18, 2023 at 01:46:40PM -0700, Nick Desaulniers wrote:
> > On Tue, Apr 18, 2023 at 1:36 PM Joel Fernandes <[email protected]> wrote:
> > >
> > > On Fri, Apr 14, 2023 at 03:47:51PM -0700, Nick Desaulniers wrote:
> > > > On Thu, Apr 13, 2023 at 5:53 PM Joel Fernandes <[email protected]> wrote:
> > > > >
> > > > > Hello!
> > > > >
> > > > > I have been trying to get clangd working properly with tree_nocb.h. clangd
> > > > > trips quite badly when trying to build tree_nocb.h to generate ASTs.
> > > >
> > > > Hi Joel,
> > > > Thanks for the report. What are you using clangd for? I'll bet
> > > > something interesting.
> > >
> > > Thanks for the response and sorry for the late reply. I am at the OSPM
> > > conference. I use vim and vscode with clangd. In vim, YCM uses it to
> > > highlight compiler errors live while editing, I am pretty happy with it so
> > > far and has been a huge time saver. Enough that now I want to use it for
> > > everything...
> >
> > Cool! I use vim, can you share more info about your set up for this?
> > I'll have to try it.
>
> This is how I installed YCM:
>
> # Install YouCompleteMe for vim
> # cd ~/.vim/bundle
> # git clone https://github.com/Valloric/YouCompleteMe.git
> # cd YouCompleteMe/
> # git submodule update --init --recursive
> # python3 install.py --clang-completer
Thanks, I needed to add this
set runtimepath+=~/.vim/bundle/YouCompleteMe/
to my ~/.vimrc as well. Via
https://stackoverflow.com/q/60797142
so not sure I fully installed YCM since I didn't see `runtimepath` in
https://github.com/ycm-core/YouCompleteMe, but it's working now for me! Cool!
>
> Then install and run bear in the kernel sources to generate
> compile_compands.json:
> bear -- make -j99 CC=clang
>
> However, there's also the script:
> scripts/clang-tools/gen_compile_commands.py
>
> This generates the .json from an existing build. Thank God because we can
> probably make this generate better .json files which may make clangd better.
So we have a compile_commands.json make target. All I did was:
$ make LLVM=1 -j128 compile_commands.json
it invokes that python script you found which parses the *.o.cmd files
produced from a build. Unfortunately, that requires a completed
build, and IIRC I think `make clean` deletes compile_commands.json. I
think we might only want to do that for mrproper...
>
> You don't need YCM to reproduce the issue though if you just use vscode with
> the clangd plugin.
>
> > > I first came across clangd when developing Chrome userspace code which is C++
> > > :). In Chrome, ninja builds can be made to output compile_commands.json.
> > > However, now I noticed the support in the kernel and was like, wow I need to
> > > try it. Further, YCM seems to work much better with it than without :)
> > >
> > > > I've never used it myself, so I don't know where to even begin with
> > > > how to reproduce the issue.
> > >
> > > Ah ok. :). When I ran get_maintainer on the script, your name popped up and
> > > someone also suggested that you're the goto person for clang on the kernel
> > > (which I kind of already knew ;)
> >
> > You've cc'ed the right set of folks. We might not have the expertise
> > related to clangd specifically, but we can point you in the right
> > direction.
>
> Sure, thanks! And thanks for CC'ing the right folks.
>
> > > > It might be worth filing a bug upstream at
> > > > https://github.com/llvm/llvm-project/issues
> > > > or internally under the component
> > > > Language Platforms > C++ > Clang > Tools > Clangd
> > > > with detailed steps to reproduce (and what the observed error actually
> > > > is). Feel free to cc me, though I don't know the first thing about
> > > > clangd.
> > >
> > > Ok I will consider doing this if needed. One thing I do observe is lack of
> > > good support for header files and it is a known clangd issue [1].
> > >
> > > However, the fixes I was proposing can purely be done in the kernel itself
> > > since all it'd require is generating compile_compands.json with the -D<macro>
> > > and editing files to keep clangd happy. I guess one question is, how welcome
> > > would such changes to header files be since they're for tooling and isn't
> > > code that will be compiled outside of clangd.
> >
> > Specifically your patch sites some log print that doesn't look
> > indicative of a failure:
> > https://github.com/llvm/llvm-project/blob/53430bfd5c9d0074dd6de06dccea366e1d40bce4/clang-tools-extra/clangd/TUScheduler.cpp#L903-L906
> > so something else is going on here. Just trying to make sure we root cause this.
> >
>
> Right, so clangd's log does not show failure, the failure is when it displays
> in the code editor that your code has compiler errors when in fact it does
> not.
>
> So if you open up tree_nocb.h in vscode, for example, you'll see squiggles
> saying 'undefined reference to rcu_data', etc. That makes clangd stop working
> at the error. Sorry to not make the failure mode clear earlier.
In vim, the very first line is highlighted with:
Too many errors emitted, stopping now [fatal_too_many_errors]
I guess that disables nice checking that's done in .c files?
--
Thanks,
~Nick Desaulniers
From: Fangrui Song
> Sent: 19 April 2023 21:02
....
> Some information about Clang based language servers
>
> It's good practice to ensure that a header file is self-contained. If
> not, for example, if a.c includes a.h, which is not self-contained,
> a.h is generally compiled on its own (as if using clang [options] a.h)
> to confine diagnostics/completion results to a.h and not to other
> headers included by a.c.
>
> However, this design choice may cause language servers to emit
> diagnostics that you don't see when building the project.
That really doesn't work for .h files that contain program
generated data used to initialise arrays.
They may not even contain numbers, just references to
#defines that that including file is expected to define.
So there will always be .h files that are not compilable.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Fangrui/Florent/David/All:
Sorry for the late reply, I got distracted.
On Wed, Apr 19, 2023 at 4:02 PM Fangrui Song <[email protected]> wrote:
>
> On Wed, Apr 19, 2023 at 6:00 AM Florent Revest <[email protected]> wrote:
> >
> > On Tue, Apr 18, 2023 at 6:28 PM Nick Desaulniers
> > <[email protected]> wrote:
> > >
> > > + Florent
> >
> > Hi there!
> >
> > > Joel, Florent is doing some cool stuff with clangd and kernels (check
> > > out the demo at go/linux-kernel-vscode). I'm pushing Florent to
> >
> > Apologies for folks outside Google, this is an internal link to a
> > kernel dev setup I originally created for myself, then for my team and
> > apparently more and more people are starting to use it internally. :)
> > If there's enough appetite for it externally too, I'll try to
> > open-source it someday. Anyway, in the context of this conversation,
> > it's just something that uses clangd. :)
> >
> > > publish some of the cool stuff he's been working on externally because
> > > it is awesome. ;)
> > >
> > > Florent, have you seen any such issues such as what Joel reported below?
> >
> > Yes, I've seen this problem a bunch of times. Afaiu, Clangd operates
> > under the assumption that every source file is a valid compilation
> > unit. My understanding is that it's generally a good practice to keep
> > things that way and I wouldn't be surprised if the userspace Chrome
> > code-base Joel saw enforces this (iirc, it's a rule for
> > Google-internal C++ too, all headers must be interpretable
> > independently).
> >
> > However, from the perspective of the C spec, anything can be included
> > anywhere and a C file can only make sense if it's included
> > after/before certain other things are defined/included. Spontaneously,
> > I would call these ".inc" rather than ".h" or ".c" because I would
> > expect a source file to be always valid and this suffix makes it
> > clearer they depend on their context, but as a matter of fact source
> > files that don't compile when interpreted individually are quite
> > common in the kernel tree. Other examples that have been reported to
> > me include a lot of kernel/sched/*, since many of these files
> > (including .c files) are included from kernel/sched/build_policy.c in
> > a specific order to form one big compilation unit.
> >
> > Unfortunately, I don't know of any solution. :( This feels like a
> > limit of C or compile_commands.json to me. "compile commands" can not
> > be enough to interpret any file, clangd would need a way to express
> > "interpret this file as if it were included in that spot of that
> > compilation unit" and maybe even need a bunch of heuristics to choose
> > one such include spot.
> >
> > I don't know if clangd has any plan to address this and so far I've
> > just lived with these error squiggles.
> >
>
> Some information about Clang based language servers
>
> It's good practice to ensure that a header file is self-contained. If
> not, for example, if a.c includes a.h, which is not self-contained,
> a.h is generally compiled on its own (as if using clang [options] a.h)
> to confine diagnostics/completion results to a.h and not to other
> headers included by a.c.
>
> However, this design choice may cause language servers to emit
> diagnostics that you don't see when building the project.
>
> For my tool ccls, the index file for an included file (e.g. a.h) is
> computed when compiling the main source file (a.c). Therefore, if the
> project builds successfully, the index is always accurate.
>
> Language servers usually have the limitation that only one
> configuration of a main source file is recorded, e.g., you only get
> index data for either clang -DA=1 a.c or clang -DA=2 a.c, not both.
> This limitation exists due to technical challenges and practicality.
> If a main source file has 10 configurations, when you edit the file,
> do you compile it 10 times to get all indexes?
Indeed regarding the 10 configs points you brought up. The .h not
being 1:1 with compilation units makes it a similar problem.
For example, if I edit a.h and a.h is included in both b.c and c.c.
Then, in order to know if edits I am making to a.h really will not
lead to build errors, clangd has to compile both b.c and c.c
separately. Why?
Because a.h's inclusion in b.c may build successfully, but in c.c may
not! This generalizes to any number of source files.
However, I was thinking of a scheme such as the following:
Have a file class Clangd.yaml in the source directory of a .h. In this
file, clearly state how to build the .h. For the above example, for
changes to a.h -- the YAML will say include a.h and then b.c , and
then build the combined thing. Then a script like the one building
compile_commands.json can use the YAML to generate something useful
for the .h That may not work for every usecase such as the one David
pointed out, but at least it will be somewhat useful for most use
cases -- versus what we currently do for the issue (nothing).
But even if you edit the .h, the compiler errors typically end up
showing up in the .c files. So even clangd says, there is a build
error in the combined result -- can it really pinpoint where in the .h
is the issue? My observation is clangd will point errors within the
thing that is being compiled (i.e. the .c file) and not the include
file.
Other Thoughts? On the process side of things, is there a place I can
file a bug report where some of this may be interesting to a team
working on this?
- Joel