[flang] More precise enforcement of runtime constraint

An OPEN statement that affects an already connected unit
without changing its external file is required to have
STATUS="OLD" or default STATUS=.  The code was eliciting
spurious runtime errors in situations where an OPEN statement
pertained to an existing unit number but did not need to have
STATUS="OLD'.

Differential Revision: https://reviews.llvm.org/D100352

3 files changed, 20 insertions, 21 deletions

diff --git a/flang/runtime/io-stmt.cpp b/flang/runtime/io-stmt.cpp
index 7474dd94b982..c04013d2d31c 100644
--- a/flang/runtime/io-stmt.cpp
+++ b/flang/runtime/io-stmt.cpp
@@ -188,17 +188,12 @@ void OpenStatementState::set_path(const char *path, std::size_t length) {
 }
 
 int OpenStatementState::EndIoStatement() {
-  if (wasExtant_ && status_ && *status_ != OpenStatus::Old) {
-    SignalError("OPEN statement for connected unit may not have STATUS= other "
-                "than 'OLD'");
-  }
   if (path_.get() || wasExtant_ ||
       (status_ && *status_ == OpenStatus::Scratch)) {
-    unit().OpenUnit(status_.value_or(OpenStatus::Unknown), action_, position_,
-        std::move(path_), pathLength_, convert_, *this);
+    unit().OpenUnit(status_, action_, position_, std::move(path_), pathLength_,
+        convert_, *this);
   } else {
-    unit().OpenAnonymousUnit(status_.value_or(OpenStatus::Unknown), action_,
-        position_, convert_, *this);
+    unit().OpenAnonymousUnit(status_, action_, position_, convert_, *this);
   }
   if (access_) {
     if (*access_ != unit().access) {
diff --git a/flang/runtime/unit.cpp b/flang/runtime/unit.cpp
index 85d83ec50bd9..d5a9f8e53af4 100644
--- a/flang/runtime/unit.cpp
+++ b/flang/runtime/unit.cpp
@@ -89,9 +89,9 @@ int ExternalFileUnit::NewUnit(const Terminator &terminator) {
   return GetUnitMap().NewUnit(terminator).unitNumber();
 }
 
-void ExternalFileUnit::OpenUnit(OpenStatus status, std::optional<Action> action,
-    Position position, OwningPtr<char> &&newPath, std::size_t newPathLength,
-    Convert convert, IoErrorHandler &handler) {
+void ExternalFileUnit::OpenUnit(std::optional<OpenStatus> status,
+    std::optional<Action> action, Position position, OwningPtr<char> &&newPath,
+    std::size_t newPathLength, Convert convert, IoErrorHandler &handler) {
   if (executionEnvironment.conversion != Convert::Unknown) {
     convert = executionEnvironment.conversion;
   }
@@ -99,11 +99,15 @@ void ExternalFileUnit::OpenUnit(OpenStatus status, std::optional<Action> action,
       (convert == Convert::LittleEndian && !isHostLittleEndian) ||
       (convert == Convert::BigEndian && isHostLittleEndian);
   if (IsOpen()) {
-    if (status == OpenStatus::Old &&
-        (!newPath.get() ||
-            (path() && pathLength() == newPathLength &&
-                std::memcmp(path(), newPath.get(), newPathLength) == 0))) {
-      // OPEN of existing unit, STATUS='OLD', not new FILE=
+    bool isSamePath{newPath.get() && path() && pathLength() == newPathLength &&
+        std::memcmp(path(), newPath.get(), newPathLength) == 0};
+    if (status && *status != OpenStatus::Old && isSamePath) {
+      handler.SignalError("OPEN statement for connected unit may not have "
+                          "explicit STATUS= other than 'OLD'");
+      return;
+    }
+    if (!newPath.get() || isSamePath) {
+      // OPEN of existing unit, STATUS='OLD' or unspecified, not new FILE=
       newPath.reset();
       return;
     }
@@ -113,7 +117,7 @@ void ExternalFileUnit::OpenUnit(OpenStatus status, std::optional<Action> action,
     Close(CloseStatus::Keep, handler);
   }
   set_path(std::move(newPath), newPathLength);
-  Open(status, action, position, handler);
+  Open(status.value_or(OpenStatus::Unknown), action, position, handler);
   auto totalBytes{knownSize()};
   if (access == Access::Direct) {
     if (!isFixedRecordLength || !recordLength) {
@@ -146,7 +150,7 @@ void ExternalFileUnit::OpenUnit(OpenStatus status, std::optional<Action> action,
   }
 }
 
-void ExternalFileUnit::OpenAnonymousUnit(OpenStatus status,
+void ExternalFileUnit::OpenAnonymousUnit(std::optional<OpenStatus> status,
     std::optional<Action> action, Position position, Convert convert,
     IoErrorHandler &handler) {
   // I/O to an unconnected unit reads/creates a local file, e.g. fort.7
diff --git a/flang/runtime/unit.h b/flang/runtime/unit.h
index 644ca4ad63cc..ff1506c24109 100644
--- a/flang/runtime/unit.h
+++ b/flang/runtime/unit.h
@@ -50,11 +50,11 @@ public:
   static void CloseAll(IoErrorHandler &);
   static void FlushAll(IoErrorHandler &);
 
-  void OpenUnit(OpenStatus, std::optional<Action>, Position,
+  void OpenUnit(std::optional<OpenStatus>, std::optional<Action>, Position,
       OwningPtr<char> &&path, std::size_t pathLength, Convert,
       IoErrorHandler &);
-  void OpenAnonymousUnit(
-      OpenStatus, std::optional<Action>, Position, Convert, IoErrorHandler &);
+  void OpenAnonymousUnit(std::optional<OpenStatus>, std::optional<Action>,
+      Position, Convert, IoErrorHandler &);
   void CloseUnit(CloseStatus, IoErrorHandler &);
   void DestroyClosed();