[PATCH] Include space.h in headers using isl_dim_type

[Lawrence Velázquez]

These headers should not assume that space.h will be included
transitively through another header or by an including source file, as
compilation can fail if those assumptions don't hold.

Signed-off-by: Lawrence Velázquez <lar...@macports.org>
---
include/isl/aff.h | 1 +
include/isl/deprecated/aff_int.h | 1 +
include/isl/deprecated/constraint_int.h | 1 +
include/isl/deprecated/map_int.h | 1 +
include/isl/deprecated/point_int.h | 1 +
include/isl/deprecated/polynomial_int.h | 1 +
include/isl/deprecated/set_int.h | 1 +
include/isl/set.h | 1 +
8 files changed, 8 insertions(+)

diff --git a/include/isl/aff.h b/include/isl/aff.h
index ea23784..29ca6c5 100644
--- a/include/isl/aff.h
+++ b/include/isl/aff.h
@@ -2,6 +2,7 @@
#define ISL_AFF_H

#include <isl/local_space.h>
+#include <isl/space.h>
#include <isl/printer.h>
#include <isl/set_type.h>
#include <isl/aff_type.h>
diff --git a/include/isl/deprecated/aff_int.h b/include/isl/deprecated/aff_int.h
index cbab1f9..da0d4e8 100644
--- a/include/isl/deprecated/aff_int.h
+++ b/include/isl/deprecated/aff_int.h
@@ -3,6 +3,7 @@

#include <isl/deprecated/int.h>
#include <isl/aff_type.h>
+#include <isl/space.h>

#if defined(__cplusplus)
extern "C" {
diff --git a/include/isl/deprecated/constraint_int.h b/include/isl/deprecated/constraint_int.h
index c5ab0aa..7759ff3 100644
--- a/include/isl/deprecated/constraint_int.h
+++ b/include/isl/deprecated/constraint_int.h
@@ -3,6 +3,7 @@

#include <isl/deprecated/int.h>
#include <isl/constraint.h>
+#include <isl/space.h>

#if defined(__cplusplus)
extern "C" {
diff --git a/include/isl/deprecated/map_int.h b/include/isl/deprecated/map_int.h
index 915afbf..c613b97 100644
--- a/include/isl/deprecated/map_int.h
+++ b/include/isl/deprecated/map_int.h
@@ -3,6 +3,7 @@

#include <isl/deprecated/int.h>
#include <isl/map_type.h>
+#include <isl/space.h>

#if defined(__cplusplus)
extern "C" {
diff --git a/include/isl/deprecated/point_int.h b/include/isl/deprecated/point_int.h
index 2e1a98b..31fc9c5 100644
--- a/include/isl/deprecated/point_int.h
+++ b/include/isl/deprecated/point_int.h
@@ -3,6 +3,7 @@

#include <isl/deprecated/int.h>
#include <isl/point.h>
+#include <isl/space.h>

#if defined(__cplusplus)
extern "C" {
diff --git a/include/isl/deprecated/polynomial_int.h b/include/isl/deprecated/polynomial_int.h
index b86fcf2..e485c97 100644
--- a/include/isl/deprecated/polynomial_int.h
+++ b/include/isl/deprecated/polynomial_int.h
@@ -3,6 +3,7 @@

#include <isl/deprecated/int.h>
#include <isl/polynomial.h>
+#include <isl/space.h>

#if defined(__cplusplus)
extern "C" {
diff --git a/include/isl/deprecated/set_int.h b/include/isl/deprecated/set_int.h
index 84bb88a..30a127a 100644
--- a/include/isl/deprecated/set_int.h
+++ b/include/isl/deprecated/set_int.h
@@ -3,6 +3,7 @@

#include <isl/deprecated/int.h>
#include <isl/set_type.h>
+#include <isl/space.h>

#if defined(__cplusplus)
extern "C" {
diff --git a/include/isl/set.h b/include/isl/set.h
index 73b6570..4934f62 100644
--- a/include/isl/set.h
+++ b/include/isl/set.h
@@ -16,6 +16,7 @@
#include <isl/mat.h>
#include <isl/point.h>
#include <isl/local_space.h>
+#include <isl/space.h>
#include <isl/val.h>

#if defined(__cplusplus)
--
2.1.3

[Sven Verdoolaege]

On Tue, Nov 25, 2014 at 02:42:04PM -0500, Lawrence Velázquez wrote:
> These headers should not assume that space.h will be included
> transitively through another header or by an including source file, as
> compilation can fail if those assumptions don't hold.

Hmmm... I was about to apply you patch, but then I noticed
that both headers where you included space.h already include
local_space.h, which, by necessity, includes space.h already,
so I don't see why the extra includes are needed.

Note that I'm ignoring your changes to the deprecated headers.
I have no interest in improving deprecated features.

skimo

[Lawrence Velázquez]

On Nov 25, 2014, at 3:51 PM, Sven Verdoolaege <skim...@kotnet.org> wrote:

> On Tue, Nov 25, 2014 at 02:42:04PM -0500, Lawrence Velázquez wrote:
>> These headers should not assume that space.h will be included
>> transitively through another header or by an including source file, as
>> compilation can fail if those assumptions don't hold.
>
> Hmmm... I was about to apply you patch, but then I noticed
> that both headers where you included space.h already include
> local_space.h, which, by necessity, includes space.h already,
> so I don't see why the extra includes are needed.

They're not required _per se_. Relying on transitive includes for symbol declaration is fragile, but if those headers will _always_ include local_space.h, and local_space.h will _always_ include space.h, then it doesn't make a practical difference.

> Note that I'm ignoring your changes to the deprecated headers.
> I have no interest in improving deprecated features.

Fair enough. I ran into issues with those headers while backporting isl 0.14 support to MacPorts' GCC 4.7 and 4.8, but we can maintain our own patches easily enough.

Thanks,
vq

[Sven Verdoolaege]

Hmm... wouldn't you have to do that anyway, given that 0.14 has
been released already?

Btw, be careful about using newer versions of isl with tools
that expect older versions. That may break in more subtle ways too.
I guess it's not that much of a problem since isl is only compiled
in but not used by default.

Note that AFAIU, the gcc maintainers are planning on supporting
newer versions of isl in point releases of older versions of gcc.

skimo

[Lawrence Velázquez]

On Nov 26, 2014, at 3:23 AM, Sven Verdoolaege <skim...@kotnet.org> wrote:

> Hmm... wouldn't you have to do that anyway, given that 0.14 has
> been released already?

We recently discovered that our GCC 4.8 and 4.9 (I misspoke earlier) were actually not being linked to isl (and CLooG) at all. It looks like 0.14 is too new for their current configure scripts.

(Although our 4.6 and 4.7 are linking to it, apparently without any special fiddling. Maybe it gets passed through CLooG-isl? I haven't looked into it.)

> Btw, be careful about using newer versions of isl with tools
> that expect older versions. That may break in more subtle ways too.
> I guess it's not that much of a problem since isl is only compiled
> in but not used by default.

Thanks for the warning; I'll keep an eye out for strangeness.

> Note that AFAIU, the gcc maintainers are planning on supporting
> newer versions of isl in point releases of older versions of gcc.

That would be quite welcome.

vq