Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "[flang][runtime] Add ACCESS library procedure" #88507

Merged
merged 1 commit into from
Apr 12, 2024

Conversation

tblah
Copy link
Contributor

@tblah tblah commented Apr 12, 2024

Reverts #88395

This broke the powerpc buildbot. That build doesn't support using std::filesystem in flang unit tests.

@llvmbot llvmbot added flang:runtime flang Flang issues not falling into any other category labels Apr 12, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 12, 2024

@llvm/pr-subscribers-flang-runtime

Author: Tom Eccles (tblah)

Changes

Reverts llvm/llvm-project#88395

This broke the powerpc buildbot. That build doesn't support using std::filesystem in flang unit tests.


Full diff: https://github.com/llvm/llvm-project/pull/88507.diff

5 Files Affected:

  • (modified) flang/docs/Intrinsics.md (-8)
  • (modified) flang/include/flang/Runtime/extensions.h (-7)
  • (modified) flang/runtime/extensions.cpp (-73)
  • (removed) flang/unittests/Runtime/AccessTest.cpp (-390)
  • (modified) flang/unittests/Runtime/CMakeLists.txt (-1)
diff --git a/flang/docs/Intrinsics.md b/flang/docs/Intrinsics.md
index 848619cb65d909..ccb93e104dab65 100644
--- a/flang/docs/Intrinsics.md
+++ b/flang/docs/Intrinsics.md
@@ -657,14 +657,6 @@ CALL CO_REDUCE
 CALL CO_SUM
 ```
 
-### Inquiry Functions
-ACCESS (GNU extension) is not supported on Windows. Otherwise:
-```
-CHARACTER(LEN=*) :: path = 'path/to/file'
-IF (ACCESS(path, 'rwx')) &
-  ...
-```
-
 ## Non-standard intrinsics
 ### PGI
 ```
diff --git a/flang/include/flang/Runtime/extensions.h b/flang/include/flang/Runtime/extensions.h
index fef651f3b2eedb..7d0952206fc195 100644
--- a/flang/include/flang/Runtime/extensions.h
+++ b/flang/include/flang/Runtime/extensions.h
@@ -44,12 +44,5 @@ std::int64_t RTNAME(Signal)(std::int64_t number, void (*handler)(int));
 // GNU extension subroutine SLEEP(SECONDS)
 void RTNAME(Sleep)(std::int64_t seconds);
 
-// GNU extension function ACCESS(NAME, MODE)
-// TODO: not supported on Windows
-#ifndef _WIN32
-std::int64_t FORTRAN_PROCEDURE_NAME(access)(const char *name,
-    std::int64_t nameLength, const char *mode, std::int64_t modeLength);
-#endif
-
 } // extern "C"
 #endif // FORTRAN_RUNTIME_EXTENSIONS_H_
diff --git a/flang/runtime/extensions.cpp b/flang/runtime/extensions.cpp
index 12498b502ae1cf..3ac98000335d7d 100644
--- a/flang/runtime/extensions.cpp
+++ b/flang/runtime/extensions.cpp
@@ -17,7 +17,6 @@
 #include "flang/Runtime/entry-names.h"
 #include "flang/Runtime/io-api.h"
 #include <chrono>
-#include <cstring>
 #include <ctime>
 #include <signal.h>
 #include <thread>
@@ -139,77 +138,5 @@ void RTNAME(Sleep)(std::int64_t seconds) {
   std::this_thread::sleep_for(std::chrono::seconds(seconds));
 }
 
-// TODO: not supported on Windows
-#ifndef _WIN32
-std::int64_t FORTRAN_PROCEDURE_NAME(access)(const char *name,
-    std::int64_t nameLength, const char *mode, std::int64_t modeLength) {
-  std::int64_t ret{-1};
-  if (nameLength <= 0 || modeLength <= 0 || !name || !mode) {
-    return ret;
-  }
-
-  // ensure name is null terminated
-  char *newName{nullptr};
-  if (name[nameLength - 1] != '\0') {
-    newName = static_cast<char *>(std::malloc(nameLength + 1));
-    std::memcpy(newName, name, nameLength);
-    newName[nameLength] = '\0';
-    name = newName;
-  }
-
-  // calculate mode
-  bool read{false};
-  bool write{false};
-  bool execute{false};
-  bool exists{false};
-  int imode{0};
-
-  for (std::int64_t i = 0; i < modeLength; ++i) {
-    switch (mode[i]) {
-    case 'r':
-      read = true;
-      break;
-    case 'w':
-      write = true;
-      break;
-    case 'x':
-      execute = true;
-      break;
-    case ' ':
-      exists = true;
-      break;
-    default:
-      // invalid mode
-      goto cleanup;
-    }
-  }
-  if (!read && !write && !execute && !exists) {
-    // invalid mode
-    goto cleanup;
-  }
-
-  if (!read && !write && !execute) {
-    imode = F_OK;
-  } else {
-    if (read) {
-      imode |= R_OK;
-    }
-    if (write) {
-      imode |= W_OK;
-    }
-    if (execute) {
-      imode |= X_OK;
-    }
-  }
-  ret = access(name, imode);
-
-cleanup:
-  if (newName) {
-    free(newName);
-  }
-  return ret;
-}
-#endif
-
 } // namespace Fortran::runtime
 } // extern "C"
