[flang] Address review comments

Original-commit: flang-compiler/f18@d96917c701
Reviewed-on: https://github.com/flang-compiler/f18/pull/101
Tree-same-pre-rewrite: false
This commit is contained in:
peter klausler 2018-06-14 13:43:02 -07:00
parent b19a9baba4
commit 57f4186ca2
13 changed files with 83 additions and 65 deletions

View File

@ -94,6 +94,8 @@ include_directories(BEFORE
${FLANG_SOURCE_DIR}/include
)
enable_testing()
add_subdirectory(include/flang)
add_subdirectory(lib)
add_subdirectory(runtime)
@ -103,10 +105,3 @@ add_subdirectory(tools)
configure_file(
${FLANG_SOURCE_DIR}/include/flang/Config/config.h.cmake
${FLANG_BINARY_DIR}/include/flang/Config/config.h)
enable_testing()
add_test(NAME Leadz COMMAND leading-zero-bit-count-test)
add_test(NAME PopPar COMMAND bit-population-count-test)
add_test(NAME Integer COMMAND integer-test)
add_test(NAME Logical COMMAND logical-test)
add_test(NAME Real COMMAND real-test)

View File

@ -12,8 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
#ifndef FORTRAN_SEMANTICS_ENUM_SET_H_
#define FORTRAN_SEMANTICS_ENUM_SET_H_
#ifndef FORTRAN_COMMON_ENUM_SET_H_
#define FORTRAN_COMMON_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.
@ -21,7 +21,7 @@
#include <bitset>
#include <cstddef>
namespace Fortran::semantics {
namespace Fortran::common {
template<typename ENUM, std::size_t BITS> class EnumSet {
static_assert(BITS > 0);
@ -172,13 +172,13 @@ private:
bitsetType bitset_;
};
} // namespace Fortran::semantics
} // namespace Fortran::common
template<typename ENUM, std::size_t values>
struct std::hash<Fortran::semantics::EnumSet<ENUM, values>> {
struct std::hash<Fortran::common::EnumSet<ENUM, values>> {
std::size_t operator()(
const Fortran::semantics::EnumSet<ENUM, values> &x) const {
const Fortran::common::EnumSet<ENUM, values> &x) const {
return std::hash(x.bitset());
}
};
#endif // FORTRAN_SEMANTICS_ENUM_SET_H_
#endif // FORTRAN_COMMON_ENUM_SET_H_

View File

@ -17,4 +17,4 @@ add_library(FortranEvaluate
integer.cc
logical.cc
real.cc
)
)

View File

