From 216999318bc5113c9a67fe43ea8b3df4faef2132 Mon Sep 17 00:00:00 2001 From: peter klausler Date: Fri, 6 Apr 2018 10:34:59 -0700 Subject: [PATCH] [flang] Address review comments. Original-commit: flang-compiler/f18@66ecc10722135e5f9f3f8d68ac6795a4ba5e0727 Reviewed-on: https://github.com/flang-compiler/f18/pull/42 --- flang/lib/parser/idioms.h | 18 ++++++++++++--- flang/lib/parser/parsing.cc | 11 +++++++++- flang/lib/parser/preprocessor.cc | 5 ----- flang/lib/parser/prescan.cc | 4 ---- flang/lib/parser/provenance.cc | 8 +++++++ flang/lib/parser/provenance.h | 1 + flang/lib/parser/source.cc | 47 +++++++++++++++++++++++----------------- flang/lib/parser/source.h | 3 +++ flang/lib/semantics/attr.cc | 10 +++++---- flang/lib/semantics/attr.h | 4 ++-- flang/lib/semantics/enum-set.h | 21 +++++++++--------- 11 files changed, 82 insertions(+), 50 deletions(-) diff --git a/flang/lib/parser/idioms.h b/flang/lib/parser/idioms.h index 0080535..43e6968 100644 --- a/flang/lib/parser/idioms.h +++ b/flang/lib/parser/idioms.h @@ -103,16 +103,28 @@ template struct BadType : std::false_type {}; } \ template constexpr bool T { class_trait_ns_##T::trait_value() } -// 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 struct ListItemCount { + constexpr ListItemCount(std::initializer_list 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(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 --- 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 --- 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 --- 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 --- 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 source{std::make_unique()}; + 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..a33dd973 100644 --- 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 --- 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(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 --- 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 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 --- 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(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 --- 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 { +class Attrs : public EnumSet { private: - using enumSetType = EnumSet; + using enumSetType = EnumSet; 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 --- 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 class EnumSet { -private: - static constexpr std::size_t bits_{static_cast(maxval) + 1}; +template class EnumSet { + static_assert(BITS > 0); public: - using bitsetType = std::bitset; + using bitsetType = std::bitset; 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(x)); } @@ -162,10 +161,10 @@ private: } // namespace semantics } // namespace Fortran -template -struct std::hash> { - std::size_t operator()(const Fortran::semantics::EnumSet &x) const { +template +struct std::hash> { + std::size_t operator()(const Fortran::semantics::EnumSet &x) const { return std::hash(x.bitset()); } }; -#endif // FORTRAN_ENUM_SET_H_ +#endif // FORTRAN_SEMANTICS_ENUM_SET_H_ -- 2.7.4