[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-25 Thread Abhina Sree via Phabricator via cfe-commits 
abhina.sreeskantharajan added a comment.

In D97785#2651008 , @amccarth wrote:

> When changing an IO stream's mode from binary to text breaks only Windows, 
> it's likely due to the fact that Windows uses CR+LF as the newline indicator. 
>  Not only are the CRs sometimes unexpected, they can also throw off code that 
> tries to seek to a computed file position.

Yes, this is probably the cause. I created a patch to set Binary mode on 
Windows only https://reviews.llvm.org/D99363 which should workaround the issue 
and unblock the bot. I do plan to investigate a better fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-25 Thread Adrian McCarthy via Phabricator via cfe-commits 
amccarth added a comment.

When changing an IO stream's mode from binary to text breaks only Windows, it's 
likely due to the fact that Windows uses CR+LF as the newline indicator.  Not 
only are the CRs sometimes unexpected, they can also throw off code that tries 
to seek to a computed file position.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-25 Thread Yvan Roux via cfe-commits 
On Thu, 25 Mar 2021 at 18:13, Abhina Sree via Phabricator <
revi...@reviews.llvm.org> wrote:

> abhina.sreeskantharajan added a comment.
>
> In D97785#2650824 , @yroux wrote:
>
> > Hi Abhina,
> >
> > I think this patch broke Windows on ARM bots, the first failure observed
> 6
> > days ago in this build:
> >
> > https://lab.llvm.org/buildbot/#/builders/65/builds/1245
> >
> > I don't really get what the problem is, but the issue is related to
> > tablegen generated file AbstractTypeReader.inc and your patch is the only
> > one in the set which is touching that area.
> >
> > Does it ring a bell for you ?
> >
> > Cheers,
> > Yvan
>
> Hi, thanks for bringing this to my attention. Since only Windows seems to
> have this error, it may be caused by my patch. I suspect that setting
> OF_None flag/Binary mode for Windows in getFile() should fix it if that is
> the case. I'll work on a patch to hopefully fix the issue.
>

Ok, great thanks.


>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D97785/new/
>
> https://reviews.llvm.org/D97785
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-25 Thread Abhina Sree via Phabricator via cfe-commits 
abhina.sreeskantharajan added a comment.

In D97785#2650824 , @yroux wrote:

> Hi Abhina,
>
> I think this patch broke Windows on ARM bots, the first failure observed 6
> days ago in this build:
>
> https://lab.llvm.org/buildbot/#/builders/65/builds/1245
>
> I don't really get what the problem is, but the issue is related to
> tablegen generated file AbstractTypeReader.inc and your patch is the only
> one in the set which is touching that area.
>
> Does it ring a bell for you ?
>
> Cheers,
> Yvan

Hi, thanks for bringing this to my attention. Since only Windows seems to have 
this error, it may be caused by my patch. I suspect that setting OF_None 
flag/Binary mode for Windows in getFile() should fix it if that is the case. 
I'll work on a patch to hopefully fix the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-25 Thread Yvan Roux via cfe-commits 
I Abhina,

I think this patch broke Windows on ARM bots, the first failure observed 6
days ago in this build:

https://lab.llvm.org/buildbot/#/builders/65/builds/1245

I don't really get what the problem is, but the issue is related to
tablegen generated file AbstractTypeReader.inc and your patch is the only
one in the set which is touching that area.

Does it ring a bell for you ?

Cheers,
Yvan
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-22 Thread Abhina Sree via Phabricator via cfe-commits 
abhina.sreeskantharajan added a comment.

In D97785#2638824 , @MaskRay wrote:

> The recommended named parameter form is probably `/*Param=*/something`. It is 
> liked by both 
> https://clang.llvm.org/extra/clang-tidy/checks/bugprone-argument-comment.html 
> and clang-format.
> Another form look to me as well, if both the two tools like it.

Thanks, I've created the following patch https://reviews.llvm.org/D99072 to fix 
the parameter formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-19 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

The recommended named parameter form is probably `/*Param=*/something`. It is 
liked by both 
https://clang.llvm.org/extra/clang-tidy/checks/bugprone-argument-comment.html 
and clang-format.
Another form look to me as well, if both the two tools like it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-19 Thread Abhina Sree via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4f750f6ebc41: [SystemZ][z/OS] Distinguish between text and 
binary files on z/OS (authored by abhina.sreeskantharajan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

Files:
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/tools/arcmt-test/arcmt-test.cpp
  llvm/include/llvm/Support/FileSystem.h
  llvm/include/llvm/Support/MemoryBuffer.h
  llvm/lib/IRReader/IRReader.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/ToolOutputFile.cpp
  llvm/lib/TableGen/Main.cpp
  llvm/utils/FileCheck/FileCheck.cpp

Index: llvm/utils/FileCheck/FileCheck.cpp
===
--- llvm/utils/FileCheck/FileCheck.cpp
+++ llvm/utils/FileCheck/FileCheck.cpp
@@ -821,7 +821,9 @@
 
   // Read the expected strings from the check file.
   ErrorOr> CheckFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(CheckFilename);
+  MemoryBuffer::getFileOrSTDIN(CheckFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (std::error_code EC = CheckFileOrErr.getError()) {
 errs() << "Could not open check file '" << CheckFilename
<< "': " << EC.message() << '\n';
@@ -843,7 +845,9 @@
 
   // Open the file to check and add it to SourceMgr.
   ErrorOr> InputFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (InputFilename == "-")
 InputFilename = ""; // Overwrite for improved diagnostic messages
   if (std::error_code EC = InputFileOrErr.getError()) {
Index: llvm/lib/TableGen/Main.cpp
===
--- llvm/lib/TableGen/Main.cpp
+++ llvm/lib/TableGen/Main.cpp
@@ -70,7 +70,7 @@
 return reportError(argv0, "the option -d must be used together with -o\n");
 
   std::error_code EC;
-  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_None);
+  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_Text);
   if (EC)
 return reportError(argv0, "error opening " + DependFilename + ":" +
   EC.message() + "\n");
@@ -93,7 +93,7 @@
 
   Records.startTimer("Parse, build records");
   ErrorOr> FileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, -1, true, true);
   if (std::error_code EC = FileOrErr.getError())
 return reportError(argv0, "Could not open input file '" + InputFilename +
   "': " + EC.message() + "\n");
@@ -137,13 +137,14 @@
 // Only updates the real output file if there are any differences.
 // This prevents recompilation of all the files depending on it if there
 // aren't any.
-if (auto ExistingOrErr = MemoryBuffer::getFile(OutputFilename))
+if (auto ExistingOrErr =
+MemoryBuffer::getFile(OutputFilename, -1, true, false, true))
   if (std::move(ExistingOrErr.get())->getBuffer() == Out.str())
 WriteFile = false;
   }
   if (WriteFile) {
 std::error_code EC;
-ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_None);
+ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_Text);
 if (EC)
   return reportError(argv0, "error opening " + OutputFilename + ": " +
 EC.message() + "\n");