@ -15,7 +15,7 @@
#ifndef FORTRAN_EVALUATE_COMMON_H_
#define FORTRAN_EVALUATE_COMMON_H_
#include "../semantics/enum-set.h"
#include "../common/enum-set.h"
#include <cinttypes>
namespace Fortran::evaluate {
@ -72,7 +72,7 @@ enum class RealFlag {
Inexact
};
using RealFlags = semantics::EnumSet<RealFlag, 5>;
using RealFlags = common::EnumSet<RealFlag, 5>;
template<typename A> struct ValueWithRealFlags {
A AccumulateFlags(RealFlags &f) {

View File

@ -47,12 +47,13 @@ namespace Fortran::evaluate::value {
// of possible values in a part), however, must be a power of two; this
// template class is not generalized to enable, say, decimal arithmetic.
// Member functions that correspond to Fortran intrinsic functions are
// named accordingly so that they can be referenced easily in the
// language standard.
// named accordingly in ALL CAPS so that they can be referenced easily in
// the language standard.
template<int BITS, bool IS_LITTLE_ENDIAN = IsHostLittleEndian,
int PARTBITS =
BITS<32 ? BITS : 32, typename PART = HostUnsignedInt<PARTBITS>,
typename BIGPART = HostUnsignedInt<PARTBITS * 2>> class Integer {
int PARTBITS = BITS <= 32 ? BITS : 32,
typename PART = HostUnsignedInt<PARTBITS>,
typename BIGPART = HostUnsignedInt<PARTBITS * 2>>
class Integer {
public:
static constexpr int bits{BITS};
static constexpr int partBits{PARTBITS};
@ -122,6 +123,20 @@ public:
}
SetLEPart(parts - 1, n);
}
constexpr Integer(int ni) {
std::int64_t n{ni};
std::int64_t signExtension{-(n < 0)};
signExtension <<= partBits;
for (int j{0}; j + 1 < parts; ++j) {
SetLEPart(j, n);
if constexpr (partBits < 64) {
n = (n >> partBits) | signExtension;
} else {
n = signExtension;
}
}
SetLEPart(parts - 1, n);
}
constexpr Integer &operator=(const Integer &) = default;
@ -172,6 +187,14 @@ public:
++p;
}
Integer radix{base};
// This code makes assumptions about local contiguity in regions of the
// character set and only works up to base 36. These assumptions hold
// for all current combinations of surviving character sets (ASCII, UTF-8,
// EBCDIC) and the bases used in Fortran source and formatted I/O
// (viz., 2, 8, 10, & 16). But: management thought that a disclaimer
// might be needed here to warn future users of this code about these
// assumptions, so here you go, future programmer in some postapocalyptic
// hellscape, and best of luck with the inexorable killer robots.
for (; std::uint64_t digit = *p; ++p) {
if (digit >= '0' && digit < '0' + base) {
digit -= '0';
@ -197,23 +220,16 @@ public:
std::uint64_t field{that.ToUInt64()};
ValueWithOverflow result{field, false};
if constexpr (bits < 64) {
if ((field >> bits) != 0) {
result.overflow = true;
return result;
}
result.overflow = (field >> bits) != 0;
}
for (int j{64}; j < that.bits; j += 64) {
for (int j{64}; j < that.bits && !result.overflow; j += 64) {
field = that.SHIFTR(j).ToUInt64();
if (bits <= j) {
if (field != 0) {
result.overflow = true;
break;
}
result.overflow = field != 0;
} else {
result.value = result.value.IOR(Integer{field}.SHIFTL(j));
if (bits < j + 64 && (field >> (bits - j)) != 0) {
result.overflow = true;
break;
if (bits < j + 64) {
result.overflow = (field >> (bits - j)) != 0;
}
}
}
@ -445,7 +461,8 @@ public:
if (size > bits) {
size = bits;
}
if ((count %= size) == 0) {
count %= size;
if (count == 0) {
return *this;
}
int middleBits{size - count}, leastBits{count};

View File

@ -77,15 +77,11 @@ public:
}
constexpr Real ABS() const { // non-arithmetic, no flags returned
Real result;
result.word_ = word_.IBCLR(bits - 1);
return result;
return {word_.IBCLR(bits - 1)};
}
constexpr Real Negate() const {
Real result;
result.word_ = word_.IEOR(word_.MASKL(1));
return result;
return {word_.IEOR(word_.MASKL(1))};
}
Relation Compare(const Real &) const;

View File

@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
// TODO: move to lib/common
#ifndef FORTRAN_PARSER_IDIOMS_H_
#define FORTRAN_PARSER_IDIOMS_H_

View File

@ -15,7 +15,7 @@
#ifndef FORTRAN_SEMANTICS_ATTR_H_
#define FORTRAN_SEMANTICS_ATTR_H_
#include "enum-set.h"
#include "../common/enum-set.h"
#include "../parser/idioms.h"
#include <cinttypes>
#include <iostream>
@ -31,9 +31,9 @@ ENUM_CLASS(Attr, ABSTRACT, ALLOCATABLE, ASYNCHRONOUS, BIND_C, CONTIGUOUS,
VOLATILE)
// Set of attributes
class Attrs : public EnumSet<Attr, Attr_enumSize> {
class Attrs : public common::EnumSet<Attr, Attr_enumSize> {
private:
using enumSetType = EnumSet<Attr, Attr_enumSize>;
using enumSetType = common::EnumSet<Attr, Attr_enumSize>;
public:
using enumSetType::enumSetType;

View File

@ -497,7 +497,8 @@ private:
void DeclareProcEntity(const parser::Name &, Attrs, ProcInterface &&);
// Set the type of an entity or report an error.
void SetType(const SourceName &name, Symbol &symbol, const DeclTypeSpec &type);
void SetType(
const SourceName &name, Symbol &symbol, const DeclTypeSpec &type);
// Declare an object or procedure entity.
template<typename T>
@ -537,7 +538,6 @@ private:
}
return symbol;
}
};
// Walk the parse tree and resolve names to symbols.
@ -547,6 +547,8 @@ class ResolveNamesVisitor : public ModuleVisitor,
public:
using ArraySpecVisitor::Post;
using ArraySpecVisitor::Pre;
using DeclarationVisitor::Post;
using DeclarationVisitor::Pre;
using ImplicitRulesVisitor::Post;
using ImplicitRulesVisitor::Pre;
using InterfaceVisitor::Post;
@ -555,8 +557,6 @@ public:
using ModuleVisitor::Pre;
using SubprogramVisitor::Post;
using SubprogramVisitor::Pre;
using DeclarationVisitor::Post;
using DeclarationVisitor::Pre;
// Default action for a parse tree node is to visit children.
template<typename T> bool Pre(const T &) { return true; }
@ -736,7 +736,7 @@ void DeclTypeSpecVisitor::Post(const parser::TypeParamSpec &x) {
bool DeclTypeSpecVisitor::Pre(const parser::TypeParamValue &x) {
typeParamValue_ = std::make_unique<ParamValue>(std::visit(
parser::visitors{
//TODO: create IntExpr from ScalarIntExpr
// TODO: create IntExpr from ScalarIntExpr
[&](const parser::ScalarIntExpr &x) { return Bound{IntExpr{}}; },
[&](const parser::Star &x) { return Bound::ASSUMED; },
[&](const parser::TypeParamValue::Deferred &x) {
@ -865,7 +865,7 @@ MessageHandler::Message &MessageHandler::Say(const SourceName &location,
void MessageHandler::SayAlreadyDeclared(
const SourceName &name, const Symbol &prev) {
Say2(name, "'%s' is already declared in this scoping unit"_err_en_US,
prev.name(), "Previous declaration of '%s'"_en_US);
prev.name(), "Previous declaration of '%s'"_en_US);
}
void MessageHandler::Say2(const SourceName &name1, MessageFixedText &&msg1,
const SourceName &name2, MessageFixedText &&msg2) {
@ -1312,7 +1312,8 @@ bool InterfaceVisitor::Pre(const parser::GenericSpec &x) {
genericSymbol_ = &MakeSymbol(ultimate.name(), ultimate.attrs());
if (const auto *details = ultimate.detailsIf<GenericDetails>()) {
genericSymbol_->set_details(GenericDetails{details->specificProcs()});
} else if (const auto *details = ultimate.detailsIf<SubprogramDetails>()) {
} else if (const auto *details =
ultimate.detailsIf<SubprogramDetails>()) {
genericSymbol_->set_details(SubprogramDetails{*details});
} else {
CHECK(!"can't happen");
@ -1325,8 +1326,8 @@ bool InterfaceVisitor::Pre(const parser::GenericSpec &x) {
}
if (genericSymbol_->has<GenericDetails>()) {
// okay
} else if (genericSymbol_->has<SubprogramDetails>()
|| genericSymbol_->has<SubprogramNameDetails>()) {
} else if (genericSymbol_->has<SubprogramDetails>() ||
genericSymbol_->has<SubprogramNameDetails>()) {
Details details;
if (auto *d = genericSymbol_->detailsIf<SubprogramNameDetails>()) {
details = *d;
@ -1684,8 +1685,7 @@ bool DeclarationVisitor::Pre(const parser::ExternalStmt &x) {
symbol->set_details(ProcEntityDetails(*details));
symbol->set(Symbol::Flag::Function);
} else {
Say2(name.source,
"EXTERNAL attribute not allowed on '%s'"_err_en_US,
Say2(name.source, "EXTERNAL attribute not allowed on '%s'"_err_en_US,
symbol->name(), "Declaration of '%s'"_en_US);
}
}
@ -1777,7 +1777,7 @@ bool DeclarationVisitor::Pre(const parser::DerivedTypeDef &x) {
}
void DeclarationVisitor::Post(const parser::DerivedTypeDef &x) {
DerivedTypeDef derivedType{*derivedTypeData_};
//TODO: do something with derivedType
// TODO: do something with derivedType
derivedTypeData_.reset();
}
bool DeclarationVisitor::Pre(const parser::DerivedTypeStmt &x) {

View File

@ -205,7 +205,6 @@ std::ostream &operator<<(std::ostream &os, const ProcEntityDetails &x) {
return os;
}
static std::ostream &DumpType(std::ostream &os, const Symbol &symbol) {
if (const auto *details = symbol.detailsIf<EntityDetails>()) {
if (details->type()) {

View File

@ -85,6 +85,7 @@ public:
SubprogramNameDetails(SubprogramKind kind) : kind_{kind} {}
SubprogramNameDetails() = delete;
SubprogramKind kind() const { return kind_; }
private:
SubprogramKind kind_;
};
@ -130,7 +131,9 @@ public:
const ProcInterface &interface() const { return interface_; }
ProcInterface &interface() { return interface_; }
void set_interface(ProcInterface &&interface) { interface_ = std::move(interface); }
void set_interface(ProcInterface &&interface) {
interface_ = std::move(interface);
}
bool HasExplicitInterface() const;
private:
@ -201,15 +204,15 @@ private:
class UnknownDetails {};
using Details = std::variant<UnknownDetails, MainProgramDetails, ModuleDetails,
SubprogramDetails, SubprogramNameDetails, EntityDetails,
ObjectEntityDetails, ProcEntityDetails, UseDetails, UseErrorDetails,
GenericDetails>;
SubprogramDetails, SubprogramNameDetails, EntityDetails,
ObjectEntityDetails, ProcEntityDetails, UseDetails, UseErrorDetails,
GenericDetails>;
std::ostream &operator<<(std::ostream &, const Details &);
class Symbol {
public:
ENUM_CLASS(Flag, Function, Subroutine);
using Flags = EnumSet<Flag, Flag_enumSize>;
using Flags = common::EnumSet<Flag, Flag_enumSize>;
Symbol(const Scope &owner, const SourceName &name, const Attrs &attrs,
Details &&details)

View File

@ -412,7 +412,8 @@ private:
class ProcComponentDef {
public:
ProcComponentDef(const ProcDecl &decl, Attrs attrs, ProcInterface &&interface);
ProcComponentDef(
const ProcDecl &decl, Attrs attrs, ProcInterface &&interface);
const ProcDecl &decl() const { return decl_; }
const Attrs &attrs() const { return attrs_; }
@ -475,8 +476,7 @@ public:
}
private:
GenericSpec(Kind kind, const SourceName *name)
: kind_{kind}, name_{name} {}
GenericSpec(Kind kind, const SourceName *name) : kind_{kind}, name_{name} {}
const Kind kind_;
const SourceName *const name_; // only for GENERIC_NAME & OP_DEFINED
friend std::ostream &operator<<(std::ostream &, const GenericSpec &);
@ -572,6 +572,7 @@ public:
}
};
explicit DerivedTypeDef(const Data &x);
private:
const Data data_;
// TODO: type-bound procedures

View File

@ -62,3 +62,9 @@ target_link_libraries(real-test
FortranEvaluateTesting
m
)
add_test(NAME Leadz COMMAND leading-zero-bit-count-test)
add_test(NAME PopPar COMMAND bit-population-count-test)
add_test(NAME Integer COMMAND integer-test)
add_test(NAME Logical COMMAND logical-test)
add_test(NAME Real COMMAND real-test)