flang-compiler / f18

F18 is a front-end for Fortran intended to replace the existing front-end in the Flang compiler
229 stars 48 forks source link

Update temp file handling for windows support #1036

Closed isuruf closed 4 years ago

DavidTruby commented 4 years ago

I think we might be better off here to use a std::optional<llvm::sys::fs::file_t> instead of representing non-present values as -1, and try and use only the functions from llvm::sys::fs that operate on file_t instead of int. Then we can avoid using platform specific functions. Would that work?

isuruf commented 4 years ago

That was my first thought, but createUniqueFile takes only a file descriptor (int) and doesn't take in a file_t.

DavidTruby commented 4 years ago

Ah yes, I remember why I did it this way now! Perhaps we can call createUniquePath ourselves and use that path to call createNativeFile?

DavidTruby commented 4 years ago

The best solution is probably to add a createUniquePath function to llvm that returns a file_t instead. I can look in to that.

isuruf commented 4 years ago

Perhaps we can call createUniquePath ourselves and use that path to call createNativeFile?

Race condition?

isuruf commented 4 years ago

Used llvm::sys::fs::convertFDToNativeFile which is found in LLVM 9.0.0 and above.

sscalpone commented 4 years ago

@isuruf Please update the commit message to explain why this change is required. (This explanation is in https://github.com/flang-compiler/f18/pull/1036#issuecomment-592699033 and in your reply.)

Also, please squash into a single commit.

isuruf commented 4 years ago

Done