nridge added a subscriber: dgoldman.
nridge added inline comments.
================
Comment at: clang-tools-extra/clangd/SystemIncludeExtractor.cpp:340
+ // is not installed.
+ if (Lang == "objective-c++-header") {
+ Lang = "c++-header";
----------------
kadircet wrote:
> nridge wrote:
> > kadircet wrote:
> > > this feels like too much of a layering violation and might (will?) go
> > > wrong in cases where language was explicitly set to
> > > `objective-c++-header`.
> > >
> > > if the user is relying on fallback commands with an overwrite of
> > > `Compiler:` in the config && --query-driver globs, would it be too much
> > > of a hassle to expect them to have a `CompileFlags: Add: ...` block too?
> > > this feels like too much of a layering violation and might (will?) go
> > > wrong in cases where language was explicitly set to
> > > `objective-c++-header`.
> >
> > This has occurred to me, and my first idea for a fix was to limit this
> > change to cases where the `-xobjective-c++-header` originates from the
> > fallback command.
> >
> > However, as mentioned
> > [here](https://github.com/clangd/clangd/issues/1568#issuecomment-1493236437),
> > when I tested this I found that `-xobjective-c++-header` did not make any
> > difference (compared to `-xc++-header` or `-xc++`) in the include paths
> > returned by gcc. In other words, in gcc's include directory structure there
> > are no objc-specific directories. This made me think this simpler fix would
> > be appropriate.
> >
> > > if the user is relying on fallback commands with an overwrite of
> > > `Compiler:` in the config && --query-driver globs, would it be too much
> > > of a hassle to expect them to have a `CompileFlags: Add: ...` block too?
> >
> > You're right, adding a section like this to the config does seem to be a
> > viable workaround:
> >
> > ```
> > ---
> >
> > If:
> > PathMatch: *\.h
> >
> > CompileFlags:
> > Add: [-xc++-header]
> > ```
> >
> > But I think it would still be nice to fix this in clangd, as being foiled
> > by objective-c support not being installed is a very unexpected failure
> > mode for a user whose project does not involve objective-c at all.
> >
> > For what it's worth, I don't think this kind of setup is uncommon. A common
> > scenario seems to be a casual user playing around with a small project
> > (hence, doesn't have a build system or compile_commands.json), on a
> > platform where --query-driver is needed to find the standard library
> > headers (most commonly, MinGW on Windows).
> > However, as mentioned
> > [here](https://github.com/clangd/clangd/issues/1568#issuecomment-1493236437),
> > when I tested this I found that `-xobjective-c++-header` did not make any
> > difference (compared to `-xc++-header` or `-xc++`) in the include paths
> > returned by gcc. In other words, in gcc's include directory structure there
> > are no objc-specific directories.
>
> Well, that's definitely re-assuring, but I am not sure if it's enough to say
> it'll work that way with all gcc's or when there are other/certain "system"
> libraries installed. As in theory objc compilation should at least add some
> framework search paths and what not by default, no?
>
> > But I think it would still be nice to fix this in clangd, as being foiled
> > by objective-c support not being installed is a very unexpected failure
> > mode for a user whose project does not involve objective-c at all.
>
> Completely agree, but we're only showing that to people that already fiddled
> with clangd internals. So I don't think that as unacceptable.
>
> > For what it's worth, I don't think this kind of setup is uncommon. A common
> > scenario seems to be a casual user playing around with a small project
> > (hence, doesn't have a build system or compile_commands.json), on a
> > platform where --query-driver is needed to find the standard library
> > headers (most commonly, MinGW on Windows).
>
> I think instead of trying to make things work with query-driver in such
> setups, we should try to make sure things work out-of-the-box in mingw (and
> other toolchain) setups. I believe people not using query-driver in such
> vanilla installation is way more common than people using query-driver and
> `CompileFlags.Compiler` override. Also this will probably make sure other
> clang-tools can work with those setups too.
> We have mingw toolchain detection
> [here](https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/MinGW.cpp).
> > However, as mentioned
> > [here](https://github.com/clangd/clangd/issues/1568#issuecomment-1493236437),
> > when I tested this I found that `-xobjective-c++-header` did not make any
> > difference (compared to `-xc++-header` or `-xc++`) in the include paths
> > returned by gcc. In other words, in gcc's include directory structure there
> > are no objc-specific directories.
>
> Well, that's definitely re-assuring, but I am not sure if it's enough to say
> it'll work that way with all gcc's or when there are other/certain "system"
> libraries installed. As in theory objc compilation should at least add some
> framework search paths and what not by default, no?
To be honest, I don't know enough about objective-c to say either way.
Perhaps @dgoldman can help us answer this question: would you expect the `-x
objective-c++` flag to cause the compiler to use any additional / objective-c
specific built-in include directories (compared to `-x c++`), for any compiler
you're aware of that has a gcc-compatible driver syntax?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147905/new/
https://reviews.llvm.org/D147905
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
