HighCommander4 wrote:

Okay, so clangd's processing of a source file involves remapping the path of 
the file to a in-memory buffer (this is so that clangd can parse a version of 
the file that's not yet saved to disk but provided by the client in the 
`didOpen` notification, and also so that clangd can build a "preamble" from a 
subset of the file's contents). This involves calling 
`FileManager::getVirtualFileRef()`, which seemingly can't fail, but rather 
asserts when given a directory path as input, i.e. has an implicit precondition 
that its input is not a directory path. No one checks this precondition in this 
scenario, so we trigger the assertion failure.

I looked at what happens when clang is run on the command line with a directory 
as input and flags similar to what clangd uses. In that case, we get an error 
diagnostic:

```console
$ clang -x-c++-header -- /
error: error reading '/': Is a directory
```

This error is produced 
[here](https://searchfox.org/llvm/rev/3f1386b9861758beb7a9ba15c24bdba3a9f8c03d/clang/lib/Frontend/CompilerInstance.cpp#936-947),
 following a call to `FileManager::getFileRef()`, which **can** fail and does 
in this case, and the caller checks the error appropriately and turns it into a 
diagnostic.

I think a proper fix here would be to make `FileEntry::getVirtualFileRef()` 
fallible, like `getFileRef()` is, and turn its failure in this case into a 
diagnostic as well.

But that change is a bit in the weeds of FileManager / libFrontend code, and I 
don't have enough familiarity with this code to take this on.

So I'd say for now, having a check in clangd is fine, and `didOpen` is an 
appropriate place to put that check.

https://github.com/llvm/llvm-project/pull/177834
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to