[Lldb-commits] [clang] [clang-tools-extra] [lldb] [llvm] Propagate IsText parameter to openFileForRead function (PR #110661)

[Abhina Sree via lldb-commits]

abhina-sree wrote:

> > @perry-ca raised some concerns in #109664 about this functionality requiring some context awareness, I don't think any of those is addressed by this patch either. Pretty much all of the callers apart from ASTReader is just using `IsText = true`.
> 
> It just happens that 90% of the time you are dealing with text files. As you noted, in the context of the ASTReader, the file being read is expected to be a binary file. In that case the parameter is false.
> 
> These change mirror interface to `getFileOrSTDIN()` which has a IsText parameter. This does touch a number of places but I feel the changes are in line with the rest of the file I/O functions in llvm.

Yes, I agree with Sean. Most of the time it is a text file when this function is called, there are only a few places where we expect binary files. And the reason for changing in VirtualFileSystem.cpp is because this is where the OF_None flag is being passed unconditionally, and there are other parameters like RequiresNullTerminator and IsVolatile that are being passed here as well which is why it seems like the correct place

https://github.com/llvm/llvm-project/pull/110661