Index: llvm/lib/Support/ToolOutputFile.cpp
===
--- llvm/lib/Support/ToolOutputFile.cpp
+++ llvm/lib/Support/ToolOutputFile.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Signals.h"
 using namespace llvm;
@@ -45,7 +46,12 @@
 EC = std::error_code();
 return;
   }
-  OSHolder.emplace(Filename, EC, Flags);
+
+  // On Windows, we set the OF_None flag even for text files to avoid
+  // CRLF translation.
+  OSHolder.emplace(
+  Filename, EC,
+  llvm::Triple(LLVM_HOST_TRIPLE).isOSWindows() ? sys::fs::OF_None : Flags);
   OS = OSHolder.getPointer();
   // If open fails, no cleanup is needed.
   if (EC)
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -167,8 +167,8 @@
 static std::error_code
 createUniqueEntity(const Twine &Model, int &ResultFD,
SmallVe

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-16 Thread Abhina Sree via Phabricator via cfe-commits 
abhina.sreeskantharajan updated this revision to Diff 330989.
abhina.sreeskantharajan added a comment.

NFC edit: Fix formatting/lint


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

Files:
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/tools/arcmt-test/arcmt-test.cpp
  llvm/include/llvm/Support/FileSystem.h
  llvm/include/llvm/Support/MemoryBuffer.h
  llvm/lib/IRReader/IRReader.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/ToolOutputFile.cpp
  llvm/lib/TableGen/Main.cpp
  llvm/utils/FileCheck/FileCheck.cpp

Index: llvm/utils/FileCheck/FileCheck.cpp
===
--- llvm/utils/FileCheck/FileCheck.cpp
+++ llvm/utils/FileCheck/FileCheck.cpp
@@ -803,7 +803,9 @@
 
   // Read the expected strings from the check file.
   ErrorOr> CheckFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(CheckFilename);
+  MemoryBuffer::getFileOrSTDIN(CheckFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (std::error_code EC = CheckFileOrErr.getError()) {
 errs() << "Could not open check file '" << CheckFilename
<< "': " << EC.message() << '\n';
@@ -825,7 +827,9 @@
 
   // Open the file to check and add it to SourceMgr.
   ErrorOr> InputFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (InputFilename == "-")
 InputFilename = ""; // Overwrite for improved diagnostic messages
   if (std::error_code EC = InputFileOrErr.getError()) {
Index: llvm/lib/TableGen/Main.cpp
===
--- llvm/lib/TableGen/Main.cpp
+++ llvm/lib/TableGen/Main.cpp
@@ -70,7 +70,7 @@
 return reportError(argv0, "the option -d must be used together with -o\n");
 
   std::error_code EC;
-  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_None);
+  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_Text);
   if (EC)
 return reportError(argv0, "error opening " + DependFilename + ":" +
   EC.message() + "\n");
@@ -93,7 +93,7 @@
 
   Records.startTimer("Parse, build records");
   ErrorOr> FileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, -1, true, true);
   if (std::error_code EC = FileOrErr.getError())
 return reportError(argv0, "Could not open input file '" + InputFilename +
   "': " + EC.message() + "\n");
@@ -137,13 +137,14 @@
 // Only updates the real output file if there are any differences.
 // This prevents recompilation of all the files depending on it if there
 // aren't any.
-if (auto ExistingOrErr = MemoryBuffer::getFile(OutputFilename))
+if (auto ExistingOrErr =
+MemoryBuffer::getFile(OutputFilename, -1, true, false, true))
   if (std::move(ExistingOrErr.get())->getBuffer() == Out.str())
 WriteFile = false;
   }
   if (WriteFile) {
 std::error_code EC;
-ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_None);
+ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_Text);
 if (EC)
   return reportError(argv0, "error opening " + OutputFilename + ": " +
 EC.message() + "\n");
