[PATCH] D63859: [Support] Add getFileOrSTDIN which calls a callback if stdin is a tty

[Alex Brachet via Phabricator via llvm-commits]

abrachet marked 4 inline comments as done.
abrachet added inline comments.


================
Comment at: llvm/include/llvm/Support/MemoryBuffer.h:134
+  static ErrorOr<std::unique_ptr<MemoryBuffer>>
+  getFileOrSTDIN(const Twine &Filename, std::function<void(void)> TtyStdin,
+                 int64_t FileSize = -1, bool RequiresNullTerminator = true);
----------------
jhenderson wrote:
> Do you actually need an overload? Can this just be added as a default parameter for the other getFileOrSTDIN?
I think so. The alternative is to put it before FileSize, which isn't great because a caller might actually be specifying this somewhere, and at the end is worse if they need to specify FileSize and RequiresNullTerminator.


================
Comment at: llvm/unittests/Support/MemoryBufferTest.cpp:19
 #include "gtest/gtest.h"
+#include <unistd.h> // for dup2(2)
 
----------------
jhenderson wrote:
> Have you tried this on Windows. I suspect you'll find it won't work as is.
I'm guessing there is more to this than dup2 being called _dup2 and defined in io.h. I'll look into testing this on Windows. Also probably I need to #define dup2 not declare it. dup2 should be defined in io.h, but Microsoft say it is deprecated. I'll change this once I have a better understanding of how Windows file redirection works.


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

https://reviews.llvm.org/D63859