
<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hi!<br class=""><br class="">A short background: we at JetBrains are experimenting with using clangd with<br class="">CLion, and we're very happy with the results so far. Unfortunately, we've hit<br class="">a problem when header files are locked on Windows, so they can't be saved,<br class="">they break `git rebase` (leading to the loss of work), etc.<br class=""><br class="">It happens when the file is opened using a memory mapped file. It's fairly<br class="">easy to reproduce with clangd:<br class=""><br class="">1. Create a header big enough to pass the threshold in `shouldUseMmap()`<br class="">(lib/Support/MemoryBuffer.cpp)<br class="">2. Include it in a source file<br class="">3. Keep this source file opened via clangd<br class="">4. Now an attempt to modify this header will fail<br class=""><br class="">The situation is especially unpleasant because the handle is locked not only<br class="">during the parse, but for all the time clangd holds the preamble containing<br class="">this header.<br class=""><br class="">This issue is mitigated to an extent in libclang: non-system files are<br class="">considered "volatile" (see r160074, r334070), so memory mapped files are not<br class="">used for them. However, I feel like it's not enough: you can easily have a<br class="">`#pragma system_header` in your codebase (IIUC it affects the mentioned<br class="">behavior), locking such file and i.e. losing user's work during `git rebase`<br class="">is still unacceptable.<br class=""><br class="">Also, I think the fact that the proper compiler has this behavior is also<br class="">unfortunate (forgotten build in the background might lead to the loss of data).<br class=""><br class="">IIUC the main reason for having memory mapped files is to reduce the memory<br class="">footprint. However, for a regular translation unit, headers usually take several<br class="">megabytes, which IMO is tolerable, and is usually topped by other per-TU data<br class="">anyway.<br class=""><br class="">Hence, what do you think about not using memory mapped files on Windows at all<br class="">for source files? Are there any implications that I'm not aware of?<br class=""><br class="">The naive patch (obviously not for commit, just for the illustration):<br class=""><br class="">--- a/lib/Basic/SourceManager.cpp<br class="">+++ b/lib/Basic/SourceManager.cpp<br class="">@@ -109,7 +109,12 @@ llvm::MemoryBuffer *ContentCache::getBuffer(DiagnosticsEngine &Diag,<br class="">return Buffer.getPointer();<br class="">}<br class=""><br class="">+#ifndef _WIN32<br class="">bool isVolatile = SM.userFilesAreVolatile() && !IsSystemFile;<br class="">+#else<br class="">+ bool isVolatile = true;<br class="">+#endif<br class="">+<br class="">auto BufferOrError =<br class="">SM.getFileManager().getBufferForFile(ContentsEntry, isVolatile);<br class=""><br class="">If I've not missed something, this would effectively disable memory mapped<br class="">files for source files on Windows, but they would still be used for<br class="">other potentially large files (including PCHs and preambles).<br class=""><br class="">If this is unacceptable, at the very least, clangd should treat user files<br class="">similar to libclang (we can work on a patch in this case).<br class=""><br class="">Note that there is a similar bug report for QtCreator/libclang with some<br class="">interesting comments: <a href="https://bugreports.qt.io/browse/QTCREATORBUG-15449" class="">https://bugreports.qt.io/browse/QTCREATORBUG-15449</a><br class=""><br class="">Cheers,<br class="">Dmitry.</body></html>