xtensor icon indicating copy to clipboard operation
xtensor copied to clipboard

Error when compiling xtensor and graphviz together

Open Victorvhrn opened this issue 2 years ago • 2 comments

The following code

#include <ostream>
#include <graphviz/gvc.h>
#include <xtensor/xarray.hpp>
#include <xtensor/xbuilder.hpp>


int main(int argc, char** argv){
	ulong u = 10; ulong v = 5;
	xt::xarray<double>::shape_type shape = {u,v};

	auto test = xt::zeros<double>(shape);
	std::cout << test(0,0) << "\n";
}

Compiled with: g++ -std=c++17 -I/usr/local/include/include -I/usr/local/include/xtl/ test_xtensor.cpp -o test_xtensor -lgvc

Gives the following error:

In file included from /usr/include/graphviz/geom.h:20:0,
                 from /usr/include/graphviz/types.h:35,
                 from /usr/include/graphviz/gvc.h:17,
                 from test_xtensor.cpp:3:
/usr/local/include/include/xtensor/xmath.hpp:46:28: error: expected unqualified-id before numeric constant
         static constexpr T SQRT2 = 1.41421356237309504880;

OS: WSL2 Ubuntu 18.04 (Windows 10 version 2004) Compiler: g++ version 7.5.0 xtensor: I've downloaded yesterday, couldn't find version number. Graphviz: 2.40.1

EDIT: One of the headers in graphviz already has a #define SQRT2, maybe this is why this error is happening. When I've changed the order of includes in the example above the code compiled correctly. Is there a way as to change the code as to not rely in the order of includes?

Victorvhrn avatar Aug 21 '21 17:08 Victorvhrn

EDIT: One of the headers in graphviz already has a #define SQRT2, maybe this is why this error is happening. When I've changed the order of includes in the example above the code compiled correctly. Is there a way as to change the code as to not rely in the order of includes?

I would say that that is indeed why the compiler gets confused. If SQRT2 is defined the pre-processor replaces the variable name SQRT2 with the contents of the macro, naturally confusing the compiler. I would say that, in principle, there is nothing xtensor can do about is : SQRT2 is quite a general name, so graphviz exposes its users to a risk of this happening. At the same time, personally I would not have introduced the all caps variable name SQRT2 in xtensor, as it is quite a common convention to define macros in all caps (and, again, SQRT2 is not an unreasonable macro to have) and thus exposes xtensor users to a risk of this happening. I don't see a solution completely free of such a risk. It would be not be unreasonable to have a the lower case sqrt2 instead of SQRT2, limiting the risk of a clash with a macro. If I'm not work there are other static variables in xtensor that are also lower case (e.g. the rank member of xtensor).

As to a pragmatic solution, indeed, if the pre-processor does not yet know about the macro in graphviz everything should work fine.

tdegeus avatar Aug 22 '21 11:08 tdegeus

These constants do not seem to be used anywhere in xtensor. They look like legacy code that should be removed imho. I'll check downstream projects and remove them if that does not break anything.

JohanMabille avatar Aug 23 '21 06:08 JohanMabille