Skip to content

Commit

Permalink
Remove custom type system (#1172)
Browse files Browse the repository at this point in the history
* Remove the custom typing system of casacore

Casacore was using a manual system to identify runtime type information,
using many virtual type() methods and Register() overloads taking
dummy variables. All this functionality was easy to replace with
'normal' run-time type info using dynamic_cast or typeid(), saving
a lot of code, and improving performance in several places.

Removing these type() and variant methods highlighted several bugs
with measures like mDirection where it was
supposed to call mDirection.getRefPtr()->getType()
(which returns an enum describing something like ITRF), but
instead called mDirection.type(), returning an int that
was assigned to the MDirection type by the custom type system.

Many classes still have an 'assure()' method that is supposed
to throw an exception when the object is not of a specified
type. These are now useless, as calls can just perform
dynamic_cast<T&>, which throws when the type is not of the
specified type. However, I left those here for now.

* Fix include

* Fix merge error

* Fix includes for use of atomic

* Review comments
  • Loading branch information
aroffringa committed Mar 16, 2022
1 parent ff44f07 commit 25a015b
Show file tree
Hide file tree
Showing 72 changed files with 73 additions and 837 deletions.
4 changes: 0 additions & 4 deletions casa/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,6 @@ Utilities/Notice.cc
Utilities/Precision.cc
Utilities/RecordTransformable.cc
Utilities/Regex.cc
Utilities/RegSequence.cc
Utilities/Sequence2.cc
Utilities/Sort.cc
Utilities/SortError.cc
Expand Down Expand Up @@ -635,9 +634,6 @@ Utilities/PtrHolder.h
Utilities/PtrHolder.tcc
Utilities/RecordTransformable.h
Utilities/Regex.h
Utilities/Register.h
Utilities/Register.tcc
Utilities/RegSequence.h
Utilities/Sequence.h
Utilities/Sequence.tcc
Utilities/SortError.h
Expand Down
14 changes: 3 additions & 11 deletions casa/Containers/RecordInterface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include <casacore/casa/Containers/RecordDesc.h>
#include <casacore/casa/Arrays/Array.h>
#include <casacore/casa/Arrays/IPosition.h>
#include <casacore/casa/Utilities/Register.h>
#include <casacore/casa/Exceptions/Error.h>
#include <casacore/casa/Utilities/Assert.h>

Expand Down Expand Up @@ -703,17 +702,10 @@ RecordNotice::RecordNotice (NoticeType changeType, uInt fieldNumber)
fieldNumber_p (fieldNumber)
{}

uInt RecordNotice::type() const
bool RecordNotice::operator== (const Notice& that) const
{
// This function returns the "Notice" type, retrieved
// from the "type registry".
return Register(this);
}

int RecordNotice::operator== (const Notice& that) const
{
if (type() != that.type()) {
return 0;
if (typeid(*this) != typeid(that)) {
return false;
}
return (changeType_p == ((const RecordNotice&)that).changeType_p)
&& (fieldNumber_p == ((const RecordNotice&)that).fieldNumber_p);
Expand Down
5 changes: 1 addition & 4 deletions casa/Containers/RecordInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -632,11 +632,8 @@ class RecordNotice : public Notice
// The field number is only used for type REMOVE.
RecordNotice (NoticeType changeType, uInt fieldNumber);

// Returns the change type.
virtual uInt type() const;

// Always returns False.
virtual int operator== (const Notice& that) const;
virtual bool operator== (const Notice& that) const;

// Return the change type.
NoticeType changeType() const;
Expand Down
7 changes: 1 addition & 6 deletions casa/Quanta/MVBaseline.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include <casacore/casa/Quanta/MVBaseline.h>
#include <casacore/casa/Utilities/Assert.h>
#include <casacore/casa/BasicMath/Math.h>
#include <casacore/casa/Utilities/Register.h>
#include <casacore/casa/Quanta/RotMatrix.h>
#include <casacore/casa/Quanta/UnitVal.h>
#include <casacore/casa/Quanta/QMath.h>
Expand Down Expand Up @@ -152,12 +151,8 @@ MVBaseline MVBaseline::operator-(const MVBaseline &right) const{

//# Member functions

uInt MVBaseline::type() const {
return Register(static_cast<MVBaseline *>(0));
}

void MVBaseline::assure(const MeasValue &in) {
if (in.type() != Register(static_cast<MVBaseline *>(0))) {
if (!dynamic_cast<const MVBaseline*>(&in)) {
throw(AipsError("Illegal MeasValue type argument: MVBaseline"));
}
}
Expand Down
1 change: 0 additions & 1 deletion casa/Quanta/MVBaseline.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,6 @@ class MVBaseline : public MVPosition {

// Tell me your type
// <group>
virtual uInt type() const;
static void assure(const MeasValue &in);
// </group>

Expand Down
7 changes: 1 addition & 6 deletions casa/Quanta/MVDirection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include <casacore/casa/Quanta/Euler.h>
#include <casacore/casa/Quanta/RotMatrix.h>
#include <casacore/casa/Utilities/Assert.h>
#include <casacore/casa/Utilities/Register.h>
#include <casacore/casa/Quanta/MVDirection.h>
#include <casacore/casa/Quanta/UnitVal.h>
#include <casacore/casa/Quanta/QMath.h>
Expand Down Expand Up @@ -131,12 +130,8 @@ MVDirection MVDirection::operator-(const MVDirection &right) const{

//# Member functions

uInt MVDirection::type() const {
return Register(static_cast<MVDirection *>(0));
}

void MVDirection::assure(const MeasValue &in) {
if (in.type() != Register(static_cast<MVDirection *>(0))) {
if (!dynamic_cast<const MVDirection*>(&in)) {
throw(AipsError("Illegal MeasValue type argument: MVDirection"));
}
}
Expand Down
1 change: 0 additions & 1 deletion casa/Quanta/MVDirection.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ class MVDirection : public MVPosition {

// Tell me your type
// <group>
virtual uInt type() const;
static void assure(const MeasValue &in);
// </group>

Expand Down
7 changes: 1 addition & 6 deletions casa/Quanta/MVDoppler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
//# Includes
#include <casacore/casa/Exceptions/Error.h>
#include <casacore/casa/Utilities/Assert.h>
#include <casacore/casa/Utilities/Register.h>
#include <casacore/casa/Quanta/MVDoppler.h>
#include <casacore/casa/BasicMath/Math.h>

Expand Down Expand Up @@ -125,12 +124,8 @@ Bool MVDoppler::nearAbs(const MVDoppler &other, Double tol) const {

// Member functions

uInt MVDoppler::type() const {
return Register(static_cast<MVDoppler *>(0));
}

void MVDoppler::assure(const MeasValue &in) {
if (in.type() != Register(static_cast<MVDoppler *>(0))) {
if (!dynamic_cast<const MVDoppler*>(&in)) {
throw(AipsError("Illegal MeasValue type argument: MVDoppler"));
}
}
Expand Down
1 change: 0 additions & 1 deletion casa/Quanta/MVDoppler.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ class MVDoppler : public MeasValue {

// Tell me your type
// <group>
virtual uInt type() const;
static void assure(const MeasValue &in);
// </group>

Expand Down
7 changes: 1 addition & 6 deletions casa/Quanta/MVDouble.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
//# Includes
#include <casacore/casa/Exceptions/Error.h>
#include <casacore/casa/Utilities/Assert.h>
#include <casacore/casa/Utilities/Register.h>
#include <casacore/casa/Quanta/MVDouble.h>
#include <casacore/casa/IO/ArrayIO.h>
#include <casacore/casa/BasicMath/Math.h>
Expand Down Expand Up @@ -115,12 +114,8 @@ Bool MVDouble::operator!=(const MVDouble &other) const {

//# Member functions

uInt MVDouble::type() const {
return Register(static_cast<MVDouble *>(0));
}

void MVDouble::assure(const MeasValue &in) {
if (in.type() != Register(static_cast<MVDouble *>(0))) {
if (!dynamic_cast<const MVDouble *>(&in)) {
throw(AipsError("Illegal MeasValue type argument: MVDouble"));
}
}
Expand Down
1 change: 0 additions & 1 deletion casa/Quanta/MVDouble.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ class MVDouble : public MeasValue {
//# General member functions
// Tell me your type
// <group>
virtual uInt type() const;
static void assure(const MeasValue &in);
// </group>
// Print data
Expand Down
7 changes: 1 addition & 6 deletions casa/Quanta/MVEarthMagnetic.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include <casacore/casa/Quanta/MVEarthMagnetic.h>
#include <casacore/casa/Utilities/Assert.h>
#include <casacore/casa/BasicMath/Math.h>
#include <casacore/casa/Utilities/Register.h>
#include <casacore/casa/Quanta/RotMatrix.h>
#include <casacore/casa/Quanta/UnitVal.h>
#include <casacore/casa/Quanta/QMath.h>
Expand Down Expand Up @@ -220,12 +219,8 @@ MVEarthMagnetic MVEarthMagnetic::operator-(const MVEarthMagnetic &right) const{

//# Member functions

uInt MVEarthMagnetic::type() const {
return Register(static_cast<MVEarthMagnetic *>(0));
}

void MVEarthMagnetic::assure(const MeasValue &in) {
if (in.type() != Register(static_cast<MVEarthMagnetic *>(0))) {
if (!dynamic_cast<const MVEarthMagnetic*>(&in)) {
throw(AipsError("Illegal MeasValue type argument: MVEarthMagnetic"));
}
}
Expand Down
1 change: 0 additions & 1 deletion casa/Quanta/MVEarthMagnetic.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ class MVEarthMagnetic : public MVPosition {

// Tell me your type
// <group>
virtual uInt type() const;
static void assure(const MeasValue &in);
// </group>

Expand Down
7 changes: 1 addition & 6 deletions casa/Quanta/MVEpoch.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include <casacore/casa/Exceptions/Error.h>
#include <casacore/casa/Quanta/Unit.h>
#include <casacore/casa/Utilities/Assert.h>
#include <casacore/casa/Utilities/Register.h>
#include <casacore/casa/Quanta/MVEpoch.h>
#include <casacore/casa/Quanta/UnitVal.h>
#include <casacore/casa/BasicMath/Math.h>
Expand Down Expand Up @@ -150,12 +149,8 @@ Bool MVEpoch::nearAbs(const MVEpoch &other, Double tol) const {

//# Member functions

uInt MVEpoch::type() const {
return Register(static_cast<MVEpoch *>(0));
}

void MVEpoch::assure(const MeasValue &in) {
if (in.type() != Register(static_cast<MVEpoch *>(0))) {
if (!dynamic_cast<const MVEpoch*>(&in)) {
throw(AipsError("Illegal MeasValue type argument: MVEpoch"));
}
}
Expand Down
1 change: 0 additions & 1 deletion casa/Quanta/MVEpoch.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ class MVEpoch : public MeasValue {

// Tell me your type
// <group>
virtual uInt type() const;
static void assure(const MeasValue &in);
// </group>

Expand Down
7 changes: 1 addition & 6 deletions casa/Quanta/MVFrequency.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include <casacore/casa/Exceptions/Error.h>
#include <casacore/casa/BasicMath/Math.h>
#include <casacore/casa/Utilities/Assert.h>
#include <casacore/casa/Utilities/Register.h>
#include <casacore/casa/Quanta/MVFrequency.h>
#include <casacore/casa/Exceptions/Error.h>
#include <casacore/casa/BasicMath/Math.h>
Expand Down Expand Up @@ -127,12 +126,8 @@ Bool MVFrequency::nearAbs(const MVFrequency &other, Double tol) const {

// Member functions

uInt MVFrequency::type() const {
return Register(static_cast<MVFrequency *>(0));
}

void MVFrequency::assure(const MeasValue &in) {
if (in.type() != Register(static_cast<MVFrequency *>(0))) {
if (!dynamic_cast<const MVFrequency*>(&in)) {
throw(AipsError("Illegal MeasValue type argument: MVFrequency"));
}
}
Expand Down
1 change: 0 additions & 1 deletion casa/Quanta/MVFrequency.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ class MVFrequency : public MeasValue {

// Tell me your type
// <group>
virtual uInt type() const;
static void assure(const MeasValue &in);
// </group>

Expand Down
7 changes: 1 addition & 6 deletions casa/Quanta/MVPosition.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include <casacore/casa/Utilities/Assert.h>
#include <casacore/casa/BasicMath/Math.h>
#include <casacore/casa/Utilities/Assert.h>
#include <casacore/casa/Utilities/Register.h>
#include <casacore/casa/Quanta/MVPosition.h>
#include <casacore/casa/Quanta/RotMatrix.h>
#include <casacore/casa/Quanta/UnitVal.h>
Expand Down Expand Up @@ -297,12 +296,8 @@ MVPosition &MVPosition::operator*=(Double right) {

//# Member functions

uInt MVPosition::type() const {
return Register(static_cast<MVPosition *>(0));
}

void MVPosition::assure(const MeasValue &in) {
if (in.type() != Register(static_cast<MVPosition *>(0))) {
if (!dynamic_cast<const MVPosition*>(&in)) {
throw(AipsError("Illegal MeasValue type argument: MVPosition"));
}
}
Expand Down
1 change: 0 additions & 1 deletion casa/Quanta/MVPosition.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ class MVPosition : public MeasValue {

// Tell me your type
// <group>
virtual uInt type() const;
static void assure(const MeasValue &in);
// </group>

Expand Down
7 changes: 1 addition & 6 deletions casa/Quanta/MVRadialVelocity.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#include <casacore/casa/Quanta/MVFrequency.h>
#include <casacore/casa/Quanta/Quantum.h>
#include <casacore/casa/Utilities/Assert.h>
#include <casacore/casa/Utilities/Register.h>

namespace casacore { //# NAMESPACE CASACORE - BEGIN

Expand Down Expand Up @@ -129,12 +128,8 @@ Bool MVRadialVelocity::nearAbs(const MVRadialVelocity &other, Double tol) const

// Member functions

uInt MVRadialVelocity::type() const {
return Register(static_cast<MVRadialVelocity *>(0));
}

void MVRadialVelocity::assure(const MeasValue &in) {
if (in.type() != Register(static_cast<MVRadialVelocity *>(0))) {
if (!dynamic_cast<const MVRadialVelocity*>(&in)) {
throw(AipsError("Illegal MeasValue type argument: MVRadialVelocity"));
}
}
Expand Down
1 change: 0 additions & 1 deletion casa/Quanta/MVRadialVelocity.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ class MVRadialVelocity : public MeasValue {

// Tell me your type
// <group>
virtual uInt type() const;
static void assure(const MeasValue &in);
// </group>

Expand Down
7 changes: 1 addition & 6 deletions casa/Quanta/MVuvw.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
#include <casacore/casa/Utilities/Assert.h>
#include <casacore/casa/BasicMath/Math.h>
#include <casacore/casa/BasicSL/Constants.h>
#include <casacore/casa/Utilities/Register.h>
#include <casacore/casa/Quanta/RotMatrix.h>
#include <casacore/casa/Quanta/Euler.h>
#include <casacore/casa/Quanta/UnitVal.h>
Expand Down Expand Up @@ -162,12 +161,8 @@ MVuvw MVuvw::operator-(const MVuvw &right) const{

//# Member functions

uInt MVuvw::type() const {
return Register(static_cast<MVuvw *>(0));
}

void MVuvw::assure(const MeasValue &in) {
if (in.type() != Register(static_cast<MVuvw *>(0))) {
if (!dynamic_cast<const MVuvw*>(&in)) {
throw(AipsError("Illegal MeasValue type argument: MVuvw"));
}
}
Expand Down
1 change: 0 additions & 1 deletion casa/Quanta/MVuvw.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ class MVuvw : public MVPosition {

// Tell me your type
// <group>
virtual uInt type() const;
static void assure(const MeasValue &in);
// </group>

Expand Down
10 changes: 0 additions & 10 deletions casa/Quanta/MeasValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,16 +147,6 @@ class MeasValue {
void dummy_operator() const {;};

//# General Member Functions
// Assert that we are the correct MeasValue type
// <thrown>
// <li> AipsError if wrong MeasValue type
// </thrown>
// Each Measure should have:
// <src> static void assure(const MeasValue &in); </src>
// Get the type (== Register(M*)) of derived MeasValue
// <group>
virtual uInt type() const = 0;
// </group>
// Print a MeasValue
virtual void print(ostream &os) const = 0;

Expand Down
1 change: 0 additions & 1 deletion casa/Quanta/Quantum.tcc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
#include <casacore/casa/IO/ArrayIO.h>
#include <casacore/casa/Exceptions/Error.h>
#include <casacore/casa/Utilities/MUString.h>
#include <casacore/casa/Utilities/Register.h>
#include <casacore/casa/sstream.h>

namespace casacore { //# NAMESPACE CASACORE - BEGIN
Expand Down
5 changes: 1 addition & 4 deletions casa/Utilities/Notice.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,8 @@ class Notice {

virtual ~Notice();

// Return the identification number of the <src>Notice</src> type.
virtual uInt type() const = 0;

// Compare two <src>Notice</src>s.
virtual int operator==(const Notice &) const = 0;
virtual bool operator==(const Notice &) const = 0;
};

// <summary>base class for notice originators</summary>
Expand Down
Loading

0 comments on commit 25a015b

Please sign in to comment.