[flang] Address review comments.

Original-commit: flang-compiler/f18@66ecc10722135e5f9f3f8d68ac6795a4ba5e0727
Reviewed-on: https://github.com/flang-compiler/f18/pull/42

diff --git a/flang/lib/parser/idioms.h b/flang/lib/parser/idioms.h
index 0080535..43e6968 100644 (file)
--- a/flang/lib/parser/idioms.h
+++ b/flang/lib/parser/idioms.h
@@ -103,16 +103,28 @@ template<typename A> struct BadType : std::false_type {};
   } \
   template<typename A> constexpr bool T { class_trait_ns_##T::trait_value<A>() }
 
-// Define enum class NAME with the given enumerators, and also a
-// static function EnumToString that maps enumerators to std::string.
+// Define enum class NAME with the given enumerators, a static
+// function EnumToString() that maps enumerators to std::string,
+// and a constant NAME_enumSize that captures the number of items
+// in the enum class.
+
 std::string EnumIndexToString(int index, const char *names);
+
+template<typename A> struct ListItemCount {
+  constexpr ListItemCount(std::initializer_list<A> list) : value{list.size()} {}
+  const std::size_t value;
+};
+
 #define ENUM_CLASS(NAME, ...) \
   enum class NAME { __VA_ARGS__ }; \
+  static constexpr std::size_t NAME##_enumSize{[] { \
+    enum { __VA_ARGS__ }; \
+    return Fortran::parser::ListItemCount{__VA_ARGS__}.value; \
+  }()}; \
   static inline std::string EnumToString(NAME e) { \
     return Fortran::parser::EnumIndexToString( \
         static_cast<int>(e), #__VA_ARGS__); \
   }
-
 }  // namespace parser
 }  // namespace Fortran
 #endif  // FORTRAN_PARSER_IDIOMS_H_

