[Support] Add docs for 'openFileFor{Write,Read}'

[modocache]

Summary

Add documentation for the LLVM Support functions openFileForWrite and
openFileForRead. The openFileForRead parameter RealPath, in
particular, I think warranted some explanation.

In addition, make the behavior of the functions more consistent across
platforms. Prior to this patch, Windows would set or not set the result
file descriptor based on the nature of the error, whereas Unix would
consistently set it to -1 if the open failed. Make Windows
consistently set it to -1 as well.

Test Plan:

1. ninja check-llvm
2. ninja docs-llvm-html

[danielmartin]

LGTM.

[scanon]

Two quick notes as someone who has never used these functions:

• Name is really the *path* of the file to open, right?
• What happens to ResultFD if the function fails?

[modocache]

Thanks for the comments!

Name is really the *path* of the file to open, right?

Correct, it's the path. It might be relative or absolute. I'll update the docs (but not the variable name).

What happens to ResultFD if the function fails?

Good point. On Unix it's set to the return value of open(2). I'll double check what happens on Windows and update the docs.

[scanon]

One more question: the caller is responsible for closing the file when they're done, right?

[modocache]

Correct! I'll mention that as well.

[modocache]

I updated this diff to not only document how the ResultFD is set, but also make it a little more consistent across platforms. Previously Windows would only sometimes set it to -1 in the case of failure, now it always does.

[scanon]

LGTM, thanks!

include/llvm/Support/FileSystem.h

inline OpenFlags operator|(OpenFlags A, OpenFlags B) {
   return OpenFlags(unsigned(A) | unsigned(B));
 }
 
 inline OpenFlags &operator|=(OpenFlags &A, OpenFlags B) {
   A = A | B;
   return A;
 }
 
+/// @brief Opens the file with the given name in a write-only or read-write
+/// mode, returning its open file descriptor. If the file does not exist, it
+/// is created.
+///
+/// The caller is responsible for closing the file descriptor once they are
+/// finished with it.
+///
+/// @param Name The path of the file to open, relative or absolute.
+/// @param ResultFD If the file could be opened successfully, its descriptor
+///                 is stored in this location. Otherwise, this is set to -1.
+/// @param Flags Additional flags used to determine whether the file should be
+///              opened in, for example, read-write or in write-only mode.
+/// @param Mode The access permissions of the file, represented in octal.
+/// @returns errc::success if \a Name has been opened, otherwise a
+///          platform-specific error_code.
 std::error_code openFileForWrite(const Twine &Name, int &ResultFD,
                                  OpenFlags Flags, unsigned Mode = 0666);
 
+/// @brief Opens the file with the given name in a read-only mode, returning
+/// its open file descriptor.
+///
+/// The caller is responsible for closing the file descriptor once they are
+/// finished with it.
+///
+/// @param Name The path of the file to open, relative or absolute.
+/// @param ResultFD If the file could be opened successfully, its descriptor
+///                 is stored in this location. Otherwise, this is set to -1.
+/// @param RealPath If nonnull, extra work is done to determine the real path
+///                 of the opened file, and that path is stored in this
+///                 location.
+/// @returns errc::success if \a Name has been opened, otherwise a
+///          platform-specific error_code.
 std::error_code openFileForRead(const Twine &Name, int &ResultFD,
                                 SmallVectorImpl<char> *RealPath = nullptr);
 
 std::error_code getUniqueID(const Twine Path, UniqueID &Result);
 
 /// Get disk space usage information.
 ///
 /// Note: Users must be careful about "Time Of Check, Time Of Use" kind of bug.

lib/Support/Windows/Path.inc

if (H == INVALID_HANDLE_VALUE)
     return mapWindowsError(GetLastError());
   std::error_code EC = realPathFromHandle(H, RealPath);
   ::CloseHandle(H);
   return EC;
 }
 
 std::error_code openFileForRead(const Twine &Name, int &ResultFD,
                                 SmallVectorImpl<char> *RealPath) {
+  ResultFD = -1;
   SmallVector<wchar_t, 128> PathUTF16;
 
   if (std::error_code EC = widenPath(Name, PathUTF16))
     return EC;
 
   HANDLE H =
       ::CreateFileW(PathUTF16.begin(), GENERIC_READ,
                     FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
                     NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
   if (H == INVALID_HANDLE_VALUE) {
     DWORD LastError = ::GetLastError();
     std::error_code EC = mapWindowsError(LastError);
     // Provide a better error message when trying to open directories.
     // This only runs if we failed to open the file, so there is probably
     // no performances issues.
     if (LastError != ERROR_ACCESS_DENIED)
       return EC;
     if (is_directory(Name))
       return make_error_code(errc::is_a_directory);
     return EC;
   }
 
-  int FD = ::_open_osfhandle(intptr_t(H), 0);
-  if (FD == -1) {
+  ResultFD = ::_open_osfhandle(intptr_t(H), 0);
+  if (ResultFD == -1) {
     ::CloseHandle(H);
     return mapWindowsError(ERROR_INVALID_HANDLE);
   }
 
   // Fetch the real name of the file, if the user asked
   if (RealPath)
     realPathFromHandle(H, *RealPath);
 
-  ResultFD = FD;
   return std::error_code();
 }
 
 std::error_code openFileForWrite(const Twine &Name, int &ResultFD,
                             sys::fs::OpenFlags Flags, unsigned Mode) {
   // Verify that we don't have both "append" and "excl".
   assert((!(Flags & sys::fs::F_Excl) || !(Flags & sys::fs::F_Append)) &&
          "Cannot specify both 'excl' and 'append' file creation flags!");
 
+  ResultFD = -1;
   SmallVector<wchar_t, 128> PathUTF16;
 
   if (std::error_code EC = widenPath(Name, PathUTF16))
     return EC;
 
   DWORD CreationDisposition;
   if (Flags & F_Excl)
     CreationDisposition = CREATE_NEW;
std::error_code openFileForWrite(const Twine &Name, int &ResultFD,
 
   int OpenFlags = 0;
   if (Flags & F_Append)
     OpenFlags |= _O_APPEND;
 
   if (Flags & F_Text)
     OpenFlags |= _O_TEXT;
 
-  int FD = ::_open_osfhandle(intptr_t(H), OpenFlags);
-  if (FD == -1) {
+  ResultFD = ::_open_osfhandle(intptr_t(H), OpenFlags);
+  if (ResultFD == -1) {
     ::CloseHandle(H);
     return mapWindowsError(ERROR_INVALID_HANDLE);
   }
 
-  ResultFD = FD;
   return std::error_code();
 }
 
 std::error_code remove_directories(const Twine &path, bool IgnoreErrors) {
   // Convert to utf-16.
   SmallVector<wchar_t, 128> Path16;
   std::error_code EC = widenPath(path, Path16);
   if (EC && !IgnoreErrors)