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