[cfe-dev] Locking files on Windows with clangd
Ilya Biryukov via cfe-dev
cfe-dev at lists.llvm.org
Tue Jul 24 00:13:47 PDT 2018
We've had a pull request with a similar fix at some point:
https://reviews.llvm.org/D35200, but it was abandoned and never made it in.
@ivan.donchevskii at qt.io <ivan.donchevskii at qt.io>, did you manage to figure
out a way to make this work on Windows? Any ideas on what's the best way to
On Sun, Jul 22, 2018 at 7:22 PM Dmitry.Kozhevnikov at jetbrains.com via
cfe-dev <cfe-dev at lists.llvm.org> wrote:
> A short background: we at JetBrains are experimenting with using clangd
> CLion, and we're very happy with the results so far. Unfortunately, we've
> a problem when header files are locked on Windows, so they can't be saved,
> they break `git rebase` (leading to the loss of work), etc.
> It happens when the file is opened using a memory mapped file. It's fairly
> easy to reproduce with clangd:
> 1. Create a header big enough to pass the threshold in `shouldUseMmap()`
> 2. Include it in a source file
> 3. Keep this source file opened via clangd
> 4. Now an attempt to modify this header will fail
> The situation is especially unpleasant because the handle is locked not
> during the parse, but for all the time clangd holds the preamble containing
> this header.
> This issue is mitigated to an extent in libclang: non-system files are
> considered "volatile" (see r160074, r334070), so memory mapped files are
> used for them. However, I feel like it's not enough: you can easily have a
> `#pragma system_header` in your codebase (IIUC it affects the mentioned
> behavior), locking such file and i.e. losing user's work during `git
> is still unacceptable.
> Also, I think the fact that the proper compiler has this behavior is also
> unfortunate (forgotten build in the background might lead to the loss of
> IIUC the main reason for having memory mapped files is to reduce the memory
> footprint. However, for a regular translation unit, headers usually take
> megabytes, which IMO is tolerable, and is usually topped by other per-TU
> Hence, what do you think about not using memory mapped files on Windows at
> for source files? Are there any implications that I'm not aware of?
> The naive patch (obviously not for commit, just for the illustration):
> --- a/lib/Basic/SourceManager.cpp
> +++ b/lib/Basic/SourceManager.cpp
> @@ -109,7 +109,12 @@ llvm::MemoryBuffer
> *ContentCache::getBuffer(DiagnosticsEngine &Diag,
> return Buffer.getPointer();
> +#ifndef _WIN32
> bool isVolatile = SM.userFilesAreVolatile() && !IsSystemFile;
> + bool isVolatile = true;
> auto BufferOrError =
> SM.getFileManager().getBufferForFile(ContentsEntry, isVolatile);
> If I've not missed something, this would effectively disable memory mapped
> files for source files on Windows, but they would still be used for
> other potentially large files (including PCHs and preambles).
> If this is unacceptable, at the very least, clangd should treat user files
> similar to libclang (we can work on a patch in this case).
> Note that there is a similar bug report for QtCreator/libclang with some
> interesting comments: https://bugreports.qt.io/browse/QTCREATORBUG-15449
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-dev