diff --git a/flang/lib/parser/parsing.cc b/flang/lib/parser/parsing.cc
index 4764823..84c4b14 100644 (file)
--- a/flang/lib/parser/parsing.cc
+++ b/flang/lib/parser/parsing.cc
@@ -14,7 +14,12 @@ void Parsing::Prescan(const std::string &path, Options options) {
   options_ = options;
 
   std::stringstream fileError;
-  const auto *sourceFile = allSources_.Open(path, &fileError);
+  const SourceFile *sourceFile;
+  if (path == "-") {
+    sourceFile = allSources_.ReadStandardInput(&fileError);
+  } else {
+    sourceFile = allSources_.Open(path, &fileError);
+  }
   if (sourceFile == nullptr) {
     ProvenanceRange range{allSources_.AddCompilerInsertion(path)};
     MessageFormattedText msg("%s"_err_en_US, fileError.str().data());
@@ -27,6 +32,10 @@ void Parsing::Prescan(const std::string &path, Options options) {
     return;
   }
 
+  // N.B. Be sure to not push the search directory paths until the primary
+  // source file has been opened.  If foo.f is missing from the current
+  // working directory, we don't want to accidentally read another foo.f
+  // from another directory that's on the search path.
   for (const auto &path : options.searchDirectories) {
     allSources_.PushSearchPathDirectory(path);
   }

diff --git a/flang/lib/parser/preprocessor.cc b/flang/lib/parser/preprocessor.cc
index f1ef8dd..df15f64 100644 (file)
--- a/flang/lib/parser/preprocessor.cc
+++ b/flang/lib/parser/preprocessor.cc
@@ -569,11 +569,6 @@ void Preprocessor::Directive(const TokenSequence &dir, Prescanner *prescanner) {
           dir.GetTokenProvenance(dirOffset));
       return;
     }
-    if (include == "-") {
-      prescanner->Say("#include: can't include standard input"_err_en_US,
-          dir.GetTokenProvenance(dirOffset));
-      return;
-    }
     std::stringstream error;
     const SourceFile *included{allSources_.Open(include, &error)};
     if (included == nullptr) {

diff --git a/flang/lib/parser/prescan.cc b/flang/lib/parser/prescan.cc
index b4633ea..b3132e1 100644 (file)
--- a/flang/lib/parser/prescan.cc
+++ b/flang/lib/parser/prescan.cc
@@ -565,10 +565,6 @@ void Prescanner::FortranInclude(const char *firstQuote) {
   if (*p != '\n' && *p != '!') {
     Say("excess characters after path name"_en_US, GetProvenance(p));
   }
-  if (path == "-") {
-    Say("cannot INCLUDE standard input"_err_en_US, GetProvenance(p));
-    return;
-  }
   std::stringstream error;
   Provenance provenance{GetProvenance(lineStart_)};
   AllSources &allSources{cooked_.allSources()};

diff --git a/flang/lib/parser/provenance.cc b/flang/lib/parser/provenance.cc
index 76aabee..625020b 100644 (file)
--- a/flang/lib/parser/provenance.cc
+++ b/flang/lib/parser/provenance.cc
@@ -94,6 +94,14 @@ const SourceFile *AllSources::Open(std::string path, std::stringstream *error) {
   return nullptr;
 }
 
+const SourceFile *AllSources::ReadStandardInput(std::stringstream *error) {
+  std::unique_ptr<SourceFile> source{std::make_unique<SourceFile>()};
+  if (source->ReadStandardInput(error)) {
+    return ownedSourceFiles_.emplace_back(std::move(source)).get();
+  }
+  return nullptr;
+}
+
 ProvenanceRange AllSources::AddIncludedFile(
     const SourceFile &source, ProvenanceRange from, bool isModule) {
   ProvenanceRange covers{range_.NextAfter(), source.bytes()};

diff --git a/flang/lib/parser/provenance.h b/flang/lib/parser/provenance.h
index e463b35..a33dd97 100644 (file)
--- a/flang/lib/parser/provenance.h
+++ b/flang/lib/parser/provenance.h
@@ -103,6 +103,7 @@ public:
   void PushSearchPathDirectory(std::string);
   std::string PopSearchPathDirectory();
   const SourceFile *Open(std::string path, std::stringstream *error);
+  const SourceFile *ReadStandardInput(std::stringstream *error);
 
   ProvenanceRange AddIncludedFile(
       const SourceFile &, ProvenanceRange, bool isModule = false);

diff --git a/flang/lib/parser/source.cc b/flang/lib/parser/source.cc
index 65fb018..4fab3c0 100644 (file)
--- a/flang/lib/parser/source.cc
+++ b/flang/lib/parser/source.cc
@@ -88,28 +88,33 @@ static std::size_t RemoveCarriageReturns(char *buffer, std::size_t bytes) {
 bool SourceFile::Open(std::string path, std::stringstream *error) {
   Close();
   path_ = path;
-  std::string error_path;
+  std::string errorPath{"'"s + path + "'"};
   errno = 0;
-  if (path == "-") {
-    error_path = "standard input";
-    fileDescriptor_ = 0;
-  } else {
-    error_path = "'"s + path + "'";
-    fileDescriptor_ = open(path.c_str(), O_RDONLY);
-    if (fileDescriptor_ < 0) {
-      *error << "could not open " << error_path << ": " << std::strerror(errno);
-      return false;
-    }
-    ++openFileDescriptors;
+  fileDescriptor_ = open(path.c_str(), O_RDONLY);
+  if (fileDescriptor_ < 0) {
+    *error << "could not open " << errorPath << ": " << std::strerror(errno);
+    return false;
   }
+  ++openFileDescriptors;
+  return ReadFile(errorPath, error);
+}
+
+bool SourceFile::ReadStandardInput(std::stringstream *error) {
+  Close();
+  path_ = "standard input";
+  fileDescriptor_ = 0;
+  return ReadFile(path_, error);
+}
+
+bool SourceFile::ReadFile(std::string errorPath, std::stringstream *error) {
   struct stat statbuf;
   if (fstat(fileDescriptor_, &statbuf) != 0) {
-    *error << "fstat failed on " << error_path << ": " << std::strerror(errno);
+    *error << "fstat failed on " << errorPath << ": " << std::strerror(errno);
     Close();
     return false;
   }
   if (S_ISDIR(statbuf.st_mode)) {
-    *error << error_path << " is a directory";
+    *error << errorPath << " is a directory";
     Close();
     return false;
   }
@@ -133,7 +138,7 @@ bool SourceFile::Open(std::string path, std::stringstream *error) {
         // The file needs to have its line endings normalized to simple
         // newlines.  Remap it for a private rewrite in place.
         vp = mmap(vp, bytes_, PROT_READ | PROT_WRITE, MAP_PRIVATE,
-                  fileDescriptor_, 0);
+            fileDescriptor_, 0);
         if (vp != MAP_FAILED) {
           auto mutableContent = static_cast<char *>(vp);
           bytes_ = RemoveCarriageReturns(mutableContent, bytes_);
@@ -167,7 +172,7 @@ bool SourceFile::Open(std::string path, std::stringstream *error) {
     char *to{buffer.FreeSpace(&count)};
     ssize_t got{read(fileDescriptor_, to, count)};
     if (got < 0) {
-      *error << "could not read " << error_path << ": " << std::strerror(errno);
+      *error << "could not read " << errorPath << ": " << std::strerror(errno);
       Close();
       return false;
     }
@@ -176,8 +181,10 @@ bool SourceFile::Open(std::string path, std::stringstream *error) {
     }
     buffer.Claim(got);
   }
-  close(fileDescriptor_);
-  --openFileDescriptors;
+  if (fileDescriptor_ > 0) {
+    close(fileDescriptor_);
+    --openFileDescriptors;
+  }
   fileDescriptor_ = -1;
   bytes_ = buffer.size();
   if (bytes_ == 0) {
@@ -211,11 +218,11 @@ void SourceFile::Close() {
   }
   content_ = nullptr;
   bytes_ = 0;
-  if (fileDescriptor_ >= 0) {
+  if (fileDescriptor_ > 0) {
     close(fileDescriptor_);
     --openFileDescriptors;
-    fileDescriptor_ = -1;
   }
+  fileDescriptor_ = -1;
   path_.clear();
 }
 

diff --git a/flang/lib/parser/source.h b/flang/lib/parser/source.h
index 9ee176a..b919388 100644 (file)
--- a/flang/lib/parser/source.h
+++ b/flang/lib/parser/source.h
@@ -28,6 +28,7 @@ public:
   std::size_t lines() const { return lineStart_.size(); }
 
   bool Open(std::string path, std::stringstream *error);
+  bool ReadStandardInput(std::stringstream *error);
   void Close();
   std::pair<int, int> FindOffsetLineAndColumn(std::size_t) const;
   std::size_t GetLineStartOffset(int lineNumber) const {
@@ -35,6 +36,8 @@ public:
   }
 
 private:
+  bool ReadFile(std::string errorPath, std::stringstream *error);
+
   std::string path_;
   int fileDescriptor_{-1};
   bool isMemoryMapped_{false};

diff --git a/flang/lib/semantics/attr.cc b/flang/lib/semantics/attr.cc
index 4708e57..d926c87 100644 (file)
--- a/flang/lib/semantics/attr.cc
+++ b/flang/lib/semantics/attr.cc
@@ -16,13 +16,15 @@ std::ostream &operator<<(std::ostream &o, Attr attr) {
 }
 
 std::ostream &operator<<(std::ostream &o, const Attrs &attrs) {
-  const char *comma{""};
-  std::size_t n{attrs.count()}, seen{0};
+  std::size_t n{attrs.count()};
+  std::size_t seen{0};
   for (std::size_t j{0}; seen < n; ++j) {
     Attr attr{static_cast<Attr>(j)};
     if (attrs.test(attr)) {
-      o << comma << attr;
-      comma = ", ";
+      if (seen > 0) {
+        o << ", ";
+      }
+      o << attr;
       ++seen;
     }
   }

diff --git a/flang/lib/semantics/attr.h b/flang/lib/semantics/attr.h
index 37982d2..074fed7 100644 (file)
--- a/flang/lib/semantics/attr.h
+++ b/flang/lib/semantics/attr.h
@@ -18,9 +18,9 @@ ENUM_CLASS(Attr, ABSTRACT, ALLOCATABLE, ASYNCHRONOUS, BIND_C, CONTIGUOUS,
     VOLATILE)
 
 // Set of attributes
-class Attrs : public EnumSet<Attr, Attr::VOLATILE> {
+class Attrs : public EnumSet<Attr, Attr_enumSize> {
 private:
-  using enumSetType = EnumSet<Attr, Attr::VOLATILE>;
+  using enumSetType = EnumSet<Attr, Attr_enumSize>;
 public:
   using enumSetType::enumSetType;
   constexpr bool HasAny(const Attrs &x) const {

diff --git a/flang/lib/semantics/enum-set.h b/flang/lib/semantics/enum-set.h
index 6fbd696..d222595 100644 (file)
--- a/flang/lib/semantics/enum-set.h
+++ b/flang/lib/semantics/enum-set.h
@@ -1,5 +1,5 @@
-#ifndef FORTRAN_ENUM_SET_H_
-#define FORTRAN_ENUM_SET_H_
+#ifndef FORTRAN_SEMANTICS_ENUM_SET_H_
+#define FORTRAN_SEMANTICS_ENUM_SET_H_
 
 // Implements a set of enums as a std::bitset<>.  APIs from bitset<> and set<>
 // can be used on these sets, whichever might be more clear to the user.
@@ -11,11 +11,10 @@
 namespace Fortran {
 namespace semantics {
 
-template<typename ENUM, ENUM maxval> class EnumSet {
-private:
-  static constexpr std::size_t bits_{static_cast<std::size_t>(maxval) + 1};
+template<typename ENUM, std::size_t BITS> class EnumSet {
+  static_assert(BITS > 0);
 public:
-  using bitsetType = std::bitset<bits_>;
+  using bitsetType = std::bitset<BITS>;
   using enumerationType = ENUM;
 
   constexpr EnumSet() {}
@@ -108,7 +107,7 @@ public:
 
   // N.B. std::bitset<> has size() for max_size(), but that's not the same
   // thing as std::set<>::size(), which is an element count.
-  static constexpr std::size_t max_size() { return bits_; }
+  static constexpr std::size_t max_size() { return BITS; }
   constexpr bool test(enumerationType x) const {
     return bitset_.test(static_cast<std::size_t>(x));
   }
@@ -162,10 +161,10 @@ private:
 }  // namespace semantics
 }  // namespace Fortran
 
-template<typename ENUM, ENUM maxval>
-struct std::hash<Fortran::semantics::EnumSet<ENUM, maxval>> {
-  std::size_t operator()(const Fortran::semantics::EnumSet<ENUM, maxval> &x) const {
+template<typename ENUM, std::size_t values>
+struct std::hash<Fortran::semantics::EnumSet<ENUM, values>> {
+  std::size_t operator()(const Fortran::semantics::EnumSet<ENUM, values> &x) const {
     return std::hash(x.bitset());
   }
 };
-#endif  // FORTRAN_ENUM_SET_H_
+#endif  // FORTRAN_SEMANTICS_ENUM_SET_H_