Index: llvm/lib/Support/ToolOutputFile.cpp
===
--- llvm/lib/Support/ToolOutputFile.cpp
+++ llvm/lib/Support/ToolOutputFile.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Signals.h"
 using namespace llvm;
@@ -45,7 +46,12 @@
 EC = std::error_code();
 return;
   }
-  OSHolder.emplace(Filename, EC, Flags);
+
+  // On Windows, we set the OF_None flag even for text files to avoid
+  // CRLF translation.
+  OSHolder.emplace(
+  Filename, EC,
+  llvm::Triple(LLVM_HOST_TRIPLE).isOSWindows() ? sys::fs::OF_None : Flags);
   OS = OSHolder.getPointer();
   // If open fails, no cleanup is needed.
   if (EC)
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -167,8 +167,8 @@
 static std::error_code
 createUniqueEntity(const Twine &Model, int &ResultFD,
SmallVectorImpl &ResultPath, bool MakeAbsolute,
-   unsigned Mode, FSEntity Type,
-   sys::fs::OpenFlags Flag

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-16 Thread Zibi Sarbino via Phabricator via cfe-commits 
zibi accepted this revision.
zibi added a comment.
This revision is now accepted and ready to land.

LTGM, thx for an extra mile, Abhina.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-16 Thread Abhina Sree via Phabricator via cfe-commits 
abhina.sreeskantharajan marked an inline comment as done.
abhina.sreeskantharajan added inline comments.



Comment at: clang/lib/Frontend/CompilerInstance.cpp:771
+TempPath, fd, TempPath,
+llvm::sys::fs::all_read | llvm::sys::fs::all_write,
+Binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_Text);

zibi wrote:
> The ` llvm::sys::fs::all_read | llvm::sys::fs::all_write` seems to be a 
> default so if we make that parameter last we won't need to pass it and worry 
> about the mode parameter which would be the second last default parameter.  
> Switch parameters only when you determine that indeed `tag` is more frequent 
> parameter which need to be set comparing to `mode` parameter.
Thanks, I've reordered the args as you suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-16 Thread Abhina Sree via Phabricator via cfe-commits 
abhina.sreeskantharajan updated this revision to Diff 330952.
abhina.sreeskantharajan added a comment.

Sorry Zibi, I misunderstood your comment. I updated the patch to switch around 
the Flag and Mode defaults in createUniqueFile because the flags was set 
explicitly more often than mode.  I also had to reorder the args in 
createUniqueEntity to match the ordering.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

Files:
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/tools/arcmt-test/arcmt-test.cpp
  llvm/include/llvm/Support/FileSystem.h
  llvm/include/llvm/Support/MemoryBuffer.h
  llvm/lib/IRReader/IRReader.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/ToolOutputFile.cpp
  llvm/lib/TableGen/Main.cpp
  llvm/utils/FileCheck/FileCheck.cpp

Index: llvm/utils/FileCheck/FileCheck.cpp
===
--- llvm/utils/FileCheck/FileCheck.cpp
+++ llvm/utils/FileCheck/FileCheck.cpp
@@ -803,7 +803,9 @@
 
   // Read the expected strings from the check file.
   ErrorOr> CheckFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(CheckFilename);
