Skip to content

Commit

Permalink
Restructure headers and includes in afw.table.
Browse files Browse the repository at this point in the history 
This commit guards against one-definition rule violations involving
explicit specializations of the Key, KeyBase, and FieldBase classes by
moving those specializations next to the default definitions of those
templates and generally making includes more "self-sufficient":
headers should now (at least mostly) include the headers for all
classes they use directly, and should not rely on other headers being
included for them in any source files (but that is always difficult to
check).  In a few cases I do assume that some headers (e.g. Schema.h)
can be relied upon to include others (Key.h, Field.h) in higher-level
code.

This also removes a few includes that were not being used in those
headers at all.  That could cause downstream compile failures if
source files or other headers were (incorrectly) relying on those
incorrect includes, but such breakage should be easy to fix and is
worth fixing anyway.
  • Loading branch information
@TallJimbo
TallJimbo committed Feb 6, 2021
1 parent b73402e commit b53abcb
Show file tree
Hide file tree
Showing 19 changed files with 249 additions and 222 deletions.
2 changes: 2 additions & 0 deletions include/lsst/afw/table/AliasMap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include <map>
#include <string>

#include "lsst/afw/table/fwd.h"

namespace lsst {
namespace afw {
namespace table {
Expand Down
1 change: 1 addition & 0 deletions include/lsst/afw/table/Field.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#ifndef AFW_TABLE_Field_h_INCLUDED
#define AFW_TABLE_Field_h_INCLUDED

#include <string>
#include <iostream>

#include "lsst/afw/table/FieldBase.h"
Expand Down
51 changes: 43 additions & 8 deletions include/lsst/afw/table/FieldBase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,13 @@
#ifndef AFW_TABLE_FieldBase_h_INCLUDED
#define AFW_TABLE_FieldBase_h_INCLUDED

#include <cstring>
#include <string>
#include <iostream>

#include "boost/mpl/vector.hpp"
#include "boost/preprocessor/punctuation/paren.hpp"
#include "Eigen/Core"
#include "ndarray.h"

#include "lsst/base.h"
#include "lsst/pex/exceptions.h"
#include "ndarray.h"
#include "lsst/afw/table/misc.h"
#include "lsst/afw/table/KeyBase.h"
#include "lsst/afw/table/types.h"

namespace lsst {
namespace afw {
Expand Down Expand Up @@ -291,6 +285,47 @@ struct FieldBase<std::string> {
private:
int _size;
};

/**
* Specialization for Flag fields.
*
* Flag fields are handled specially in many places, because their keys have both an offset into an
* integer element and the bit in that element; while other fields have one or more elements per field,
* Flags have multiple fields per element. This means we can't put all the custom code for Flag in
* FieldBase, and because Flags have an explicit Key specialization, we put the record access
* implementation in Key.
*/
template <>
struct FieldBase<Flag> {
typedef bool Value; ///< the type returned by BaseRecord::get
typedef std::int64_t Element; ///< the actual storage type (shared by multiple flag fields)

/// Return the number of subfield elements (always one for scalars).
int getElementCount() const { return 1; }

/// Return a string description of the field type.
static std::string getTypeString() { return "Flag"; }

// Only the first of these constructors is valid for this specializations, but
// it's convenient to be able to instantiate both, since the other is used
// by other specializations.
FieldBase() = default;
FieldBase(int) {
throw LSST_EXCEPT(lsst::pex::exceptions::LogicError,
"Constructor disabled (this Field type is not sized).");
}

FieldBase(FieldBase const &) = default;
FieldBase(FieldBase &&) = default;
FieldBase &operator=(FieldBase const &) = default;
FieldBase &operator=(FieldBase &&) = default;
~FieldBase() = default;

protected:
/// Defines how fields are printed.
void stream(std::ostream &os) const {}
};

} // namespace table
} // namespace afw
} // namespace lsst
Expand Down
193 changes: 6 additions & 187 deletions include/lsst/afw/table/Flag.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,195 +2,14 @@
#ifndef LSST_AFW_TABLE_Flag_h_INCLUDED
#define LSST_AFW_TABLE_Flag_h_INCLUDED

#include <cstdint>
// This is a backwards-compatibility header; the template specializations for
// flag have now been included in the same headers as the default definitions
// of those templates (included below). This guards against implicit
// instantiation of the default templates for Flag, which would be a violation
// of the One Definition Rule.

#include "lsst/utils/hashCombine.h"

#include "lsst/afw/table/misc.h"
#include "lsst/afw/table/FieldBase.h"
#include "lsst/afw/table/KeyBase.h"

namespace lsst {
namespace afw {
namespace table {

namespace detail {

class Access;

} // namespace detail

/**
* Specialization for Flag fields.
*
* Flag fields are handled specially in many places, because their keys have both an offset into an
* integer element and the bit in that element; while other fields have one or more elements per field,
* Flags have multiple fields per element. This means we can't put all the custom code for Flag in
* FieldBase, and because Flags have an explicit Key specialization, we put the record access
* implementation in Key.
*/
template <>
struct FieldBase<Flag> {
typedef bool Value; ///< the type returned by BaseRecord::get
typedef std::int64_t Element; ///< the actual storage type (shared by multiple flag fields)

/// Return the number of subfield elements (always one for scalars).
int getElementCount() const { return 1; }

/// Return a string description of the field type.
static std::string getTypeString() { return "Flag"; }

// Only the first of these constructors is valid for this specializations, but
// it's convenient to be able to instantiate both, since the other is used
// by other specializations.
FieldBase() = default;
FieldBase(int) {
throw LSST_EXCEPT(lsst::pex::exceptions::LogicError,
"Constructor disabled (this Field type is not sized).");
}

FieldBase(FieldBase const &) = default;
FieldBase(FieldBase &&) = default;
FieldBase &operator=(FieldBase const &) = default;
FieldBase &operator=(FieldBase &&) = default;
~FieldBase() = default;

protected:
/// Defines how fields are printed.
void stream(std::ostream &os) const {}
};

/**
* A base class for Key that allows the underlying storage field to be extracted.
*/
template <>
class KeyBase<Flag> {
public:
static bool const HAS_NAMED_SUBFIELDS = false;

/// Return a key corresponding to the integer element where this field's bit is packed.
Key<FieldBase<Flag>::Element> getStorage() const;

KeyBase() = default;
KeyBase(KeyBase const &) = default;
KeyBase(KeyBase &&) = default;
KeyBase &operator=(KeyBase const &) = default;
KeyBase &operator=(KeyBase &&) = default;
~KeyBase() = default;
};

/**
* Key specialization for Flag.
*
* Flag fields are special; their keys need to contain not only the offset to the
* integer element they share with other Flag fields, but also their position
* in that shared field.
*
* Flag fields operate mostly like a bool field, but they do not support reference
* access, and internally they are packed into an integer shared by multiple fields
* so the marginal cost of each Flag field is only one bit.
*/
template <>
class Key<Flag> : public KeyBase<Flag>, public FieldBase<Flag> {
public:
//@{
/**
* Equality comparison.
*
* Two keys with different types are never equal. Keys with the same type
* are equal if they point to the same location in a table, regardless of
* what Schema they were constructed from (for instance, if a field has a
* different name in one Schema than another, but is otherwise the same,
* the two keys will be equal).
*/
template <typename OtherT>
bool operator==(Key<OtherT> const &other) const {
return false;
}
template <typename OtherT>
bool operator!=(Key<OtherT> const &other) const {
return true;
}

bool operator==(Key const &other) const { return _offset == other._offset && _bit == other._bit; }
bool operator!=(Key const &other) const { return !this->operator==(other); }
//@}

/// Return a hash of this object.
std::size_t hash_value() const noexcept {
// Completely arbitrary seed
return utils::hashCombine(17, _offset, _bit);
}

/// Return the offset in bytes of the integer element that holds this field's bit.
int getOffset() const { return _offset; }

/// The index of this field's bit within the integer it shares with other Flag fields.
int getBit() const { return _bit; }

/**
* Return true if the key was initialized to valid offset.
*
* This does not guarantee that a key is valid with any particular schema, or even
* that any schemas still exist in which this key is valid.
*
* A key that is default constructed will always be invalid.
*/
bool isValid() const { return _offset >= 0; }

/**
* Default construct a field.
*
* The new field will be invalid until a valid Key is assigned to it.
*/
Key() : FieldBase<Flag>(), _offset(-1), _bit(0) {}

Key(Key const &) = default;
Key(Key &&) = default;
Key &operator=(Key const &) = default;
Key &operator=(Key &&) = default;
~Key() = default;

/// Stringification.
inline friend std::ostream &operator<<(std::ostream &os, Key<Flag> const &key) {
return os << "Key['" << Key<Flag>::getTypeString() << "'](offset=" << key.getOffset()
<< ", bit=" << key.getBit() << ")";
}

private:
friend class detail::Access;
friend class BaseRecord;

/// Used to implement BaseRecord::get.
Value getValue(Element const *p, ndarray::Manager::Ptr const &) const {
return (*p) & (Element(1) << _bit);
}

/// Used to implement BaseRecord::set.
void setValue(Element *p, ndarray::Manager::Ptr const &, Value v) const {
if (v) {
*p |= (Element(1) << _bit);
} else {
*p &= ~(Element(1) << _bit);
}
}

explicit Key(int offset, int bit) : _offset(offset), _bit(bit) {}

int _offset;
int _bit;
};
} // namespace table
} // namespace afw
} // namespace lsst

namespace std {
template <>
struct hash<lsst::afw::table::Key<lsst::afw::table::Flag>> {
using argument_type = lsst::afw::table::Key<lsst::afw::table::Flag>;
using result_type = size_t;
size_t operator()(argument_type const &obj) const noexcept { return obj.hash_value(); }
};
} // namespace std
#include "lsst/afw/table/Key.h"

#endif // !LSST_AFW_TABLE_Flag_h_INCLUDED
Loading

0 comments on commit b53abcb

Please sign in to comment.