[clang] [clang-tools-extra] [lldb] [llvm] [SystemZ][z/OS] Propagate IsText parameter to open text files as text (PR #107906)

[Abhina Sree via cfe-commits]

abhina-sree wrote:

> thanks a lot for the swift response!
> 
> I can see how none of these implementations are not using those flags _today_, but we're changing the observable behavior for them as well, and if some of those implementations decides to give meaning to these flags, it might be impossible to keep it working. Especially because we're not clearly defining the semantics around how callers are going to figure those bits out.
> 
> So I am wondering:
> 
> * if we can make these changes in a very limited way, e.g. just in https://github.com/llvm/llvm-project/blob/main/llvm/lib/Support/Unix/Path.inc#L972-L1118. We already have lots of `__MVS__` specific regions in there. that's also the "right" layer if you want to perform syscalls.
> * if we need to make those changes in the VFS interfaces indeed, can you provide the rationale explaining that need and what the new semantics mean both for implementors of the interface and the callers of it?

So when we call this function sys::fs::openNativeFileForRead, there is an assumption that we already know whether the file is text or binary based on the OpenFlags that are passed to it. This function RealFileSystem::openFileForRead also assumes the file is binary by passing the OF_None flag. I'm not sure if there is a better way to determine whether the file is text or binary in this function without querying the file tag using a z/OS syscall in this function. 

I think I can limit it to the following change to avoid adding additional parameters to all the other functions which I agree is a much cleaner and less invasive solution. Please let me know your thoughts, thanks!

```
diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index 928c0b5a24ed..7153560754d1 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -22,6 +22,7 @@
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/Support/AutoConvert.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Chrono.h"
 #include "llvm/Support/Compiler.h"
@@ -325,8 +326,16 @@ ErrorOr<Status> RealFileSystem::status(const Twine &Path) {
 ErrorOr<std::unique_ptr<File>>
 RealFileSystem::openFileForRead(const Twine &Name) {
   SmallString<256> RealName, Storage;
+  bool IsText = true;
+#ifdef __MVS__
+  // If the file is tagged with a text ccsid, it may require autoconversion.
+  llvm::ErrorOr<bool> IsFileText =
+      llvm::iszOSTextFile(Name.str().c_str());
+  if (IsFileText)
+    IsText = *IsFileText;
+#endif
   Expected<file_t> FDOrErr = sys::fs::openNativeFileForRead(
-      adjustPath(Name, Storage), sys::fs::OF_None, &RealName);
+      adjustPath(Name, Storage), IsText ? sys::fs::OF_Text : sys::fs::OF_None, &RealName);
   if (!FDOrErr)
     return errorToErrorCode(FDOrErr.takeError());
   return std::unique_ptr<File>(
```



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