+  MemoryBuffer::getFileOrSTDIN(CheckFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (std::error_code EC = CheckFileOrErr.getError()) {
 errs() << "Could not open check file '" << CheckFilename
<< "': " << EC.message() << '\n';
@@ -825,7 +827,9 @@
 
   // Open the file to check and add it to SourceMgr.
   ErrorOr> InputFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (InputFilename == "-")
 InputFilename = ""; // Overwrite for improved diagnostic messages
   if (std::error_code EC = InputFileOrErr.getError()) {
Index: llvm/lib/TableGen/Main.cpp
===
--- llvm/lib/TableGen/Main.cpp
+++ llvm/lib/TableGen/Main.cpp
@@ -70,7 +70,7 @@
 return reportError(argv0, "the option -d must be used together with -o\n");
 
   std::error_code EC;
-  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_None);
+  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_Text);
   if (EC)
 return reportError(argv0, "error opening " + DependFilename + ":" +
   EC.message() + "\n");
@@ -93,7 +93,7 @@
 
   Records.startTimer("Parse, build records");
   ErrorOr> FileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, -1, true, true);
   if (std::error_code EC = FileOrErr.getError())
 return reportError(argv0, "Could not open input file '" + InputFilename +
   "': " + EC.message() + "\n");
@@ -137,13 +137,14 @@
 // Only updates the real output file if there are any differences.
 // This prevents recompilation of all the files depending on it if there
 // aren't any.
-if (auto ExistingOrErr = MemoryBuffer::getFile(OutputFilename))
+if (auto ExistingOrErr =
+MemoryBuffer::getFile(OutputFilename, -1, true, false, true))
   if (std::move(ExistingOrErr.get())->getBuffer() == Out.str())
 WriteFile = false;
   }
   if (WriteFile) {
 std::error_code EC;
-ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_None);
+ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_Text);
 if (EC)
   return reportError(argv0, "error opening " + OutputFilename + ": " +
 EC.message() + "\n");
Index: llvm/lib/Support/ToolOutputFile.cpp
===
--- llvm/lib/Support/ToolOutputFile.cpp
+++ llvm/lib/Support/ToolOutputFile.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Signals.h"
 using namespace llvm;
@@ -45,7 +46,12 @@
 EC = std::error_code();
 return;
   }
-  OSHolder.emplace(Filename, EC, Flags);
+
+  // On Windows, we set the OF_None flag even for text files to avoid
+  // CRLF translation.
+  OSHolder.emplace(
+  Filename, EC,
+  llvm::Triple(LLVM_HOST_TRIPLE).isOSWindows() ? sys::fs::OF_None : Flags);
   OS = OSHolder.getPointer();
   // If open fails, no cleanup is needed.
   if (EC)
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -167,8 +167,8 @@
 stat

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-15 Thread Zibi Sarbino via Phabricator via cfe-commits 
zibi added inline comments.



Comment at: clang/lib/Frontend/CompilerInstance.cpp:771
+TempPath, fd, TempPath,
+llvm::sys::fs::all_read | llvm::sys::fs::all_write,
+Binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_Text);

The ` llvm::sys::fs::all_read | llvm::sys::fs::all_write` seems to be a default 
so if we make that parameter last we won't need to pass it and worry about the 
mode parameter which would be the second last default parameter.  Switch 
parameters only when you determine that indeed `tag` is more frequent parameter 
which need to be set comparing to `mode` parameter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-15 Thread Anirudh Prasad via Phabricator via cfe-commits 
anirudhp added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-15 Thread Abhina Sree via Phabricator via cfe-commits 
abhina.sreeskantharajan marked 2 inline comments as done.
abhina.sreeskantharajan added a comment.

In D97785#2626427 , @zibi wrote:

> LGTM, I just wonder if we can make an extra parameter to be default. I notice 
> some places that is a default parameter but not in all instances. With 
> default parameter some of the calls might be simplified if there is no need 
> to override it.

Thanks for reviewing. Do you have an example of which function is missing a 
default? The functions I modified in the .h files all have defaults to OF_None 
or binary. I can make another patch for additional changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-15 Thread Abhina Sree via Phabricator via cfe-commits 
abhina.sreeskantharajan updated this revision to Diff 330747.
abhina.sreeskantharajan added a comment.

Removing llvm::Triple variables


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

Files:
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/tools/arcmt-test/arcmt-test.cpp
  llvm/include/llvm/Support/FileSystem.h
  llvm/include/llvm/Support/MemoryBuffer.h
  llvm/lib/IRReader/IRReader.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/ToolOutputFile.cpp
  llvm/lib/TableGen/Main.cpp
  llvm/utils/FileCheck/FileCheck.cpp

