[PATCH] D54995: [MemoryBuffer] Add the setter to be able to force disabled mmap

Mon Dec 3 07:22:24 PST 2018

ilya-biryukov added a comment.

In D54995#1316476 <https://reviews.llvm.org/D54995#1316476>, @yvvan wrote:

> I don't think removing the flag is a good idea since I can easily assume cases when user wants mmap and is ready to encounter locks. In our case it can be an IDE option which behavior to choose.

Would that option be useful if changing it forces the files user is editing in the IDE to be indefinitely locked and stops them from saving the files?
Anyway, even if you feel this behavior should be configurable, that's totally fine - I was referring to removing the flag from the vfs::FileSystem **interface**, possibly replacing it with a filesystem implementation that would handle the same thing at a lower level.
This flag is not useful to any of the filesystem implementation, except RealFileSystem, which actually has to deal with opening OS files.

> The ideal solution in my opinion would be to have isSystem(filename) in FileSystem class. And is based on system directories with which ASTUnit feeds the FileSystem for example.

I would disagree that `isSystem()` is a property of the filesystem. The filesystem abstraction is responsible for providing access to the files and their contents, why should classification of the files be filesystem-dependent? It's the wrong layer for the `isSystem()` check.
E.g. imagine implementing a filesystem that gets files from the network. The concerns filesystem is responsible for clearly include doing network IO. But why should the system/user classification of the files be different for it? What policy decisions does it have to make there? Clearly some other should do this classification and it's clearly independent of the filesystem being used.
OTOH, passing whether we want to open the files as memory-mapped does not sound completely crazy. But it would still be nice to avoid having it in the interface, since we **only** need it to workaround limitations specific to Windows + interactive editors, which is only a small subset of LLVM-based tools uses.

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

https://reviews.llvm.org/D54995