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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to