Index: llvm/utils/FileCheck/FileCheck.cpp
===
--- llvm/utils/FileCheck/FileCheck.cpp
+++ llvm/utils/FileCheck/FileCheck.cpp
@@ -803,7 +803,9 @@
 
   // Read the expected strings from the check file.
   ErrorOr> CheckFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(CheckFilename);
+  MemoryBuffer::getFileOrSTDIN(CheckFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (std::error_code EC = CheckFileOrErr.getError()) {
 errs() << "Could not open check file '" << CheckFilename
<< "': " << EC.message() << '\n';
@@ -825,7 +827,9 @@
 
   // Open the file to check and add it to SourceMgr.
   ErrorOr> InputFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (InputFilename == "-")
 InputFilename = ""; // Overwrite for improved diagnostic messages
   if (std::error_code EC = InputFileOrErr.getError()) {
Index: llvm/lib/TableGen/Main.cpp
===
--- llvm/lib/TableGen/Main.cpp
+++ llvm/lib/TableGen/Main.cpp
@@ -70,7 +70,7 @@
 return reportError(argv0, "the option -d must be used together with -o\n");
 
   std::error_code EC;
-  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_None);
+  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_Text);
   if (EC)
 return reportError(argv0, "error opening " + DependFilename + ":" +
   EC.message() + "\n");
@@ -93,7 +93,7 @@
 
   Records.startTimer("Parse, build records");
   ErrorOr> FileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, -1, true, true);
   if (std::error_code EC = FileOrErr.getError())
 return reportError(argv0, "Could not open input file '" + InputFilename +
   "': " + EC.message() + "\n");
@@ -137,13 +137,14 @@
 // Only updates the real output file if there are any differences.
 // This prevents recompilation of all the files depending on it if there
 // aren't any.
-if (auto ExistingOrErr = MemoryBuffer::getFile(OutputFilename))
+if (auto ExistingOrErr =
+MemoryBuffer::getFile(OutputFilename, -1, true, false, true))
   if (std::move(ExistingOrErr.get())->getBuffer() == Out.str())
 WriteFile = false;
   }
   if (WriteFile) {
 std::error_code EC;
-ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_None);
+ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_Text);
 if (EC)
   return reportError(argv0, "error opening " + OutputFilename + ": " +
 EC.message() + "\n");
Index: llvm/lib/Support/ToolOutputFile.cpp
===
--- llvm/lib/Support/ToolOutputFile.cpp
+++ llvm/lib/Support/ToolOutputFile.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Signals.h"
 using namespace llvm;
@@ -45,7 +46,12 @@
 EC = std::error_code();
 return;
   }