diff --git a/flang/unittests/Runtime/AccessTest.cpp b/flang/unittests/Runtime/AccessTest.cpp
deleted file mode 100644
index 5c8a634192d92b..00000000000000
--- a/flang/unittests/Runtime/AccessTest.cpp
+++ /dev/null
@@ -1,390 +0,0 @@
-//===-- flang/unittests/Runtime/AccessTest.cpp ----------------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-
-// TODO: ACCESS is not yet implemented on Windows
-#ifndef _WIN32
-
-#include "CrashHandlerFixture.h"
-#include "gtest/gtest.h"
-#include "flang/Runtime/extensions.h"
-
-#include <fcntl.h>
-#include <filesystem>
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <unistd.h>
-
-namespace {
-
-struct AccessTests : public CrashHandlerFixture {};
-
-struct AccessType {
-  bool read{false};
-  bool write{false};
-  bool execute{false};
-  bool exists{false};
-};
-
-} // namespace
-
-static std::string addPIDSuffix(const char *name) {
-  std::stringstream ss;
-  ss << name;
-  ss << '.';
-
-  ss << getpid();
-
-  return ss.str();
-}
-
-static std::filesystem::path createTemporaryFile(
-    const char *name, const AccessType &accessType) {
-  std::filesystem::path path{
-      std::filesystem::temp_directory_path() / addPIDSuffix(name)};
-
-  // O_CREAT | O_EXCL enforces that this file is newly created by this call.
-  // This feels risky. If we don't have permission to create files in the
-  // temporary directory or if the files already exist, the test will fail.
-  // But we can't use std::tmpfile() because we need a path to the file and
-  // to control the filesystem permissions
-  mode_t mode{0};
-  if (accessType.read) {
-    mode |= S_IRUSR;
-  }
-  if (accessType.write) {
-    mode |= S_IWUSR;
-  }
-  if (accessType.execute) {
-    mode |= S_IXUSR;
-  }
-
-  int file = open(path.c_str(), O_CREAT | O_EXCL, mode);
-  if (file == -1) {
-    return {};
-  }
-
-  close(file);
-
-  return path;
-}
-
-static std::int64_t callAccess(
-    const std::filesystem::path &path, const AccessType &accessType) {
-  const char *cpath{path.c_str()};
-  std::int64_t pathlen = std::strlen(cpath);
-
-  std::string mode;
-  if (accessType.read) {
-    mode += 'r';
-  }
-  if (accessType.write) {
-    mode += 'w';
-  }
-  if (accessType.execute) {
-    mode += 'x';
-  }
-  if (accessType.exists) {
-    mode += ' ';
-  }
-
-  const char *cmode = mode.c_str();
-  std::int64_t modelen = std::strlen(cmode);
-
-  return FORTRAN_PROCEDURE_NAME(access)(cpath, pathlen, cmode, modelen);
-}
-
-TEST(AccessTests, TestExists) {
-  AccessType accessType;
-  accessType.exists = true;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_EQ(res, 0);
-}
-
-TEST(AccessTests, TestNotExists) {
-  std::filesystem::path nonExistant{addPIDSuffix(__func__)};
-  ASSERT_FALSE(std::filesystem::exists(nonExistant));
-
-  AccessType accessType;
-  accessType.exists = true;
-  std::int64_t res = callAccess(nonExistant, accessType);
-
-  ASSERT_NE(res, 0);
-}
-
-TEST(AccessTests, TestRead) {
-  AccessType accessType;
-  accessType.read = true;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_EQ(res, 0);
-}
-
-TEST(AccessTests, TestNotRead) {
-  AccessType accessType;
-  accessType.read = false;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  accessType.read = true;
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_NE(res, 0);
-}
-
-TEST(AccessTests, TestWrite) {
-  AccessType accessType;
-  accessType.write = true;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_EQ(res, 0);
-}
-
-TEST(AccessTests, TestNotWrite) {
-  AccessType accessType;
-  accessType.write = false;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  accessType.write = true;
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_NE(res, 0);
-}
-
-TEST(AccessTests, TestReadWrite) {
-  AccessType accessType;
-  accessType.read = true;
-  accessType.write = true;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_EQ(res, 0);
-}
-
-TEST(AccessTests, TestNotReadWrite0) {
-  AccessType accessType;
-  accessType.read = false;
-  accessType.write = false;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  accessType.read = true;
-  accessType.write = true;
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_NE(res, 0);
-}
-
-TEST(AccessTests, TestNotReadWrite1) {
-  AccessType accessType;
-  accessType.read = true;
-  accessType.write = false;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  accessType.read = true;
-  accessType.write = true;
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_NE(res, 0);
-}
-
-TEST(AccessTests, TestNotReadWrite2) {
-  AccessType accessType;
-  accessType.read = false;
-  accessType.write = true;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  accessType.read = true;
-  accessType.write = true;
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_NE(res, 0);
-}
-
-TEST(AccessTests, TestExecute) {
-  AccessType accessType;
-  accessType.execute = true;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_EQ(res, 0);
-}
-
-TEST(AccessTests, TestNotExecute) {
-  AccessType accessType;
-  accessType.execute = false;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  accessType.execute = true;
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_NE(res, 0);
-}
-
-TEST(AccessTests, TestRWX) {
-  AccessType accessType;
-  accessType.read = true;
-  accessType.write = true;
-  accessType.execute = true;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_EQ(res, 0);
-}
-
-TEST(AccessTests, TestNotRWX0) {
-  AccessType accessType;
-  accessType.read = false;
-  accessType.write = false;
-  accessType.execute = false;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  accessType.read = true;
-  accessType.write = true;
-  accessType.execute = true;
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_NE(res, 0);
-}
-
-TEST(AccessTests, TestNotRWX1) {
-  AccessType accessType;
-  accessType.read = true;
-  accessType.write = false;
-  accessType.execute = false;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  accessType.read = true;
-  accessType.write = true;
-  accessType.execute = true;
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_NE(res, 0);
-}
-
-TEST(AccessTests, TestNotRWX2) {
-  AccessType accessType;
-  accessType.read = true;
-  accessType.write = true;
-  accessType.execute = false;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  accessType.read = true;
-  accessType.write = true;
-  accessType.execute = true;
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_NE(res, 0);
-}
-
-TEST(AccessTests, TestNotRWX3) {
-  AccessType accessType;
-  accessType.read = true;
-  accessType.write = false;
-  accessType.execute = true;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  accessType.read = true;
-  accessType.write = true;
-  accessType.execute = true;
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_NE(res, 0);
-}
-
-TEST(AccessTests, TestNotRWX4) {
-  AccessType accessType;
-  accessType.read = false;
-  accessType.write = true;
-  accessType.execute = true;
-
-  std::filesystem::path path = createTemporaryFile(__func__, accessType);
-  ASSERT_FALSE(path.empty());
-
-  accessType.read = true;
-  accessType.write = true;
-  accessType.execute = true;
-  std::int64_t res = callAccess(path, accessType);
-
-  std::filesystem::remove(path);
-
-  ASSERT_NE(res, 0);
-}
-
-#endif // !_WIN32
diff --git a/flang/unittests/Runtime/CMakeLists.txt b/flang/unittests/Runtime/CMakeLists.txt
index f7caacad3a598f..23f02aa751246b 100644
--- a/flang/unittests/Runtime/CMakeLists.txt
+++ b/flang/unittests/Runtime/CMakeLists.txt
@@ -1,5 +1,4 @@
 add_flang_unittest(FlangRuntimeTests
-  AccessTest.cpp
   Allocatable.cpp
   ArrayConstructor.cpp
   BufferTest.cpp

@tblah tblah merged commit 7e7468c into main Apr 12, 2024
7 of 8 checks passed
@tblah tblah deleted the revert-88395-ecclescake/access branch April 12, 2024 12:50
@tblah
Copy link
Contributor Author

tblah commented Apr 12, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:runtime flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants