kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/SystemIncludeExtractor.cpp:340
+ // is not installed.
+ if (Lang == "objective-c++-header") {
+ Lang = "c++-header";
----------------
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).
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
