fix: add GTSAM_USE_BOOST_FEATURES to definitions

[ArkadiuszNiemiec]

This fixes the issue #1994 and some issues mentioned in #1967 and #1981.

The change defines the proper identifier if GTSAM_USE_BOOST_FEATURES option is set in CMakeLists.txt.
It's used in:

gtsam/gtsam/base/concepts.h

Lines 11 to 24 in 7b56d66

	#if GTSAM_USE_BOOST_FEATURES
	#include <boost/concept_check.hpp>
	#include <boost/concept/assert.hpp>
	#include <boost/concept/requires.hpp>
	#include <boost/concept_check.hpp>
	#define GTSAM_CONCEPT_ASSERT(concept) BOOST_CONCEPT_ASSERT((concept))
	#define GTSAM_CONCEPT_REQUIRES(concept, return_type) BOOST_CONCEPT_REQUIRES(((concept)), (return_type))
	#else
	// This does something sensible:
	#define BOOST_CONCEPT_USAGE(concept) void check##concept()
	// These just ignore the concept checking for now:
	#define GTSAM_CONCEPT_ASSERT(concept) static_assert(true, "")
	#define GTSAM_CONCEPT_REQUIRES(concept, return_type) return_type
	#endif

...and without it the compiler throws BOOST_CONCEPT_USAGE redefinition error.

[dellaert]

Thanks, but I might not fully understand then: I checked and we don't do this for GTSAM_ENABLE_BOOST_SERIALIZATION, but we can still branch on that in code. Why is GTSAM_USE_BOOST_FEATURES different?

[ArkadiuszNiemiec]

Thanks, but I might not fully understand then: I checked and we don't do this for GTSAM_ENABLE_BOOST_SERIALIZATION, but we can still branch on that in code. Why is GTSAM_USE_BOOST_FEATURES different?

Oh, right. The GTSAM_ENABLE_BOOST_SERIALIZATION didn't generate a build error for me, but it should be added as well. Fixed.

[dellaert]

Well, I guess my question was the other way :-) Why can we branch on GTSAM_ENABLE_BOOST_SERIALIZATION currently? And could we use the same mechanism to branch on GTSAM_USE_BOOST_FEATURES? It seems the add_definitions was not needed for GTSAM_ENABLE_BOOST_SERIALIZATION?

[ArkadiuszNiemiec]

AFAIK both identifiers/macros were always not defined in the code. My linter confirms that but to prove it I added this to one of the examples:

...checked out my first commit (with just GTSAM_USE_BOOST_FEATURES fixed), made sure the flag is ON and built:

The output:

...and you are right. For some reason both are ON 😕 So why it's not working during compile time?

[dellaert]

right :-)
No idea ! Sleuthing required!
But if we find a]out why we can avoid the “add_definitions” step.
Note: it requires #if and not #ifdef. Maybe there is a straggling #ifdef ?

[dellaert]

Note, your test is compile time. All macro pre-processing is gone at runtime.

[ArkadiuszNiemiec]

OK, got it. GTSAM is using https://github.com/borglab/gtsam/blob/develop/gtsam/config.h.in
It creates a config.h that needs to be included if one want to use the CMake variables. To include was missing from that one specific file. Thank you @dellaert, great questions!

[dellaert]

Ah ! Nice, LGTM :-) I will kick off CI and we can merge.

[varunagrawal]

FYI add_definitions is an inefficient approach. Please see #1944 for details.

Also sorry I didn't see this before, was busy with other stuff. Else we could have fixed this sooner. :)