[Flang] Fix compilation on MinGW-w64

[mstorsjo]

What part of the code in the ifdefs is msvc-specific? On a first glance, it looks like regular win32 code that should work fine on mingw. What's the error that it tries to fix?

[ChinouneMehdi]

In D94707#2500131, @mstorsjo wrote:

What part of the code in the ifdefs is msvc-specific? On a first glance, it looks like regular win32 code that should work fine on mingw. What's the error that it tries to fix?

D:/dev/llvm-project/flang/runtime/file.cpp:48:57: error: use of undeclared identifier '_S_IREAD'
  int fd{::_open(tempFileName, _O_CREAT | _O_TEMPORARY, _S_IREAD | _S_IWRITE)};
                                                        ^
D:/dev/llvm-project/flang/runtime/file.cpp:48:68: error: use of undeclared identifier '_S_IWRITE'
  int fd{::_open(tempFileName, _O_CREAT | _O_TEMPORARY, _S_IREAD | _S_IWRITE)};
                                                                   ^
D:/dev/llvm-project/flang/runtime/file.cpp:404:9: warning: 'F_OK' macro redefined [-Wmacro-redefined]
#define F_OK 00
        ^
D:\Programs\msys64\mingw64\x86_64-w64-mingw32\include\io.h:182:9: note: previous definition is here
#define F_OK    0       /* Check for file existence */
        ^
D:/dev/llvm-project/flang/runtime/file.cpp:405:9: warning: 'W_OK' macro redefined [-Wmacro-redefined]
#define W_OK 02
        ^
D:\Programs\msys64\mingw64\x86_64-w64-mingw32\include\io.h:184:9: note: previous definition is here
#define W_OK    2       /* Check for write permission */
        ^
D:/dev/llvm-project/flang/runtime/file.cpp:406:9: warning: 'R_OK' macro redefined [-Wmacro-redefined]
#define R_OK 04
        ^
D:\Programs\msys64\mingw64\x86_64-w64-mingw32\include\io.h:185:9: note: previous definition is here
#define R_OK    4       /* Check for read permission */
        ^
4 warnings and 2 errors generated.

[mstorsjo]

In D94707#2500579, @ChinouneMehdi wrote:

In D94707#2500131, @mstorsjo wrote:

What part of the code in the ifdefs is msvc-specific? On a first glance, it looks like regular win32 code that should work fine on mingw. What's the error that it tries to fix?

D:/dev/llvm-project/flang/runtime/file.cpp:48:57: error: use of undeclared identifier '_S_IREAD'
  int fd{::_open(tempFileName, _O_CREAT | _O_TEMPORARY, _S_IREAD | _S_IWRITE)};
                                                        ^
D:/dev/llvm-project/flang/runtime/file.cpp:48:68: error: use of undeclared identifier '_S_IWRITE'
  int fd{::_open(tempFileName, _O_CREAT | _O_TEMPORARY, _S_IREAD | _S_IWRITE)};

The problem here is just that the right header for the _S_IREAD and _S_IWRITE constants haven't been included. Include sys/stat.h (which exists in both MSVC and mingw environments), i.e. move that include out of the ifdef/else, and this should work for mingw too.

[ChinouneMehdi]

In D94707#2500607, @mstorsjo wrote:

In D94707#2500579, @ChinouneMehdi wrote:

In D94707#2500131, @mstorsjo wrote:

What part of the code in the ifdefs is msvc-specific? On a first glance, it looks like regular win32 code that should work fine on mingw. What's the error that it tries to fix?

D:/dev/llvm-project/flang/runtime/file.cpp:48:57: error: use of undeclared identifier '_S_IREAD'
  int fd{::_open(tempFileName, _O_CREAT | _O_TEMPORARY, _S_IREAD | _S_IWRITE)};
                                                        ^
D:/dev/llvm-project/flang/runtime/file.cpp:48:68: error: use of undeclared identifier '_S_IWRITE'
  int fd{::_open(tempFileName, _O_CREAT | _O_TEMPORARY, _S_IREAD | _S_IWRITE)};

The problem here is just that the right header for the _S_IREAD and _S_IWRITE constants haven't been included. Include sys/stat.h (which exists in both MSVC and mingw environments), i.e. move that include out of the ifdef/else, and this should work for mingw too.

This is the easiest fix I could find.
You are free to make modifications on my patch.

[mstorsjo]

In D94707#2500752, @ChinouneMehdi wrote:

In D94707#2500607, @mstorsjo wrote:

In D94707#2500579, @ChinouneMehdi wrote:

In D94707#2500131, @mstorsjo wrote:

What part of the code in the ifdefs is msvc-specific? On a first glance, it looks like regular win32 code that should work fine on mingw. What's the error that it tries to fix?

D:/dev/llvm-project/flang/runtime/file.cpp:48:57: error: use of undeclared identifier '_S_IREAD'
  int fd{::_open(tempFileName, _O_CREAT | _O_TEMPORARY, _S_IREAD | _S_IWRITE)};
                                                        ^
D:/dev/llvm-project/flang/runtime/file.cpp:48:68: error: use of undeclared identifier '_S_IWRITE'
  int fd{::_open(tempFileName, _O_CREAT | _O_TEMPORARY, _S_IREAD | _S_IWRITE)};

The problem here is just that the right header for the _S_IREAD and _S_IWRITE constants haven't been included. Include sys/stat.h (which exists in both MSVC and mingw environments), i.e. move that include out of the ifdef/else, and this should work for mingw too.

This is the easiest fix I could find.
You are free to make modifications on my patch.

I'd prefer if you update your patch in this fashion. I'm not a flang reviewer/approver, so someone else needs to ack it in the end anyway.

[ChinouneMehdi]

Ping

[mstorsjo]

In D94707#2518887, @ChinouneMehdi wrote:

Ping

Before pinging, please update the patch with the change I suggested; don't change the ifdefs, but just move sys/stat.h out of the ifdef, above the #ifdef _WIN32.

[ChinouneMehdi]

Update diff.

[mstorsjo]

LGTM, although I think maybe some the flang team should comment as well. (The change for isCrash wasn't there in the previous iteration of the patch, but that bit looks correct as well, and affects any windows compilation, not only mingw.)

[awarzynski]

Thank you for working on this @ChinouneMehdi !

The bit in flang/tools/flang-driver/driver.cpp is an obvious typo - thank you for fixing that! The other change (sys/stat.h) makes sense to me, but I haven't worked on Flang's runtime and I don't use Windows. I know that @Meinersbur worked on Windows support.

[ChinouneMehdi]

Ping

[Meinersbur]

LGTM

[Meinersbur]

Do you need again help to commit?

[ChinouneMehdi]

In D94707#2568728, @Meinersbur wrote:

Do you need again help to commit?

Yes
Thanks.

flang/runtime/file.cpp

 //===-- runtime/file.cpp ----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "file.h"
 #include "magic-numbers.h"
 #include "memory.h"
 #include <algorithm>
 #include <cerrno>
 #include <cstring>
 #include <fcntl.h>
 #include <stdlib.h>
+#include <sys/stat.h>
 #ifdef _WIN32
 #define NOMINMAX
 #include <io.h>
 #include <windows.h>
 #else
-#include <sys/stat.h>
 #include <unistd.h>
 #endif
 
 namespace Fortran::runtime::io {
 
 void OpenFile::set_path(OwningPtr<char> &&path, std::size_t bytes) {
   path_ = std::move(path);
   pathLength_ = bytes;

flang/tools/flang-driver/driver.cpp

if (!res)
       res = CommandRes;
 
     // If result status is < 0 (e.g. when sys::ExecuteAndWait returns -1),
     // then the driver command signalled an error. On Windows, abort will
     // return an exit code of 3. In these cases, generate additional diagnostic
     // information if possible.
     isCrash = CommandRes < 0;
 #ifdef _WIN32
-    IsCrash |= CommandRes == 3;
+    isCrash |= CommandRes == 3;
 #endif
     if (isCrash) {
       theDriver.generateCompilationDiagnostics(*c, *failingCommand);
       break;
     }
   }
 
   diags.getClient()->finish();
 
   // If we have multiple failing commands, we return the result of the first
   // failing command.
   return res;
 }