-  OSHolder.emplace(Filename, EC, Flags);
+
+  // On Windows, we set the OF_None flag even for text files to avoid
+  // CRLF translation.
+  OSHolder.emplace(
+  Filename, EC,
+  llvm::Triple(LLVM_HOST_TRIPLE).isOSWindows() ? sys::fs::OF_None : Flags);
   OS = OSHolder.getPointer();
   // If open fails, no cleanup is needed.
   if (EC)
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -816,13 +816,7 @@
 
 std::error_code createUniqueFile(const Twine &Model, int &ResultFd,
  SmallVectorImpl &ResultPath,
- unsigned Mode) {
-  return createUniqueEntity(Model, ResultFd, Result

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-15 Thread Zibi Sarbino via Phabricator via cfe-commits 
zibi added a comment.

LGTM, I just wonder if we can make an extra parameter to be default. I notice 
some places that is a default parameter but not in all instances. With default 
parameter some of the calls might be simplified if there is no need to override 
it.




Comment at: clang/lib/Frontend/FrontendActions.cpp:812
+  bool BinaryMode = false;
+  llvm::Triple HostTriple(LLVM_HOST_TRIPLE);
+  if (HostTriple.isOSWindows()) {

If we don't need Triple anywhere else except here we can remove the new for 
local variable and do crate it as part of the test.

```
if (HostTriple(LLVM_HOST_TRIPLE).isOSWindows()) {
...
```
or even

```
bool BinaryMode = (HostTriple(LLVM_HOST_TRIPLE).isOSWindows())  ? true : false;

```



Comment at: llvm/lib/Support/ToolOutputFile.cpp:50
+
+  llvm::Triple HostTriple(LLVM_HOST_TRIPLE);
+  // On Windows, we set the OF_None flag even for text files to avoid

Can we avoid creating  `HostTriple` here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-09 Thread Abhina Sree via Phabricator via cfe-commits 
abhina.sreeskantharajan added a comment.

If there are no concerns about setting OF_None flag for all ToolOutputFiles on 
Windows only, I would also like to get your reviews for this patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-05 Thread Abhina Sree via Phabricator via cfe-commits 
abhina.sreeskantharajan updated this revision to Diff 328483.
abhina.sreeskantharajan added reviewers: amccarth, MaskRay, jhenderson.
abhina.sreeskantharajan added a comment.

Fix formatting from lint.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

Files:
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/tools/arcmt-test/arcmt-test.cpp
  llvm/include/llvm/Support/FileSystem.h
  llvm/include/llvm/Support/MemoryBuffer.h
  llvm/lib/IRReader/IRReader.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/ToolOutputFile.cpp
  llvm/lib/TableGen/Main.cpp
  llvm/utils/FileCheck/FileCheck.cpp

Index: llvm/utils/FileCheck/FileCheck.cpp
===
--- llvm/utils/FileCheck/FileCheck.cpp
+++ llvm/utils/FileCheck/FileCheck.cpp
@@ -803,7 +803,9 @@
 
   // Read the expected strings from the check file.
   ErrorOr> CheckFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(CheckFilename);
+  MemoryBuffer::getFileOrSTDIN(CheckFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (std::error_code EC = CheckFileOrErr.getError()) {
 errs() << "Could not open check file '" << CheckFilename
<< "': " << EC.message() << '\n';
@@ -825,7 +827,9 @@
 
   // Open the file to check and add it to SourceMgr.
   ErrorOr> InputFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (InputFilename == "-")
 InputFilename = ""; // Overwrite for improved diagnostic messages
   if (std::error_code EC = InputFileOrErr.getError()) {
Index: llvm/lib/TableGen/Main.cpp
===
--- llvm/lib/TableGen/Main.cpp
+++ llvm/lib/TableGen/Main.cpp
@@ -70,7 +70,7 @@
 return reportError(argv0, "the option -d must be used together with -o\n");
 
   std::error_code EC;
-  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_None);
+  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_Text);
   if (EC)
 return reportError(argv0, "error opening " + DependFilename + ":" +
   EC.message() + "\n");
@@ -93,7 +93,7 @@
 
   Records.startTimer("Parse, build records");
   ErrorOr> FileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, -1, true, true);
   if (std::error_code EC = FileOrErr.getError())
 return reportError(argv0, "Could not open input file '" + InputFilename +
   "': " + EC.message() + "\n");
@@ -137,13 +137,14 @@
 // Only updates the real output file if there are any differences.
 // This prevents recompilation of all the files depending on it if there
 // aren't any.
-if (auto ExistingOrErr = MemoryBuffer::getFile(OutputFilename))
+if (auto ExistingOrErr =
+MemoryBuffer::getFile(OutputFilename, -1, true, false, true))
   if (std::move(ExistingOrErr.get())->getBuffer() == Out.str())
 WriteFile = false;
   }
   if (WriteFile) {
 std::error_code EC;
-ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_None);
+ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_Text);
 if (EC)
   return reportError(argv0, "error opening " + OutputFilename + ": " +
 EC.message() + "\n");
Index: llvm/lib/Support/ToolOutputFile.cpp
===
--- llvm/lib/Support/ToolOutputFile.cpp
+++ llvm/lib/Support/ToolOutputFile.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Signals.h"
 using namespace llvm;
@@ -45,7 +46,12 @@
 EC = std::error_code();
 return;
   }
-  OSHolder.emplace(Filename, EC, Flags);
+
+  llvm::Triple HostTriple(LLVM_HOST_TRIPLE);
+  // On Windows, we set the OF_None flag even for text files to avoid
+  // CRLF translation.
+  OSHolder.emplace(Filename, EC,
+   HostTriple.isOSWindows() ? sys::fs::OF_None : Flags);
   OS = OSHolder.getPointer();
   // If open fails, no cleanup is needed.
   if (EC)
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -816,13 +816,7 @@
 
 std::error_code createUniqueFile(const Twine &Model, int &ResultFd,
  SmallVectorImpl &ResultPath,
-

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-05 Thread Abhina Sree via Phabricator via cfe-commits 
abhina.sreeskantharajan added inline comments.



Comment at: clang/lib/Frontend/FrontendActions.cpp:841
 }
   }
 

amccarth wrote:
> The preceding block that detects the type of line separator seems ripe for 
> factoring out into a separate function.  It's a lot of low-level detail that 
> visually outweighs the primary intent of this method.
> 
> But I'm fine with the change as it doesn't impact Windows and--as far as I 
> understand--Posix platforms don't distinguish between text and binary mode.
Right, my intention was not to change the current behaviour, but only limit it 
to Windows which is the only affected platform.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-04 Thread Adrian McCarthy via Phabricator via cfe-commits 
amccarth added a comment.

