Improve Compiler Errors

[akleeman]

One of the more difficult issues when iterating on albatross is the often cryptic compiler errors that are produced when developing. The current approach to this problem is to insert a bunch of static checks which explicitly delete functions that aren't valid which then leads the compiler to dump all the information about the templated types used which lead to the selection of the deleted function. The problem with this is that there isn't a way to add a human readable description of what the problem was.

This change attempts to improve the situation slightly by defining a macro ALBATROSS_FAIL which lets you use the = delete approach or switch to a delay_static_assert approach similar to cereal. Using delay_static_assert let's you provide a human readable message along with the resolved template parameters which should make debugging issues easier.

In addition to the these changes I've adding a few additional macros which build some of the frequently used trait inspection routines. This made it easier to create even more has_any_* methods which in turn should make it easier to add more ALBATROSS_FAIL instances.

BEFORE:

/home/kleeman/dev/swift/albatross/tests/test_core_model.cc:35:74: error: use of deleted function ‘void albatross::Prediction<ModelType, FeatureType, FitType>::mean() const [with DummyType = albatross::MockFeature; typename std::enable_if<(! albatross::can_predict_mean<albatross::MeanPredictor, ModelType, DummyType, FitType>::value), int>::type <anonymous> = 0; ModelType = albatross::MockModel; FeatureType = albatross::MockFeature; FitType = albatross::Fit<albatross::MockModel>]’
   Eigen::VectorXd predictions = fit_model.predict(dataset.features).mean();
                                                                          ^
In file included from /home/kleeman/dev/swift/albatross/include/albatross/Core:26:0,
                 from /home/kleeman/dev/swift/albatross/tests/test_utils.h:16,
                 from /home/kleeman/dev/swift/albatross/tests/test_core_model.cc:15:
/home/kleeman/dev/swift/albatross/include/albatross/src/core/prediction.hpp:146:8: note: declared here
   void mean() const

AFTER:

In file included from /home/kleeman/dev/swift/albatross/include/albatross/Core:26:0,
                 from /home/kleeman/dev/swift/albatross/tests/test_utils.h:16,
                 from /home/kleeman/dev/swift/albatross/tests/test_core_model.cc:15:
/home/kleeman/dev/swift/albatross/include/albatross/src/core/prediction.hpp: In instantiation of ‘void albatross::Prediction<ModelType, FeatureType, FitType>::mean() const [with DummyType = albatross::MockFeature; typename std::enable_if<(! albatross::can_predict_mean<albatross::MeanPredictor, ModelType, DummyType, FitType>::value), int>::type <anonymous> = 0; ModelType = albatross::MockModel; FeatureType = albatross::MockFeature; FitType = albatross::Fit<albatross::MockModel>]’:
/home/kleeman/dev/swift/albatross/tests/test_core_model.cc:35:74:   required from here
/home/kleeman/dev/swift/albatross/include/albatross/src/core/prediction.hpp:147:9: error: static assertion failed: No valid predict method in ModelType for the mean with FitType and FeatureType.
       ALBATROSS_FAIL(DummyType, "No valid predict method in ModelType for the "
         ^~~~~~~~~~~~~

[seth-swiftnav]

Do you have some before-after compiler errors?
I see nothing wrong with the code, but macros aren't something I'm very familiar with.

If this helps development by being more explicit that templated functions aren't defined, I'm for the changes.

[akleeman]

@seth-swiftnav just updated the description with an example.

[akleeman]

Perhaps worth noting that I haven't done it in this change, but in the future I hope to use these macros to start catching some common errors such as slight misspellings, lack of const on methods, invalid return type etc ... Ie, setup some catches like has_any_predict_impl and has_valid_predict_impl to flag situations where a user has clearly tried to define a function but something isn't correct. This seems to be the most frequent sort of issue that I run into and it'd be nice to have the compiler give some direction in those situations.

[jbangelo]

Should this be deleted?

[akleeman]

I intentionally left that in as an example of an alternate definition of ALBATROSS_FAIL. I'll make that more explicit.

[jbangelo]

Would it make sense to use std::decay<X>::type to get rid of potential reference/const-ness?

[akleeman]

Yeah seems prudent, thanks.