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:
> 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).


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

Reply via email to