From a Windows point of view, this LGTM.




Comment at: clang/lib/Frontend/FrontendActions.cpp:841
 }
   }
 

The preceding block that detects the type of line separator seems ripe for 
factoring out into a separate function.  It's a lot of low-level detail that 
visually outweighs the primary intent of this method.

But I'm fine with the change as it doesn't impact Windows and--as far as I 
understand--Posix platforms don't distinguish between text and binary mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-04 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk added a comment.

Yes, we took steps to suppress CRLF generation. Part of that was to avoid 
Windows-only test failures from trailing CR whitespace interacting with tools 
like `diff`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-03 Thread Abhina Sree via Phabricator via cfe-commits 
abhina.sreeskantharajan updated this revision to Diff 327790.
abhina.sreeskantharajan added a comment.

Remove some changes that cause lit failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97785

Files:
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/tools/arcmt-test/arcmt-test.cpp
  llvm/include/llvm/Support/FileSystem.h
  llvm/include/llvm/Support/MemoryBuffer.h
  llvm/lib/IRReader/IRReader.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/ToolOutputFile.cpp
  llvm/lib/TableGen/Main.cpp
  llvm/utils/FileCheck/FileCheck.cpp

Index: llvm/utils/FileCheck/FileCheck.cpp
===
--- llvm/utils/FileCheck/FileCheck.cpp
+++ llvm/utils/FileCheck/FileCheck.cpp
@@ -803,7 +803,9 @@
 
   // Read the expected strings from the check file.
   ErrorOr> CheckFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(CheckFilename);
+  MemoryBuffer::getFileOrSTDIN(CheckFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (std::error_code EC = CheckFileOrErr.getError()) {
 errs() << "Could not open check file '" << CheckFilename
<< "': " << EC.message() << '\n';
@@ -825,7 +827,9 @@
 
   // Open the file to check and add it to SourceMgr.
   ErrorOr> InputFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (InputFilename == "-")
 InputFilename = ""; // Overwrite for improved diagnostic messages
   if (std::error_code EC = InputFileOrErr.getError()) {
Index: llvm/lib/TableGen/Main.cpp
===
--- llvm/lib/TableGen/Main.cpp
+++ llvm/lib/TableGen/Main.cpp
@@ -70,7 +70,7 @@
 return reportError(argv0, "the option -d must be used together with -o\n");
 
   std::error_code EC;
-  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_None);
+  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_Text);
   if (EC)
 return reportError(argv0, "error opening " + DependFilename + ":" +
   EC.message() + "\n");
@@ -93,7 +93,7 @@
 
   Records.startTimer("Parse, build records");
   ErrorOr> FileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, -1, true, true);
   if (std::error_code EC = FileOrErr.getError())
 return reportError(argv0, "Could not open input file '" + InputFilename +
   "': " + EC.message() + "\n");
@@ -137,13 +137,14 @@
 // Only updates the real output file if there are any differences.
 // This prevents recompilation of all the files depending on it if there
 // aren't any.
-if (auto ExistingOrErr = MemoryBuffer::getFile(OutputFilename))
+if (auto ExistingOrErr = MemoryBuffer::getFile(OutputFilename, -1,
+   true, false, true))
   if (std::move(ExistingOrErr.get())->getBuffer() == Out.str())
 WriteFile = false;
   }
   if (WriteFile) {
 std::error_code EC;
-ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_None);
+ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_Text);
 if (EC)
   return reportError(argv0, "error opening " + OutputFilename + ": " +
 EC.message() + "\n");
Index: llvm/lib/Support/ToolOutputFile.cpp
===
--- llvm/lib/Support/ToolOutputFile.cpp
+++ llvm/lib/Support/ToolOutputFile.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Signals.h"
 using namespace llvm;
@@ -45,7 +46,12 @@
 EC = std::error_code();
 return;
   }
