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

Zibi Sarbino via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 15 09:30:00 PDT 2021


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



More information about the llvm-commits mailing list