[PATCH] D54995: [MemoryBuffer] By default assume that all files are volatile to prevent unintended file locks

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 30 12:03:10 PST 2018


ilya-biryukov added a comment.

In D54995#1315024 <https://reviews.llvm.org/D54995#1315024>, @zturner wrote:

> We can work around it by reading the whole file, but it looks like a bug in QtCreator to me.  We have the file mapped, but maybe when they open the file to save it, *they* are opening the file without `FILE_SHARE_READ`.


I haven't tried QtCreator, but this also happens in VSCode and Vim. I suspect other editors are also affected, but I haven't checked.
I looked at the logs in Process Monitor and I think they do set the share flags properly (~95% certain, will still have to double-check). Even if they do it wrong, it will take time to fix this and I don't think we can afford breaking users with existing editors in our tooling.

That being said, I don't think the "isVolatile" parameter should be present in the filesystem APIs. I propose to handle that at the `vfs::FileSystem` implementation itself that is used underneath.
D55139 <https://reviews.llvm.org/D55139> is an initial fix for clangd that wraps existing `RealFileSystem` to force all files being open as volatile. It's much more reliable than passing the flags around, but it has a downside of also not memory-mapping **all** files, not just user files. However, I would not bother to try saving those bits unless we know that can actually critical for performance or memory usage (which I doubt would matter in the editors use-case, given that we still need to run the compiler on the files that we've read).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54995/new/

https://reviews.llvm.org/D54995





More information about the cfe-commits mailing list