-  OSHolder.emplace(Filename, EC, Flags);
+
+  llvm::Triple HostTriple(LLVM_HOST_TRIPLE);
+  // On Windows, we set the OF_None flag even for text files to avoid
+  // CRLF translation.
+  OSHolder.emplace(Filename, EC,
+   HostTriple.isOSWindows() ? sys::fs::OF_None : Flags);
   OS = OSHolder.getPointer();
   // If open fails, no cleanup is needed.
   if (EC)
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -816,13 +816,7 @@
 
 std::error_code createUniqueFile(const Twine &Model, int &ResultFd,
  SmallVectorImpl &ResultPath,
-

[PATCH] D97785: [SystemZ][z/OS] Distinguish between text and binary files on z/OS

2021-03-02 Thread Abhina Sree via Phabricator via cfe-commits 
abhina.sreeskantharajan created this revision.
Herald added subscribers: dexonsmith, martong, thopre, hiraditya.
abhina.sreeskantharajan requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This patch consists of changes to help distinguish between text and binary 
content correctly on z/OS. I would like to get feedback from Windows users on 
setting OF_None for all ToolOutputFiles. This seems to have been done as an 
optimization to prevent CRLF translation on Windows in the past.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97785

Files:
  clang/lib/ARCMigrate/FileRemapper.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/tools/arcmt-test/arcmt-test.cpp
  llvm/include/llvm/Support/FileSystem.h
  llvm/include/llvm/Support/MemoryBuffer.h
  llvm/lib/IRReader/IRReader.cpp
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/ToolOutputFile.cpp
  llvm/lib/TableGen/Main.cpp
  llvm/utils/FileCheck/FileCheck.cpp

Index: llvm/utils/FileCheck/FileCheck.cpp
===
--- llvm/utils/FileCheck/FileCheck.cpp
+++ llvm/utils/FileCheck/FileCheck.cpp
@@ -803,7 +803,9 @@
 
   // Read the expected strings from the check file.
   ErrorOr> CheckFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(CheckFilename);
+  MemoryBuffer::getFileOrSTDIN(CheckFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (std::error_code EC = CheckFileOrErr.getError()) {
 errs() << "Could not open check file '" << CheckFilename
<< "': " << EC.message() << '\n';
@@ -825,7 +827,9 @@
 
   // Open the file to check and add it to SourceMgr.
   ErrorOr> InputFileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, /*FileSize*/ -1,
+   /*RequiresNullTerminator*/ true,
+   /*IsText*/ true);
   if (InputFilename == "-")
 InputFilename = ""; // Overwrite for improved diagnostic messages
   if (std::error_code EC = InputFileOrErr.getError()) {
Index: llvm/lib/TableGen/Main.cpp
===
--- llvm/lib/TableGen/Main.cpp
+++ llvm/lib/TableGen/Main.cpp
@@ -70,7 +70,7 @@
 return reportError(argv0, "the option -d must be used together with -o\n");
 
   std::error_code EC;
-  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_None);
+  ToolOutputFile DepOut(DependFilename, EC, sys::fs::OF_Text);
   if (EC)
 return reportError(argv0, "error opening " + DependFilename + ":" +
   EC.message() + "\n");
@@ -93,7 +93,7 @@
 
   Records.startTimer("Parse, build records");
   ErrorOr> FileOrErr =
-  MemoryBuffer::getFileOrSTDIN(InputFilename);
+  MemoryBuffer::getFileOrSTDIN(InputFilename, -1, true, true);
   if (std::error_code EC = FileOrErr.getError())
 return reportError(argv0, "Could not open input file '" + InputFilename +
   "': " + EC.message() + "\n");
@@ -137,13 +137,14 @@
 // Only updates the real output file if there are any differences.
 // This prevents recompilation of all the files depending on it if there
 // aren't any.
-if (auto ExistingOrErr = MemoryBuffer::getFile(OutputFilename))
+if (auto ExistingOrErr = MemoryBuffer::getFile(OutputFilename, -1,
+   true, false, true))
   if (std::move(ExistingOrErr.get())->getBuffer() == Out.str())
 WriteFile = false;
   }
   if (WriteFile) {
 std::error_code EC;
-ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_None);
+ToolOutputFile OutFile(OutputFilename, EC, sys::fs::OF_Text);
 if (EC)
   return reportError(argv0, "error opening " + OutputFilename + ": " +
 EC.message() + "\n");
Index: llvm/lib/Support/ToolOutputFile.cpp
===
--- llvm/lib/Support/ToolOutputFile.cpp
+++ llvm/lib/Support/ToolOutputFile.cpp
@@ -11,6 +11,7 @@
 //===--===//
 
 #include "llvm/Support/ToolOutputFile.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Signals.h"
 using namespace llvm;
@@ -45,7 +46,12 @@
 EC = std::error_code();
 return;
   }
-  OSHolder.emplace(Filename, EC, Flags);
+
+  llvm::Triple HostTriple(LLVM_HOST_TRIPLE);
+  // On Windows, we set the OF_None flag even for text files to avoid
+  // CRLF translation.
+  OSHolder.emplace(Filename, EC,
+   HostTriple.isOSWindows() ? sys::fs::OF_None : Flags);
   OS = OSHolder.getPointer();
   //