[PATCH 1/4] split interface generator in generic and python-specific part

68 views
Skip to first unread message

Tobias Grosser

unread,
Oct 19, 2016, 2:39:07 PM10/19/16
to isl-dev...@googlegroups.com, Armin Groesslinger, Andreas Simbuerger, Sven Verdoolaege, Tobias Grosser
From: Armin Groesslinger <armin.gro...@uni-passau.de>

To support bindings for other languages than Python in the
future, move the functions for querying information about
isl functions and types (e.g., takes(), is_subclass()) and
the struct isl_class to generator.h and generator.cc. The
generator routines for Python remain in python.h and python.cc.

This change does not change current behavior. The generated isl.py files
are identical.

Signed-off-by: Armin Groesslinger <armin.gro...@uni-passau.de>
Signed-off-by: Andreas Simbuerger <simb...@fim.uni-passau.de>
Signed-off-by: Sven Verdoolaege <sk...@kotnet.org>
Signed-off-by: Tobias Grosser <tob...@grosser.es>
---
interface/Makefile.am | 2 +
interface/extract_interface.cc | 1 +
interface/generator.cc | 260 +++++++++++++++++++++++++++++++++++++++++
interface/generator.h | 58 +++++++++
interface/python.cc | 251 ---------------------------------------
interface/python.h | 3 +-
6 files changed, 322 insertions(+), 253 deletions(-)
create mode 100644 interface/generator.cc
create mode 100644 interface/generator.h

diff --git a/interface/Makefile.am b/interface/Makefile.am
index 242294e..a37cb18 100644
--- a/interface/Makefile.am
+++ b/interface/Makefile.am
@@ -10,6 +10,8 @@ includes = -I$(top_builddir) -I$(top_srcdir) \

extract_interface_CPPFLAGS = $(includes)
extract_interface_SOURCES = \
+ generator.h \
+ generator.cc \
python.h \
python.cc \
extract_interface.h \
diff --git a/interface/extract_interface.cc b/interface/extract_interface.cc
index 331ec44..9013f66 100644
--- a/interface/extract_interface.cc
+++ b/interface/extract_interface.cc
@@ -74,6 +74,7 @@
#include <clang/Sema/Sema.h>

#include "extract_interface.h"
+#include "generator.h"
#include "python.h"

using namespace std;
diff --git a/interface/generator.cc b/interface/generator.cc
new file mode 100644
index 0000000..63b21f9
--- /dev/null
+++ b/interface/generator.cc
@@ -0,0 +1,260 @@
+/*
+ * Copyright 2011,2015 Sven Verdoolaege. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY SVEN VERDOOLAEGE ''AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL SVEN VERDOOLAEGE OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA,
+ * OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * The views and conclusions contained in the software and documentation
+ * are those of the authors and should not be interpreted as
+ * representing official policies, either expressed or implied, of
+ * Sven Verdoolaege.
+ */
+
+#include <clang/AST/Attr.h>
+
+#include "isl_config.h"
+#include "extract_interface.h"
+
+#include "generator.h"
+
+#include <iostream>
+
+/* Print error message "msg" and abort.
+ */
+void die(const char *msg)
+{
+ fprintf(stderr, "%s", msg);
+ abort();
+}
+
+/* If "method" is overloaded, then drop the suffix of "name"
+ * corresponding to the type of the final argument and
+ * return the modified name (or the original name if
+ * no modifications were made).
+ */
+string drop_type_suffix(string name, FunctionDecl *method)
+{
+ int num_params;
+ ParmVarDecl *param;
+ string type;
+ size_t name_len, type_len;
+
+ if (!is_overload(method))
+ return name;
+
+ num_params = method->getNumParams();
+ param = method->getParamDecl(num_params - 1);
+ type = extract_type(param->getOriginalType());
+ type = type.substr(4);
+ name_len = name.length();
+ type_len = type.length();
+
+ if (name_len > type_len && name.substr(name_len - type_len) == type)
+ name = name.substr(0, name_len - type_len - 1);
+
+ return name;
+}
+
+/* Return a sequence of the types of which the given type declaration is
+ * marked as being a subtype.
+ */
+std::vector<string> find_superclasses(RecordDecl *decl)
+{
+ vector<string> super;
+
+ if (!decl->hasAttrs())
+ return super;
+
+ string sub = "isl_subclass";
+ size_t len = sub.length();
+ AttrVec attrs = decl->getAttrs();
+ for (AttrVec::const_iterator i = attrs.begin() ; i != attrs.end(); ++i) {
+ const AnnotateAttr *ann = dyn_cast<AnnotateAttr>(*i);
+ if (!ann)
+ continue;
+ string s = ann->getAnnotation().str();
+ if (s.substr(0, len) == sub) {
+ s = s.substr(len + 1, s.length() - len - 2);
+ super.push_back(s);
+ }
+ }
+
+ return super;
+}
+
+/* Is decl marked as being part of an overloaded method?
+ */
+bool is_overload(Decl *decl)
+{
+ return has_annotation(decl, "isl_overload");
+}
+
+/* Is decl marked as a constructor?
+ */
+bool is_constructor(Decl *decl)
+{
+ return has_annotation(decl, "isl_constructor");
+}
+
+/* Is decl marked as consuming a reference?
+ */
+bool takes(Decl *decl)
+{
+ return has_annotation(decl, "isl_take");
+}
+
+/* Is decl marked as returning a reference that is required to be freed.
+ */
+bool gives(Decl *decl)
+{
+ return has_annotation(decl, "isl_give");
+}
+
+/* Return the class that has a name that matches the initial part
+ * of the name of function "fd" or NULL if no such class could be found.
+ */
+isl_class *method2class(map<string, isl_class> &classes,
+ FunctionDecl *fd)
+{
+ string best;
+ map<string, isl_class>::iterator ci;
+ string name = fd->getNameAsString();
+
+ for (ci = classes.begin(); ci != classes.end(); ++ci) {
+ if (name.substr(0, ci->first.length()) == ci->first)
+ best = ci->first;
+ }
+
+ if (classes.find(best) == classes.end()) {
+ cerr << "Unable to find class of " << name << endl;
+ return NULL;
+ }
+
+ return &classes[best];
+}
+
+/* Is "type" the type "isl_ctx *"?
+ */
+bool is_isl_ctx(QualType type)
+{
+ if (!type->isPointerType())
+ return 0;
+ type = type->getPointeeType();
+ if (type.getAsString() != "isl_ctx")
+ return false;
+
+ return true;
+}
+
+/* Is the first argument of "fd" of type "isl_ctx *"?
+ */
+bool first_arg_is_isl_ctx(FunctionDecl *fd)
+{
+ ParmVarDecl *param;
+
+ if (fd->getNumParams() < 1)
+ return false;
+
+ param = fd->getParamDecl(0);
+ return is_isl_ctx(param->getOriginalType());
+}
+
+/* Is "type" that of a pointer to an isl_* structure?
+ */
+bool is_isl_type(QualType type)
+{
+ if (type->isPointerType()) {
+ string s;
+
+ type = type->getPointeeType();
+ if (type->isFunctionType())
+ return false;
+ s = type.getAsString();
+ return s.substr(0, 4) == "isl_";
+ }
+
+ return false;
+}
+
+/* Is "type" the type isl_bool?
+ */
+bool is_isl_bool(QualType type)
+{
+ string s;
+
+ if (type->isPointerType())
+ return false;
+
+ s = type.getAsString();
+ return s == "isl_bool";
+}
+
+/* Is "type" that of a pointer to char.
+ */
+bool is_string_type(QualType type)
+{
+ if (type->isPointerType()) {
+ string s;
+
+ type = type->getPointeeType();
+ if (type->isFunctionType())
+ return false;
+ s = type.getAsString();
+ return s == "const char" || "char";
+ }
+
+ return false;
+}
+
+/* Is "type" that of a pointer to a function?
+ */
+bool is_callback(QualType type)
+{
+ if (!type->isPointerType())
+ return false;
+ type = type->getPointeeType();
+ return type->isFunctionType();
+}
+
+/* Is "type" that of "char *" of "const char *"?
+ */
+bool is_string(QualType type)
+{
+ if (type->isPointerType()) {
+ string s = type->getPointeeType().getAsString();
+ return s == "const char" || s == "char";
+ }
+
+ return false;
+}
+
+/* Return the name of the type that "type" points to.
+ * The input "type" is assumed to be a pointer type.
+ */
+string extract_type(QualType type)
+{
+ if (type->isPointerType())
+ return type->getPointeeType().getAsString();
+ die("Cannot extract type from non-pointer type");
+}
diff --git a/interface/generator.h b/interface/generator.h
new file mode 100644
index 0000000..d577b4d
--- /dev/null
+++ b/interface/generator.h
@@ -0,0 +1,58 @@
+#ifndef ISL_INTERFACE_GENERATOR_H
+#define ISL_INTERFACE_GENERATOR_H
+
+#include <clang/AST/Decl.h>
+#include <map>
+#include <set>
+#include <string>
+
+
+using namespace std;
+using namespace clang;
+
+/* isl_class collects all constructors and methods for an isl "class".
+ * "name" is the name of the class.
+ * "type" is the declaration that introduces the type.
+ * "methods" contains the set of methods, grouped by method name.
+ * "fn_to_str" is a reference to the *_to_str method of this class, if any.
+ * "fn_free" is a reference to the *_free method of this class, if any.
+ */
+struct isl_class {
+ string name;
+ RecordDecl *type;
+ set<FunctionDecl *> constructors;
+ map<string, set<FunctionDecl *> > methods;
+ FunctionDecl *fn_to_str;
+ FunctionDecl *fn_free;
+
+ bool is_static(FunctionDecl *method);
+
+ void print(map<string, isl_class> &classes, set<string> &done);
+ void print_constructor(FunctionDecl *method);
+ void print_representation(const string &python_name);
+ void print_method_type(FunctionDecl *fd);
+ void print_method_types();
+ void print_method(FunctionDecl *method, vector<string> super);
+ void print_method_overload(FunctionDecl *method, vector<string> super);
+ void print_method(const string &fullname,
+ const set<FunctionDecl *> &methods, vector<string> super);
+};
+
+void die(const char *msg) __attribute__((noreturn));
+string drop_type_suffix(string name, FunctionDecl *method);
+vector<string> find_superclasses(RecordDecl *decl);
+bool is_overload(Decl *decl);
+bool is_constructor(Decl *decl);
+bool takes(Decl *decl);
+bool gives(Decl *decl);
+isl_class *method2class(map<string, isl_class> &classes, FunctionDecl *fd);
+bool is_isl_ctx(QualType type);
+bool first_arg_is_isl_ctx(FunctionDecl *fd);
+bool is_isl_type(QualType type);
+bool is_isl_bool(QualType type);
+bool is_string_type(QualType type);
+bool is_callback(QualType type);
+bool is_string(QualType type);
+string extract_type(QualType type);
+
+#endif /* ISL_INTERFACE_GENERATOR_H */
diff --git a/interface/python.cc b/interface/python.cc
index 80977e7..4ec166f 100644
--- a/interface/python.cc
+++ b/interface/python.cc
@@ -37,231 +37,8 @@
#include <iostream>
#include <map>
#include <vector>
-#include <clang/AST/Attr.h>
-#include "extract_interface.h"
#include "python.h"

-static void die(const char *msg) __attribute__((noreturn));
-
-/* Print error message "msg" and abort.
- */
-static void die(const char *msg)
-{
- fprintf(stderr, "%s", msg);
- abort();
-}
-
-/* Return a sequence of the types of which the given type declaration is
- * marked as being a subtype.
- */
-static vector<string> find_superclasses(RecordDecl *decl)
-{
- vector<string> super;
-
- if (!decl->hasAttrs())
- return super;
-
- string sub = "isl_subclass";
- size_t len = sub.length();
- AttrVec attrs = decl->getAttrs();
- for (AttrVec::const_iterator i = attrs.begin() ; i != attrs.end(); ++i) {
- const AnnotateAttr *ann = dyn_cast<AnnotateAttr>(*i);
- if (!ann)
- continue;
- string s = ann->getAnnotation().str();
- if (s.substr(0, len) == sub) {
- s = s.substr(len + 1, s.length() - len - 2);
- super.push_back(s);
- }
- }
-
- return super;
-}
-
-/* Is decl marked as being part of an overloaded method?
- */
-static bool is_overload(Decl *decl)
-{
- return has_annotation(decl, "isl_overload");
-}
-
-/* Is decl marked as a constructor?
- */
-static bool is_constructor(Decl *decl)
-{
- return has_annotation(decl, "isl_constructor");
-}
-
-/* Is decl marked as consuming a reference?
- */
-static bool takes(Decl *decl)
-{
- return has_annotation(decl, "isl_take");
-}
-
-/* Is decl marked as returning a reference that is required to be freed.
- */
-static bool gives(Decl *decl)
-{
- return has_annotation(decl, "isl_give");
-}
-
-/* isl_class collects all constructors and methods for an isl "class".
- * "name" is the name of the class.
- * "type" is the declaration that introduces the type.
- * "methods" contains the set of methods, grouped by method name.
- * "fn_to_str" is a reference to the *_to_str method of this class, if any.
- * "fn_free" is a reference to the *_free method of this class, if any.
- */
-struct isl_class {
- string name;
- RecordDecl *type;
- set<FunctionDecl *> constructors;
- map<string, set<FunctionDecl *> > methods;
- FunctionDecl *fn_to_str;
- FunctionDecl *fn_free;
-
- bool is_static(FunctionDecl *method);
-
- void print(map<string, isl_class> &classes, set<string> &done);
- void print_constructor(FunctionDecl *method);
- void print_representation(const string &python_name);
- void print_method_type(FunctionDecl *fd);
- void print_method_types();
- void print_method(FunctionDecl *method, vector<string> super);
- void print_method_overload(FunctionDecl *method, vector<string> super);
- void print_method(const string &fullname,
- const set<FunctionDecl *> &methods, vector<string> super);
-};
-
-/* Return the class that has a name that matches the initial part
- * of the name of function "fd" or NULL if no such class could be found.
- */
-static isl_class *method2class(map<string, isl_class> &classes,
- FunctionDecl *fd)
-{
- string best;
- map<string, isl_class>::iterator ci;
- string name = fd->getNameAsString();
-
- for (ci = classes.begin(); ci != classes.end(); ++ci) {
- if (name.substr(0, ci->first.length()) == ci->first)
- best = ci->first;
- }
-
- if (classes.find(best) == classes.end()) {
- cerr << "Unable to find class of " << name << endl;
- return NULL;
- }
-
- return &classes[best];
-}
-
-/* Is "type" the type "isl_ctx *"?
- */
-static bool is_isl_ctx(QualType type)
-{
- if (!type->isPointerType())
- return 0;
- type = type->getPointeeType();
- if (type.getAsString() != "isl_ctx")
- return false;
-
- return true;
-}
-
-/* Is the first argument of "fd" of type "isl_ctx *"?
- */
-static bool first_arg_is_isl_ctx(FunctionDecl *fd)
-{
- ParmVarDecl *param;
-
- if (fd->getNumParams() < 1)
- return false;
-
- param = fd->getParamDecl(0);
- return is_isl_ctx(param->getOriginalType());
-}
-
-/* Is "type" that of a pointer to an isl_* structure?
- */
-static bool is_isl_type(QualType type)
-{
- if (type->isPointerType()) {
- string s;
-
- type = type->getPointeeType();
- if (type->isFunctionType())
- return false;
- s = type.getAsString();
- return s.substr(0, 4) == "isl_";
- }
-
- return false;
-}
-
-/* Is "type" the type isl_bool?
- */
-static bool is_isl_bool(QualType type)
-{
- string s;
-
- if (type->isPointerType())
- return false;
-
- s = type.getAsString();
- return s == "isl_bool";
-}
-
-/* Is "type" that of a pointer to char.
- */
-static bool is_string_type(QualType type)
-{
- if (type->isPointerType()) {
- string s;
-
- type = type->getPointeeType();
- if (type->isFunctionType())
- return false;
- s = type.getAsString();
- return s == "const char" || "char";
- }
-
- return false;
-}
-
-/* Is "type" that of a pointer to a function?
- */
-static bool is_callback(QualType type)
-{
- if (!type->isPointerType())
- return false;
- type = type->getPointeeType();
- return type->isFunctionType();
-}
-
-/* Is "type" that of "char *" of "const char *"?
- */
-static bool is_string(QualType type)
-{
- if (type->isPointerType()) {
- string s = type->getPointeeType().getAsString();
- return s == "const char" || s == "char";
- }
-
- return false;
-}
-
-/* Return the name of the type that "type" points to.
- * The input "type" is assumed to be a pointer type.
- */
-static string extract_type(QualType type)
-{
- if (type->isPointerType())
- return type->getPointeeType().getAsString();
- die("Cannot extract type from non-pointer type");
-}
-
/* Drop the "isl_" initial part of the type name "name".
*/
static string type2python(string name)
@@ -269,34 +46,6 @@ static string type2python(string name)
return name.substr(4);
}

-/* If "method" is overloaded, then drop the suffix of "name"
- * corresponding to the type of the final argument and
- * return the modified name (or the original name if
- * no modifications were made).
- */
-static string drop_type_suffix(string name, FunctionDecl *method)
-{
- int num_params;
- ParmVarDecl *param;
- string type;
- size_t name_len, type_len;
-
- if (!is_overload(method))
- return name;
-
- num_params = method->getNumParams();
- param = method->getParamDecl(num_params - 1);
- type = extract_type(param->getOriginalType());
- type = type.substr(4);
- name_len = name.length();
- type_len = type.length();
-
- if (name_len > type_len && name.substr(name_len - type_len) == type)
- name = name.substr(0, name_len - type_len - 1);
-
- return name;
-}
-
/* Should "method" be considered to be a static method?
* That is, is the first argument something other than
* an instance of the class?
diff --git a/interface/python.h b/interface/python.h
index 5dff126..1706dd7 100644
--- a/interface/python.h
+++ b/interface/python.h
@@ -1,5 +1,4 @@
-#include <set>
-#include <clang/AST/Decl.h>
+#include "generator.h"

using namespace std;
using namespace clang;
--
2.7.4

Tobias Grosser

unread,
Oct 19, 2016, 2:39:08 PM10/19/16
to isl-dev...@googlegroups.com, Armin Groesslinger, Andreas Simbuerger, Tobias Grosser
From: Armin Groesslinger <armin.gro...@uni-passau.de>

This change is large, but mostly mechanical. In generator.cc the prefix
'generator::' is added to turn static functions into class methods. In
python.cc functions are moved into the newly introduced
python_generator. This sometimes requires the introduction of a new
isl_class argument.

The only non-trivial change is the separation of generate_python into
generator::generator() and python_generator::generate(). The former now
constructs the set of isl_classses which we want to generate bindings
for, whereas the latter prints for each class the corresponding bindings.

This change does not change current behavior. The generated isl.py files
are identical.

Signed-off-by: Armin Groesslinger <armin.gro...@uni-passau.de>
Signed-off-by: Andreas Simbuerger <simb...@fim.uni-passau.de>
Signed-off-by: Tobias Grosser <tob...@grosser.es>
---
interface/extract_interface.cc | 3 +-
interface/generator.cc | 100 +++++++++++++++++----
interface/generator.h | 65 ++++++++------
interface/python.cc | 191 +++++++++++++++--------------------------
interface/python.h | 39 ++++++++-
5 files changed, 231 insertions(+), 167 deletions(-)

diff --git a/interface/extract_interface.cc b/interface/extract_interface.cc
index 9013f66..ddca368 100644
--- a/interface/extract_interface.cc
+++ b/interface/extract_interface.cc
@@ -423,8 +423,9 @@ int main(int argc, char *argv[])
ParseAST(*sema);
Diags.getClient()->EndSourceFile();

- generate_python(consumer.exported_types, consumer.exported_functions,
+ python_generator gen(consumer.exported_types, consumer.exported_functions,
consumer.functions);
+ gen.generate();

delete sema;
delete Clang;
diff --git a/interface/generator.cc b/interface/generator.cc
index 63b21f9..9cc6e2d 100644
--- a/interface/generator.cc
+++ b/interface/generator.cc
@@ -40,9 +40,77 @@

#include <iostream>

+/* Should "method" be considered to be a static method?
+ * That is, is the first argument something other than
+ * an instance of the class?
+ */
+bool generator::is_static(const isl_class &clazz, FunctionDecl *method)
+{
+ ParmVarDecl *param = method->getParamDecl(0);
+ QualType type = param->getOriginalType();
+
+ if (!is_isl_type(type))
+ return true;
+ return extract_type(type) != clazz.name;
+}
+
+/* Collect all functions that belong to a certain type, separating
+ * constructors from regular methods and keeping track of the _to_str and
+ * _free functions, if any, separately. If there are any overloaded
+ * functions, then they are grouped based on their name after removing the
+ * argument type suffix.
+ */
+generator::generator(set<RecordDecl *> &exported_types,
+ set<FunctionDecl *> exported_functions, set<FunctionDecl *> functions)
+{
+ map<string, isl_class>::iterator ci;
+
+ set<FunctionDecl *>::iterator in;
+ for (in = functions.begin(); in != functions.end(); ++in) {
+ FunctionDecl *decl = *in;
+ functions_by_name[decl->getName()] = decl;
+ }
+
+ set<RecordDecl *>::iterator it;
+ for (it = exported_types.begin(); it != exported_types.end(); ++it) {
+ RecordDecl *decl = *it;
+ map<string, FunctionDecl *>::iterator i;
+
+ string name = decl->getName();
+ classes[name].name = name;
+ classes[name].type = decl;
+ classes[name].fn_to_str = NULL;
+ classes[name].fn_free = NULL;
+
+ i = functions_by_name.find(name + "_to_str");
+ if (i != functions_by_name.end())
+ classes[name].fn_to_str = i->second;
+
+ i = functions_by_name.find (name + "_free");
+ if (i == functions_by_name.end())
+ die("No _free function found");
+ classes[name].fn_free = i->second;
+ }
+
+ for (in = exported_functions.begin(); in != exported_functions.end();
+ ++in) {
+ isl_class *c = method2class(classes, *in);
+ if (!c)
+ continue;
+ if (is_constructor(*in)) {
+ c->constructors.insert(*in);
+ } else {
+ FunctionDecl *method = *in;
+ string fullname = method->getName();
+ fullname = drop_type_suffix(fullname, method);
+ c->methods[fullname].insert(method);
+ }
+ }
+}
+
/* Print error message "msg" and abort.
*/
-void die(const char *msg)
+void generator::die(const char *msg)
{
fprintf(stderr, "%s", msg);
abort();
@@ -53,7 +121,7 @@ void die(const char *msg)
* return the modified name (or the original name if
* no modifications were made).
*/
-string drop_type_suffix(string name, FunctionDecl *method)
+string generator::drop_type_suffix(string name, FunctionDecl *method)
{
int num_params;
ParmVarDecl *param;
@@ -79,7 +147,7 @@ string drop_type_suffix(string name, FunctionDecl *method)
/* Return a sequence of the types of which the given type declaration is
* marked as being a subtype.
*/
-std::vector<string> find_superclasses(RecordDecl *decl)
+std::vector<string> generator::find_superclasses(RecordDecl *decl)
{
vector<string> super;

@@ -105,28 +173,28 @@ std::vector<string> find_superclasses(RecordDecl *decl)

/* Is decl marked as being part of an overloaded method?
*/
-bool is_overload(Decl *decl)
+bool generator::is_overload(Decl *decl)
{
return has_annotation(decl, "isl_overload");
}

/* Is decl marked as a constructor?
*/
-bool is_constructor(Decl *decl)
+bool generator::is_constructor(Decl *decl)
{
return has_annotation(decl, "isl_constructor");
}

/* Is decl marked as consuming a reference?
*/
-bool takes(Decl *decl)
+bool generator::takes(Decl *decl)
{
return has_annotation(decl, "isl_take");
}

/* Is decl marked as returning a reference that is required to be freed.
*/
-bool gives(Decl *decl)
+bool generator::gives(Decl *decl)
{
return has_annotation(decl, "isl_give");
}
@@ -134,7 +202,7 @@ bool gives(Decl *decl)
/* Return the class that has a name that matches the initial part
* of the name of function "fd" or NULL if no such class could be found.
*/
-isl_class *method2class(map<string, isl_class> &classes,
+isl_class *generator::method2class(map<string, isl_class> &classes,
FunctionDecl *fd)
{
string best;
@@ -156,7 +224,7 @@ isl_class *method2class(map<string, isl_class> &classes,

/* Is "type" the type "isl_ctx *"?
*/
-bool is_isl_ctx(QualType type)
+bool generator::is_isl_ctx(QualType type)
{
if (!type->isPointerType())
return 0;
@@ -169,7 +237,7 @@ bool is_isl_ctx(QualType type)

/* Is the first argument of "fd" of type "isl_ctx *"?
*/
-bool first_arg_is_isl_ctx(FunctionDecl *fd)
+bool generator::first_arg_is_isl_ctx(FunctionDecl *fd)
{
ParmVarDecl *param;

@@ -182,7 +250,7 @@ bool first_arg_is_isl_ctx(FunctionDecl *fd)

/* Is "type" that of a pointer to an isl_* structure?
*/
-bool is_isl_type(QualType type)
+bool generator::is_isl_type(QualType type)
{
if (type->isPointerType()) {
string s;
@@ -199,7 +267,7 @@ bool is_isl_type(QualType type)

/* Is "type" the type isl_bool?
*/
-bool is_isl_bool(QualType type)
+bool generator::is_isl_bool(QualType type)
{
string s;

@@ -212,7 +280,7 @@ bool is_isl_bool(QualType type)

/* Is "type" that of a pointer to char.
*/
-bool is_string_type(QualType type)
+bool generator::is_string_type(QualType type)
{
if (type->isPointerType()) {
string s;
@@ -229,7 +297,7 @@ bool is_string_type(QualType type)

/* Is "type" that of a pointer to a function?
*/
-bool is_callback(QualType type)
+bool generator::is_callback(QualType type)
{
if (!type->isPointerType())
return false;
@@ -239,7 +307,7 @@ bool is_callback(QualType type)

/* Is "type" that of "char *" of "const char *"?
*/
-bool is_string(QualType type)
+bool generator::is_string(QualType type)
{
if (type->isPointerType()) {
string s = type->getPointeeType().getAsString();
@@ -252,7 +320,7 @@ bool is_string(QualType type)
/* Return the name of the type that "type" points to.
* The input "type" is assumed to be a pointer type.
*/
-string extract_type(QualType type)
+string generator::extract_type(QualType type)
{
if (type->isPointerType())
return type->getPointeeType().getAsString();
diff --git a/interface/generator.h b/interface/generator.h
index d577b4d..b55de99 100644
--- a/interface/generator.h
+++ b/interface/generator.h
@@ -24,35 +24,44 @@ struct isl_class {
map<string, set<FunctionDecl *> > methods;
FunctionDecl *fn_to_str;
FunctionDecl *fn_free;
-
- bool is_static(FunctionDecl *method);
-
- void print(map<string, isl_class> &classes, set<string> &done);
- void print_constructor(FunctionDecl *method);
- void print_representation(const string &python_name);
- void print_method_type(FunctionDecl *fd);
- void print_method_types();
- void print_method(FunctionDecl *method, vector<string> super);
- void print_method_overload(FunctionDecl *method, vector<string> super);
- void print_method(const string &fullname,
- const set<FunctionDecl *> &methods, vector<string> super);
};

-void die(const char *msg) __attribute__((noreturn));
-string drop_type_suffix(string name, FunctionDecl *method);
-vector<string> find_superclasses(RecordDecl *decl);
-bool is_overload(Decl *decl);
-bool is_constructor(Decl *decl);
-bool takes(Decl *decl);
-bool gives(Decl *decl);
-isl_class *method2class(map<string, isl_class> &classes, FunctionDecl *fd);
-bool is_isl_ctx(QualType type);
-bool first_arg_is_isl_ctx(FunctionDecl *fd);
-bool is_isl_type(QualType type);
-bool is_isl_bool(QualType type);
-bool is_string_type(QualType type);
-bool is_callback(QualType type);
-bool is_string(QualType type);
-string extract_type(QualType type);
+/* Base class for interface generators.
+ */
+class generator {
+protected:
+ map<string,isl_class> classes;
+ map<string, FunctionDecl *> functions_by_name;
+
+public:
+ generator(set<RecordDecl *> &exported_types,
+ set<FunctionDecl *> exported_functions,
+ set<FunctionDecl *> functions);
+
+ virtual void generate() = 0;
+ virtual ~generator() {};
+
+protected:
+ void print_class_header(const isl_class &clazz, const string &name,
+ const vector<string> &super);
+ string drop_type_suffix(string name, FunctionDecl *method);
+ void die(const char *msg) __attribute__((noreturn));
+ vector<string> find_superclasses(RecordDecl *decl);
+ bool is_overload(Decl *decl);
+ bool is_constructor(Decl *decl);
+ bool takes(Decl *decl);
+ bool gives(Decl *decl);
+ isl_class *method2class(map<string, isl_class> &classes,
+ FunctionDecl *fd);
+ bool is_isl_ctx(QualType type);
+ bool first_arg_is_isl_ctx(FunctionDecl *fd);
+ bool is_isl_type(QualType type);
+ bool is_isl_bool(QualType type);
+ bool is_string_type(QualType type);
+ bool is_callback(QualType type);
+ bool is_string(QualType type);
+ bool is_static(const isl_class &clazz, FunctionDecl *method);
+ string extract_type(QualType type);
+};

#endif /* ISL_INTERFACE_GENERATOR_H */
diff --git a/interface/python.cc b/interface/python.cc
index 4ec166f..3b23c79 100644
--- a/interface/python.cc
+++ b/interface/python.cc
@@ -39,25 +39,17 @@
#include <vector>
#include "python.h"

-/* Drop the "isl_" initial part of the type name "name".
- */
-static string type2python(string name)
+python_generator::python_generator( set<RecordDecl *> &exported_types,
+ set<FunctionDecl *> exported_functions, set<FunctionDecl *> functions)
+ : generator(exported_types, exported_functions, functions)
{
- return name.substr(4);
}

-/* Should "method" be considered to be a static method?
- * That is, is the first argument something other than
- * an instance of the class?
+/* Drop the "isl_" initial part of the type name "name".
*/
-bool isl_class::is_static(FunctionDecl *method)
+static string type2python(string name)
{
- ParmVarDecl *param = method->getParamDecl(0);
- QualType type = param->getOriginalType();
-
- if (!is_isl_type(type))
- return true;
- return extract_type(type) != name;
+ return name.substr(4);
}

/* Print the header of the method "name" with "n_arg" arguments.
@@ -66,7 +58,8 @@ bool isl_class::is_static(FunctionDecl *method)
* If the method is called "from", then rename it to "convert_from"
* because "from" is a python keyword.
*/
-static void print_method_header(bool is_static, const string &name, int n_arg)
+void python_generator::print_method_header(bool is_static, const string &name,
+ int n_arg)
{
const char *s;

@@ -95,8 +88,8 @@ static void print_method_header(bool is_static, const string &name, int n_arg)
* If "upcast" is not set, then the "super", "name" and "n" arguments
* to this function are ignored.
*/
-static void print_type_check(const string &type, int pos, bool upcast,
- const string &super, const string &name, int n)
+void python_generator::print_type_check(const string &type, int pos,
+ bool upcast, const string &super, const string &name, int n)
{
printf(" try:\n");
printf(" if not arg%d.__class__ is %s:\n",
@@ -125,7 +118,7 @@ static void print_type_check(const string &type, int pos, bool upcast,
* If any exception is thrown, the wrapper keeps track of it in exc_info[0]
* and returns -1. Otherwise the wrapper returns 0.
*/
-static void print_callback(QualType type, int arg)
+void python_generator::print_callback(QualType type, int arg)
{
const FunctionProtoType *fn = type->getAs<FunctionProtoType>();
unsigned n_arg = fn->getNumArgs();
@@ -179,7 +172,7 @@ static void print_callback(QualType type, int arg)
* Otherwise, if the argument is a pointer, then pass this pointer itself.
* Otherwise, pass the argument directly.
*/
-static void print_arg_in_call(FunctionDecl *fd, int arg, int skip)
+void python_generator::print_arg_in_call(FunctionDecl *fd, int arg, int skip)
{
ParmVarDecl *param = fd->getParamDecl(arg);
QualType type = param->getOriginalType();
@@ -205,7 +198,7 @@ static void print_arg_in_call(FunctionDecl *fd, int arg, int skip)
* If the return type is isl_bool, then convert the result to
* a Python boolean, raising an error on isl_bool_error.
*/
-static void print_method_return(FunctionDecl *method)
+void python_generator::print_method_return(FunctionDecl *method)
{
QualType return_type = method->getReturnType();

@@ -256,10 +249,11 @@ static void print_method_return(FunctionDecl *method)
* If the function consumes a reference, then we pass it a copy of
* the actual argument.
*/
-void isl_class::print_method(FunctionDecl *method, vector<string> super)
+void python_generator::print_method(const isl_class &clazz,
+ FunctionDecl *method, vector<string> super)
{
string fullname = method->getName();
- string cname = fullname.substr(name.length() + 1);
+ string cname = fullname.substr(clazz.name.length() + 1);
int num_params = method->getNumParams();
int drop_user = 0;
int drop_ctx = first_arg_is_isl_ctx(method);
@@ -271,7 +265,7 @@ void isl_class::print_method(FunctionDecl *method, vector<string> super)
drop_user = 1;
}

- print_method_header(is_static(method), cname,
+ print_method_header(is_static(clazz, method), cname,
num_params - drop_ctx - drop_user);

for (int i = drop_ctx; i < num_params; ++i) {
@@ -328,15 +322,15 @@ void isl_class::print_method(FunctionDecl *method, vector<string> super)
* the python method correspond to the arguments expected by "method"
* and to call "method" if they do.
*/
-void isl_class::print_method_overload(FunctionDecl *method,
- vector<string> super)
+void python_generator::print_method_overload(const isl_class &clazz,
+ FunctionDecl *method, vector<string> super)
{
string fullname = method->getName();
int num_params = method->getNumParams();
int first;
string type;

- first = is_static(method) ? 0 : 1;
+ first = is_static(clazz, method) ? 0 : 1;

printf(" if ");
for (int i = first; i < num_params; ++i) {
@@ -372,8 +366,9 @@ void isl_class::print_method_overload(FunctionDecl *method,
* Otherwise, print an overloaded method with pieces corresponding
* to each function in "methods".
*/
-void isl_class::print_method(const string &fullname,
- const set<FunctionDecl *> &methods, vector<string> super)
+void python_generator::print_method(const isl_class &clazz,
+ const string &fullname, const set<FunctionDecl *> &methods,
+ vector<string> super)
{
string cname;
set<FunctionDecl *>::const_iterator it;
@@ -382,17 +377,17 @@ void isl_class::print_method(const string &fullname,

any_method = *methods.begin();
if (methods.size() == 1 && !is_overload(any_method)) {
- print_method(any_method, super);
+ print_method(clazz, any_method, super);
return;
}

- cname = fullname.substr(name.length() + 1);
+ cname = fullname.substr(clazz.name.length() + 1);
num_params = any_method->getNumParams();

- print_method_header(is_static(any_method), cname, num_params);
+ print_method_header(is_static(clazz, any_method), cname, num_params);

for (it = methods.begin(); it != methods.end(); ++it)
- print_method_overload(*it, super);
+ print_method_overload(clazz, *it, super);
}

/* Print part of the constructor for this isl_class.
@@ -404,10 +399,11 @@ void isl_class::print_method(const string &fullname,
* If the function consumes a reference, then we pass it a copy of
* the actual argument.
*/
-void isl_class::print_constructor(FunctionDecl *cons)
+void python_generator::print_constructor(const isl_class &clazz,
+ FunctionDecl *cons)
{
string fullname = cons->getName();
- string cname = fullname.substr(name.length() + 1);
+ string cname = fullname.substr(clazz.name.length() + 1);
int num_params = cons->getNumParams();
int drop_ctx = first_arg_is_isl_ctx(cons);

@@ -452,7 +448,8 @@ void isl_class::print_constructor(FunctionDecl *cons)

/* Print the header of the class "name" with superclasses "super".
*/
-static void print_class_header(const string &name, const vector<string> &super)
+void python_generator::print_class_header(const isl_class &clazz,
+ const string &name, const vector<string> &super)
{
printf("class %s", name.c_str());
if (super.size() > 0) {
@@ -474,7 +471,7 @@ static void print_class_header(const string &name, const vector<string> &super)
* then tell ctypes it returns a "c_bool".
* If "fd" returns a char *, then simply tell ctypes.
*/
-static void print_restype(FunctionDecl *fd)
+void python_generator::print_restype(FunctionDecl *fd)
{
string fullname = fd->getName();
QualType type = fd->getReturnType();
@@ -488,7 +485,7 @@ static void print_restype(FunctionDecl *fd)

/* Tell ctypes about the types of the arguments of the function "fd".
*/
-static void print_argtypes(FunctionDecl *fd)
+void python_generator::print_argtypes(FunctionDecl *fd)
{
string fullname = fd->getName();
int n = fd->getNumParams();
@@ -518,7 +515,7 @@ static void print_argtypes(FunctionDecl *fd)

/* Print type definitions for the method 'fd'.
*/
-void isl_class::print_method_type(FunctionDecl *fd)
+void python_generator::print_method_type(FunctionDecl *fd)
{
print_restype(fd);
print_argtypes(fd);
@@ -536,15 +533,16 @@ void isl_class::print_method_type(FunctionDecl *fd)
* Check the type of the argument before calling the *_to_str function
* on it in case the method was called on an object from a subclass.
*/
-void isl_class::print_representation(const string &python_name)
+void python_generator::print_representation(const isl_class &clazz,
+ const string &python_name)
{
- if (!fn_to_str)
+ if (!clazz.fn_to_str)
return;

printf(" def __str__(arg0):\n");
print_type_check(python_name, 0, false, "", "", -1);
printf(" ptr = isl.%s(arg0.ptr)\n",
- string(fn_to_str->getName()).c_str());
+ string(clazz.fn_to_str->getName()).c_str());
printf(" res = str(cast(ptr, c_char_p).value)\n");
printf(" libc.free(ptr)\n");
printf(" return res\n");
@@ -566,21 +564,22 @@ void isl_class::print_representation(const string &python_name)
* Assuming each exported class has a *_free method,
* also unconditionally set the type of such methods.
*/
-void isl_class::print_method_types()
+void python_generator::print_method_types(const isl_class &clazz)
{
- set<FunctionDecl *>::iterator in;
- map<string, set<FunctionDecl *> >::iterator it;
+ set<FunctionDecl *>::const_iterator in;
+ map<string, set<FunctionDecl *> >::const_iterator it;

- for (in = constructors.begin(); in != constructors.end(); ++in)
+ for (in = clazz.constructors.begin(); in != clazz.constructors.end();
+ ++in)
print_method_type(*in);

- for (it = methods.begin(); it != methods.end(); ++it)
+ for (it = clazz.methods.begin(); it != clazz.methods.end(); ++it)
for (in = it->second.begin(); in != it->second.end(); ++in)
print_method_type(*in);

- print_method_type(fn_free);
- if (fn_to_str)
- print_method_type(fn_to_str);
+ print_method_type(clazz.fn_free);
+ if (clazz.fn_to_str)
+ print_method_type(clazz.fn_to_str);
}

/* Print out the definition of this isl_class.
@@ -599,20 +598,20 @@ void isl_class::print_method_types()
* constructor functions and the return types of those function returning
* an isl object.
*/
-void isl_class::print(map<string, isl_class> &classes, set<string> &done)
+void python_generator::print(const isl_class &clazz)
{
- string p_name = type2python(name);
- set<FunctionDecl *>::iterator in;
- map<string, set<FunctionDecl *> >::iterator it;
- vector<string> super = find_superclasses(type);
+ string p_name = type2python(clazz.name);
+ set<FunctionDecl *>::const_iterator in;
+ map<string, set<FunctionDecl *> >::const_iterator it;
+ vector<string> super = find_superclasses(clazz.type);

for (unsigned i = 0; i < super.size(); ++i)
if (done.find(super[i]) == done.end())
- classes[super[i]].print(classes, done);
- done.insert(name);
+ print(classes[super[i]]);
+ done.insert(clazz.name);

printf("\n");
- print_class_header(p_name, super);
+ print_class_header(clazz, p_name, super);
printf(" def __init__(self, *args, **keywords):\n");

printf(" if \"ptr\" in keywords:\n");
@@ -620,85 +619,37 @@ void isl_class::print(map<string, isl_class> &classes, set<string> &done)
printf(" self.ptr = keywords[\"ptr\"]\n");
printf(" return\n");

- for (in = constructors.begin(); in != constructors.end(); ++in)
- print_constructor(*in);
+ for (in = clazz.constructors.begin(); in != clazz.constructors.end();
+ ++in)
+ print_constructor(clazz, *in);
printf(" raise Error\n");
printf(" def __del__(self):\n");
printf(" if hasattr(self, 'ptr'):\n");
- printf(" isl.%s_free(self.ptr)\n", name.c_str());
+ printf(" isl.%s_free(self.ptr)\n", clazz.name.c_str());

- print_representation(p_name);
+ print_representation(clazz, p_name);

- for (it = methods.begin(); it != methods.end(); ++it)
- print_method(it->first, it->second, super);
+ for (it = clazz.methods.begin(); it != clazz.methods.end(); ++it)
+ print_method(clazz, it->first, it->second, super);

printf("\n");

- print_method_types();
+ print_method_types(clazz);
}

-/* Generate a python interface based on the extracted types and functions.
- * We first collect all functions that belong to a certain type,
- * separating constructors from regular methods and keeping track
- * of the _to_str and _free functions, if any, separately. If there are any
- * overloaded functions, then they are grouped based on their name
- * after removing the argument type suffix.
+/* Generate a python interface based on the extracted types and
+ * functions.
*
- * Then we print out each class in turn. If one of these is a subclass
- * of some other class, it will make sure the superclass is printed out first.
+ * Print out each class in turn. If one of these is a subclass of some
+ * other class, make sure the superclass is printed out first.
+ * functions.
*/
-void generate_python(set<RecordDecl *> &exported_types,
- set<FunctionDecl *> exported_functions, set<FunctionDecl *> functions)
+void python_generator::generate()
{
- map<string, isl_class> classes;
map<string, isl_class>::iterator ci;
- set<string> done;
- map<string, FunctionDecl *> functions_by_name;
-
- set<FunctionDecl *>::iterator in;
- for (in = functions.begin(); in != functions.end(); ++in) {
- FunctionDecl *decl = *in;
- functions_by_name[decl->getName()] = decl;
- }
-
- set<RecordDecl *>::iterator it;
- for (it = exported_types.begin(); it != exported_types.end(); ++it) {
- RecordDecl *decl = *it;
- map<string, FunctionDecl *>::iterator i;
-
- string name = decl->getName();
- classes[name].name = name;
- classes[name].type = decl;
- classes[name].fn_to_str = NULL;
- classes[name].fn_free = NULL;
-
- i = functions_by_name.find(name + "_to_str");
- if (i != functions_by_name.end())
- classes[name].fn_to_str = i->second;
-
- i = functions_by_name.find (name + "_free");
- if (i == functions_by_name.end())
- die("No _free function found");
- classes[name].fn_free = i->second;
- }
-
- for (in = exported_functions.begin(); in != exported_functions.end();
- ++in) {
- isl_class *c = method2class(classes, *in);
- if (!c)
- continue;
- if (is_constructor(*in)) {
- c->constructors.insert(*in);
- } else {
- FunctionDecl *method = *in;
- string fullname = method->getName();
- fullname = drop_type_suffix(fullname, method);
- c->methods[fullname].insert(method);
- }
- }

for (ci = classes.begin(); ci != classes.end(); ++ci) {
if (done.find(ci->first) == done.end())
- ci->second.print(classes, done);
+ print(ci->second);
}
}
diff --git a/interface/python.h b/interface/python.h
index 1706dd7..ea361b0 100644
--- a/interface/python.h
+++ b/interface/python.h
@@ -3,5 +3,40 @@
using namespace std;
using namespace clang;

-void generate_python(set<RecordDecl *> &exported_types,
- set<FunctionDecl *> exported_functions, set<FunctionDecl *> functions);
+class python_generator : public generator {
+private:
+ set<string> done;
+
+public:
+ python_generator(set<RecordDecl *> &exported_types,
+ set<FunctionDecl *> exported_functions,
+ set<FunctionDecl *> functions);
+
+ virtual void generate() override;
+
+private:
+ void print(const isl_class &clazz);
+ void print_method_header(bool is_static, const string &name, int n_arg);
+ void print_class_header(const isl_class &clazz, const string &name,
+ const vector<string> &super);
+ void print_type_check(const string &type, int pos, bool upcast,
+ const string &super, const string &name, int n);
+ void print_callback(QualType type, int arg);
+ void print_arg_in_call(FunctionDecl *fd, int arg, int skip);
+ void print_argtypes(FunctionDecl *fd);
+ void print_method_return(FunctionDecl *method);
+ void print_restype(FunctionDecl *fd);
+ void print(map<string, isl_class> &classes, set<string> &done);
+ void print_constructor(const isl_class &clazz, FunctionDecl *method);
+ void print_representation(const isl_class &clazz,
+ const string &python_name);
+ void print_method_type(FunctionDecl *fd);
+ void print_method_types(const isl_class &clazz);
+ void print_method(const isl_class &clazz, FunctionDecl *method,
+ vector<string> super);
+ void print_method_overload(const isl_class &clazz,
+ FunctionDecl *method, vector<string> super);
+ void print_method(const isl_class &clazz, const string &fullname,
+ const set<FunctionDecl *> &methods, vector<string> super);
+
+};
--
2.7.4

Tobias Grosser

unread,
Oct 19, 2016, 2:39:09 PM10/19/16
to isl-dev...@googlegroups.com, Armin Groesslinger, Andreas Simbuerger, Tobias Grosser
From: Armin Groesslinger <armin.gro...@uni-passau.de>

Add option "--language" to specify for which language (currently only
"python") to generate bindings.

Signed-off-by: Armin Groesslinger <armin.gro...@uni-passau.de>
Signed-off-by: Andreas Simbuerger <simb...@fim.uni-passau.de>
Signed-off-by: Tobias Grosser <tob...@grosser.es>
---
interface/Makefile.am | 3 ++-
interface/extract_interface.cc | 18 +++++++++++++++---
2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/interface/Makefile.am b/interface/Makefile.am
index a37cb18..4d2b733 100644
--- a/interface/Makefile.am
+++ b/interface/Makefile.am
@@ -29,7 +29,8 @@ test: extract_interface

isl.py: extract_interface isl.py.top
(cat $(srcdir)/isl.py.top; \
- ./extract_interface$(EXEEXT) $(includes) $(srcdir)/all.h) \
+ ./extract_interface$(EXEEXT) --language=python $(includes) \
+ $(srcdir)/all.h) \
> isl.py

dist-hook: isl.py
diff --git a/interface/extract_interface.cc b/interface/extract_interface.cc
index ddca368..1a05520 100644
--- a/interface/extract_interface.cc
+++ b/interface/extract_interface.cc
@@ -91,6 +91,11 @@ static llvm::cl::list<string> Includes("I",
llvm::cl::desc("Header search path"),
llvm::cl::value_desc("path"), llvm::cl::Prefix);

+static llvm::cl::opt<string> Language(llvm::cl::Required,
+ llvm::cl::ValueRequired, "language",
+ llvm::cl::desc("Bindings to generate"),
+ llvm::cl::value_desc("python"));
+
static const char *ResourceDir =
CLANG_PREFIX "/lib/clang/" CLANG_VERSION_STRING;

@@ -423,9 +428,16 @@ int main(int argc, char *argv[])
ParseAST(*sema);
Diags.getClient()->EndSourceFile();

- python_generator gen(consumer.exported_types, consumer.exported_functions,
- consumer.functions);
- gen.generate();
+ generator *gen = 0;
+ if (Language.compare("python") == 0)
+ gen = new python_generator(consumer.exported_types,
+ consumer.exported_functions, consumer.functions);
+ else
+ cerr << "Language '" << Language << "' not recognized." << endl
+ << "Not generating bindings." << endl;
+
+ if (gen)
+ gen->generate();

delete sema;
delete Clang;
--
2.7.4

Tobias Grosser

unread,
Oct 19, 2016, 2:39:11 PM10/19/16
to isl-dev...@googlegroups.com, Tobias Grosser
.. which generates for each exported isl object a smart pointer class of
the following form:

class Set;
inline Set manage(__isl_take isl_set *Set);

class Set {
friend inline Set manage(__isl_take isl_set *Set);

isl_set *Ptr = nullptr;

inline explicit Set(__isl_take isl_set *Ptr);

public:
inline Set();
inline Set(const Set &Obj);
inline Set& operator=(Set Obj);
inline ~Set();
inline __isl_give isl_set *copy() const &;
inline __isl_give isl_set *copy() && = delete;
inline isl_set *get() const;
inline __isl_give isl_set *release() const;
};

The interface and functionality resembles C++11 std::unique_ptr. The
isl smart pointer keeps unique ownership of an object. The currently
held pointer can be observed by calling get(). When calling release(),
both the current pointer is returned and ownership is released. Names
and semantics of get() and release() match the corresponding methods in
std::unique_ptr. In addition, a copy() method is provided, which returns
a pointer to a copy of the managed object. These three methods allow the
isl C++ objects to be used together with the isl C functions.

Example:

void foo(Set S) {

isl_set_is_universe(S.get());

isl_set_free(S.copy());

isl_set_free(S.release());
}

An alternative choice to name get() and release() would have been keep()
and take(). The name 'take' is for example used in llvm::OwningPtr. For
consistency with std::unique_ptr, this alternative naming has not been
choosen.

isl C++ objects can be constructed either by copying existing isl
objects of the same type or by passing an isl C pointer to a global
constructor method isl::manage(isl_type *ptr). Constructing an
isl C++ object from an isl C pointer using an explicit class constructor
is not allowed. This interface has been choosen in correspondence with
std::make_unique. Besides making object construction and especially
pointer consumption explicit, global constructor methods also make
specifying the needed object type unnecessary. We also allow default
construction of empty isl objects, as default-constructabity is
important when using isl objects in C++ containers and especially
when exploiting C++ move semantics. To complete the "Big Three" we also
define the copy assignment operator. The remaining two functions needed
to complete the "Big Five", move constructor and move assignment, are not
needed for an initial implementation and are left for a subsequent
commit.

Example:

void foo (__isl_take *s1, __isl_take *s2) {

isl::Set S1 = isl::manage(s1);

auto S = isl::manage(s2);

auto S2 = S1;

isl::Set EmptySet;
}

The currently generated library is header-only, which resembles the
already existing python bindings and is the best choice for a
lightweight interface library. In the future, implementations for
larger functions or debugging methods might require the addition
of implementations as part of a compiled (not-header-only) library.

The currently generated library is generated in a single unified header
file, which matches the generated python bindings. In the future,
this might evolve to a combination of per-class header files and a
unifying header file, as there is interest in both. However, for
an initial binding a unified header has been choosen.

No class hierarchy is constructed, as there is currently no obvious
benefit and a hierarchy can always be introduced later. However,
most conversions are probably better implemented through implicit
constructors from related isl objects. Such constructors are planned
to be added in the future.

All functions are declared 'inline' to not cause link errors when
included in multiple translation units.

The public interfaces have currently no defined behavior in case isl
ptrs are NULL, but instead assert when used on a NULL pointer. We
explicitly do not support isl's ability to handle NULL pointers
gracefully to ensure no software relies on this feature, which could
block the possible introduction of a more C++ style error handling
approach later on.

This change currently only enables C++ interface generation for sets
and maps, as only this part has been throughly tested. Other isl
datatypes will be exposed in future commits.

This change intentionally only introduces a minimalistic interface. The
interface established with this change is already sufficient to manage
isl pointers. Polly today uses an isl C++ interface that has about the
same functionality as this change and already the simplified memory
management has proven to facilitate software development a lot. The
infrastructur established with this change will serve as basis for a
future function call interface that makes isl C functions conveniently
accessible through the C++ interface.

File IO is performed through ostream, as this makes it easy to split
the bindings into multiple files later on. However, we still use C
style string formatting as this is more readable and closer to the
python binding generator. This approach is similar to the string
generation in Armin Groesslinger's SCALA bindings.

The design of the proposed isl C++ smart pointer interface is the result
of discussions with Michael Kruse, Jakob Leben, and Alexandre Isoard. It
was inspired by earlier bindings developed by Michael, Jakob, and
Alexandre, but also the C++ bindings developed by Andreas Simbuerger,
the SCALA bindings from Armin Groesslinger, as well as islpy developed
by Andreas Kloeckner. The actual code generator has been newly
developed, but relies on three enabling patches from Armin Groesslinger,
which establish an infrastructure also used in Armin's and Andreas'
bindings. As a result, this change also will make it easier to upstream
Armin's SCALA bindings and the similar structure of this new code
generator should make it easy to port changes from Andreas' C++
generator.

Signed-off-by: Tobias Grosser <tob...@grosser.es>
---
interface/Makefile.am | 12 +-
interface/cpp.cc | 349 +++++++++++++++++++++++++++++++++++++++++
interface/cpp.h | 175 +++++++++++++++++++++
interface/extract_interface.cc | 4 +
interface/isl.h.top | 4 +
5 files changed, 542 insertions(+), 2 deletions(-)
create mode 100644 interface/cpp.cc
create mode 100644 interface/cpp.h
create mode 100644 interface/isl.h.top

diff --git a/interface/Makefile.am b/interface/Makefile.am
index 4d2b733..3d7642b 100644
--- a/interface/Makefile.am
+++ b/interface/Makefile.am
@@ -14,6 +14,8 @@ extract_interface_SOURCES = \
generator.cc \
python.h \
python.cc \
+ cpp.h \
+ cpp.cc \
extract_interface.h \
extract_interface.cc
extract_interface_LDADD = \
@@ -33,5 +35,11 @@ isl.py: extract_interface isl.py.top
$(srcdir)/all.h) \
> isl.py

-dist-hook: isl.py
- cp isl.py $(distdir)/
+isl.h: extract_interface all.h isl.h.top
+ (cat $(srcdir)/isl.h.top; \
+ ./extract_interface$(EXEEXT) --language=cpp $(includes) \
+ $(srcdir)/all.h) \
+ > isl.h
+
+dist-hook: isl.py isl.hh
+ cp isl.py $(distdir)/ ; cp isl.hh $(distdir)
diff --git a/interface/cpp.cc b/interface/cpp.cc
new file mode 100644
index 0000000..ed69abc
--- /dev/null
+++ b/interface/cpp.cc
@@ -0,0 +1,349 @@
+/*
+ * Copyright 2016 Tobias Grosser. All rights reserved.
+#include "isl_config.h"
+
+#include <stdio.h>
+#include <iostream>
+#include <map>
+#include <vector>
+#include "cpp.h"
+
+/* Print string formatted according to "fmt" to ostream "os".
+ *
+ * This fprintf method allows us to use printf style formatting constructs when
+ * writing to an ostream.
+ */
+void fprintf(ostream &os, const char *format, ...)
+{
+ va_list arguments;
+ FILE *string_stream;
+ char *string_pointer;
+ size_t size;
+
+ va_start(arguments, format);
+
+ string_stream = open_memstream(&string_pointer, &size);
+
+ if (!string_stream) {
+ fprintf(stderr, "open_memstream failed -- aborting!\n");
+ exit(1);
+ }
+
+ vfprintf(string_stream, format, arguments);
+ fclose(string_stream);
+ os << string_pointer;
+}
+
+cpp_generator::cpp_generator( set<RecordDecl *> &exported_types,
+ set<FunctionDecl *> exported_functions, set<FunctionDecl *> functions)
+ : generator(exported_types, exported_functions, functions)
+{
+}
+
+void cpp_generator::generate()
+{
+ ostream &os = cout;
+
+ fprintf(os, "\n");
+ fprintf(os, "#ifndef ISL_CPP_ALL\n");
+ fprintf(os, "#define ISL_CPP_ALL\n\n");
+ fprintf(os, "namespace isl {\n\n");
+
+ print_forward_declarations(os);
+ print_declarations(os);
+ print_implementations(os);
+
+ fprintf(os, "};\n\n");
+ fprintf(os, "#endif /* ISL_CPP_ALL */\n");
+}
+
+void cpp_generator::print_forward_declarations(ostream &os)
+{
+ map<string, isl_class>::iterator ci;
+
+ fprintf(os, "// forward declarations\n");
+
+ for (ci = classes.begin(); ci != classes.end(); ++ci)
+ if (is_supported_class(ci->second.name))
+ print_class_forward_decl(os, ci->second);
+ fprintf(os, "\n");
+}
+
+void cpp_generator::print_declarations(ostream &os)
+{
+ map<string, isl_class>::iterator ci;
+
+ for (ci = classes.begin(); ci != classes.end(); ++ci)
+ print_class(os, ci->second);
+}
+
+void cpp_generator::print_implementations(ostream &os)
+{
+ map<string, isl_class>::iterator ci;
+
+ for (ci = classes.begin(); ci != classes.end(); ++ci)
+ print_class_impl(os, ci->second);
+}
+
+void cpp_generator::print_class(ostream &os, const isl_class &clazz)
+{
+ const char *name = clazz.name.c_str();
+ std::string cppstring = type2cpp(clazz.name);
+ const char *cppname = cppstring.c_str();
+
+ if (!is_supported_class(clazz.name))
+ return;
+
+ fprintf(os, "// declarations for isl::%s\n", cppname);
+
+ print_class_global_constructor(os, clazz);
+ fprintf(os, "class %s {\n", cppname);
+ fprintf(os, " friend ");
+ print_class_global_constructor(os, clazz);
+ fprintf(os, " %s *Ptr = nullptr;\n", name);
+ fprintf(os, "\n");
+ print_private_constructors(os, clazz);
+ fprintf(os, "\n");
+ fprintf(os, "public:\n");
+ print_public_constructors(os, clazz);
+ print_copy_assignment(os, clazz);
+ print_destructor(os, clazz);
+ print_ptr(os, clazz);
+
+ fprintf(os, "};\n\n");
+}
+
+void cpp_generator::print_class_forward_decl(ostream &os,
+ const isl_class &clazz)
+{
+ std::string cppstring = type2cpp(clazz.name);
+ const char *cppname = cppstring.c_str();
+
+ fprintf(os, "class %s;\n", cppname);
+}
+
+void cpp_generator::print_class_global_constructor(ostream &os,
+ const isl_class &clazz)
+{
+ const char *name = clazz.name.c_str();
+ std::string cppstring = type2cpp(clazz.name);
+ const char *cppname = cppstring.c_str();
+
+ fprintf(os, "inline %s manage(__isl_take %s *Ptr);\n\n", cppname,
+ name);
+}
+
+void cpp_generator::print_private_constructors(ostream &os,
+ const isl_class &clazz)
+{
+ const char *name = clazz.name.c_str();
+ std::string cppstring = type2cpp(clazz.name);
+ const char *cppname = cppstring.c_str();
+ fprintf(os, " inline explicit %s(__isl_take %s *Ptr);\n", cppname,
+ name);
+}
+
+void cpp_generator::print_public_constructors(ostream &os,
+ const isl_class &clazz)
+{
+ std::string cppstring = type2cpp(clazz.name);
+ const char *cppname = cppstring.c_str();
+ fprintf(os, " inline %s();\n", cppname);
+ fprintf(os, " inline %s(const %s &Obj);\n", cppname, cppname);
+}
+
+void cpp_generator::print_copy_assignment(ostream &os,
+ const isl_class &clazz)
+{
+ std::string cppstring = type2cpp(clazz.name);
+ const char *cppname = cppstring.c_str();
+ fprintf(os, " inline %s& operator=(%s Obj);\n", cppname, cppname);
+}
+
+void cpp_generator::print_destructor(ostream &os, const isl_class &clazz)
+{
+ std::string cppstring = type2cpp(clazz.name);
+ const char *cppname = cppstring.c_str();
+ fprintf(os, " inline ~%s();\n", cppname);
+}
+
+void cpp_generator::print_ptr(ostream &os, const isl_class &clazz)
+{
+ const char *name = clazz.name.c_str();
+ fprintf(os, " inline __isl_give %s *copy() const &;\n", name);
+ fprintf(os, " inline __isl_give %s *copy() && = delete;\n", name);
+ fprintf(os, " inline %s *get() const;\n", name);
+ fprintf(os, " inline __isl_give %s *release();\n", name);
+}
+
+void cpp_generator::print_class_impl(ostream &os, const isl_class &clazz)
+{
+ std::string cppstring = type2cpp(clazz.name);
+ const char *cppname = cppstring.c_str();
+
+ if (!is_supported_class(clazz.name))
+ return;
+
+ fprintf(os, "// implementations for isl::%s\n", cppname);
+
+ print_class_global_constructor_impl(os, clazz);
+ print_public_constructors_impl(os, clazz);
+ print_private_constructors_impl(os, clazz);
+ print_copy_assignment_impl(os, clazz);
+ print_destructor_impl(os, clazz);
+ print_ptr_impl(os, clazz);
+}
+
+void cpp_generator::print_class_global_constructor_impl(ostream &os,
+ const isl_class &clazz)
+{
+ const char *name = clazz.name.c_str();
+ std::string cppstring = type2cpp(clazz.name);
+ const char *cppname = cppstring.c_str();
+
+ fprintf(os, "%s manage(__isl_take %s *Ptr) {\n", cppname, name);
+ fprintf(os, " assert(Ptr && \"NULL ptr cannot be managed\");\n");
+ fprintf(os, " return %s(Ptr);\n", cppname);
+ fprintf(os, "}\n\n");
+}
+
+void cpp_generator::print_private_constructors_impl(ostream &os,
+ const isl_class &clazz)
+{
+ const char *name = clazz.name.c_str();
+ std::string cppstring = type2cpp(clazz.name);
+ const char *cppname = cppstring.c_str();
+ fprintf(os, "%s::%s(__isl_take %s *Ptr) : Ptr(Ptr) {}\n\n", cppname,
+ cppname, name);
+}
+
+void cpp_generator::print_public_constructors_impl(ostream &os,
+ const isl_class &clazz)
+{
+ const char *name = clazz.name.c_str();
+ std::string cppstring = type2cpp(clazz.name);
+ const char *cppname = cppstring.c_str();
+ fprintf(os, "%s::%s() : Ptr(nullptr) {}\n\n", cppname, cppname);
+ fprintf(os, "%s::%s(const %s &Obj) : Ptr(Obj.copy()) {}\n\n",
+ cppname, cppname, cppname, name);
+}
+
+void cpp_generator::print_copy_assignment_impl(ostream &os,
+ const isl_class &clazz)
+{
+ const char *name = clazz.name.c_str();
+ std::string cppstring = type2cpp(clazz.name);
+ const char *cppname = cppstring.c_str();
+ fprintf(os, "%s& %s::operator=(%s Obj) {\n", cppname,
+ cppname, cppname);
+ fprintf(os, " std::swap(this->Ptr, Obj.Ptr);\n", name);
+ fprintf(os, " return *this;\n");
+ fprintf(os, "}\n\n");
+}
+
+void cpp_generator::print_destructor_impl(ostream &os,
+ const isl_class &clazz)
+{
+ const char *name = clazz.name.c_str();
+ std::string cppstring = type2cpp(clazz.name);
+ const char *cppname = cppstring.c_str();
+ fprintf(os, "%s::~%s() {\n", cppname, cppname);
+ fprintf(os, " if (Ptr)\n");
+ fprintf(os, " %s_free(Ptr);\n", name);
+ fprintf(os, "}\n\n");
+}
+
+void cpp_generator::print_ptr_impl(ostream &os, const isl_class &clazz)
+{
+ const char *name = clazz.name.c_str();
+ std::string cppstring = type2cpp(clazz.name);
+ const char *cppname = cppstring.c_str();
+ fprintf(os, "__isl_give %s *%s::copy() const & {\n", name, cppname);
+ fprintf(os, " assert(Ptr && \"NULL object cannot be copied\");\n");
+ fprintf(os, " return %s_copy(Ptr);\n", name);
+ fprintf(os, "}\n\n");
+ fprintf(os, "%s *%s::get() const {\n", name, cppname);
+ fprintf(os, " assert(Ptr && \"Cannot return ptr to NULL object\");\n");
+ fprintf(os, " return Ptr;\n");
+ fprintf(os, "}\n\n");
+ fprintf(os, "__isl_give %s *%s::release() {\n", name, cppname);
+ fprintf(os, " assert(Ptr && \"Cannot return ptr to NULL object\");\n");
+ fprintf(os, " %s *Tmp = Ptr;\n", name);
+ fprintf(os, " Ptr = nullptr;\n");
+ fprintf(os, " return Tmp;\n");
+ fprintf(os, "}\n\n");
+}
+
+string cpp_generator::to_camel_case(const string &input)
+{
+ string output;
+
+ bool uppercase = true;
+
+ for (const char &character : input) {
+
+ if (character == '_') {
+ uppercase = true;
+ continue;
+ }
+ if (uppercase) {
+ output.append(1, toupper(character));
+ uppercase = false;
+ } else {
+ output.append(1, character);
+ }
+ }
+
+ return output;
+}
+
+string cpp_generator::type2cpp(string name)
+{
+ return to_camel_case(name.substr(4));
+}
+
+std::vector<const char*> supported_classes = {
+ "isl_set",
+ "isl_map",
+};
+
+bool cpp_generator::is_supported_class(string name)
+{
+ for (size_t i = 0; i < supported_classes.size(); i++) {
+ if (name == supported_classes[i])
+ return true;
+ }
+
+ return false;
+}
diff --git a/interface/cpp.h b/interface/cpp.h
new file mode 100644
index 0000000..bc12cd9
--- /dev/null
+++ b/interface/cpp.h
@@ -0,0 +1,175 @@
+#include "generator.h"
+
+using namespace std;
+using namespace clang;
+
+class cpp_generator : public generator {
+public:
+ cpp_generator(set<RecordDecl *> &exported_types,
+ set<FunctionDecl *> exported_functions,
+ set<FunctionDecl *> functions);
+
+ virtual void generate() override;
+private:
+
+ /* Print forward declarations for all classes to "os".
+ */
+ void print_forward_declarations(ostream &os);
+
+ /* Print all declarations to "os".
+ */
+ void print_declarations(ostream &os);
+
+ /* Print declarations for class "clazz" to "os".
+ */
+ void print_class(ostream &os, const isl_class &clazz);
+
+ /* Print forward declaration of class "clazz" to "os".
+ */
+ void print_class_forward_decl(ostream &os, const isl_class &clazz);
+
+ /* Print global constructor method to "os".
+ *
+ * Each class has one global constructor:
+ *
+ * Set manage(__isl_take isl_set *Ptr);
+ *
+ * The only public way to construct isl C++ objects from a raw pointer
+ * is through this global constructor method. This ensures isl object
+ * constructions is very explicit and pointers do not converted by
+ * accident. Due to overloading manage() can be called on any isl
+ * raw pointer and the corresponding object is automatically
+ * constructure, without the user having to choose the right isl object
+ * type.
+ *
+ */
+ void print_class_global_constructor(ostream &os,
+ const isl_class &clazz);
+
+ /* Print declarations of private constructors for class "clazz" to "os".
+ *
+ * Each class currently as one private constructor:
+ *
+ * 1) Constructor from a plain isl_* C pointer
+ *
+ * Example:
+ *
+ * Set(__isl_take isl_set *Ptr);
+ *
+ * The raw pointer constructor is kept private. Object construction
+ * is only possible through isl::manage().
+ */
+ void print_private_constructors(ostream &os, const isl_class &clazz);
+
+ /* Print declarations of copy assignment operator for class "clazz"
+ * to "os".
+ *
+ * Each class has one assignment operator.
+ *
+ * Set& Set::operator=(Set Obj)
+ *
+ */
+ void print_copy_assignment(ostream &os, const isl_class &clazz);
+
+ /* Print declarations of public constructors for class "clazz" to "os".
+ *
+ * Each class currently as two public constructors:
+ *
+ * 1) A default constructor
+ * 2) A copy constructor
+ *
+ * Example:
+ *
+ * Set();
+ * Set(const Set &set);
+ */
+ void print_public_constructors(ostream &os, const isl_class &clazz);
+
+ /* Print declaration of destructor for class "clazz" to "os".
+ */
+ void print_destructor(ostream &os, const isl_class &clazz);
+
+ /* Print declaration of pointer functions for class "clazz" to "os".
+ *
+ * To obtain a raw pointer three functions are provided:
+ *
+ * 1) __isl_give isl_set *copy()
+ *
+ * Returns a pointer to a _copy_ of the internal object
+ *
+ * 2) isl_set *get()
+ *
+ * Returns a pointer to the internal object
+ *
+ * 3) __isl_give isl_set *release()
+ *
+ * Returns a pointer to the internal object and resets the
+ * internal pointer to nullptr.
+ *
+ * The functions get() and release() model std::unique ptr. The copy()
+ * function is an extension to allow the user to explicitly copy
+ * the underlying object.
+ *
+ * Also generate a declaration to delete copy() for r-values. For
+ * r-values release() should be used to avoid unnecessary copies.
+ */
+ void print_ptr(ostream &os, const isl_class &clazz);
+
+ /* Print all implementations to "os".
+ */
+ void print_implementations(ostream &os);
+
+ /* Print implementations for class "clazz" to "os".
+ */
+ void print_class_impl(ostream &os, const isl_class &clazz);
+
+ /* Print implementation of global constructor method to "os".
+ */
+ void print_class_global_constructor_impl(ostream &os,
+ const isl_class &clazz);
+
+ /* Print implementations of private constructors for class "clazz" to
+ * "os".
+ */
+ void print_private_constructors_impl(ostream &os,
+ const isl_class &clazz);
+
+ /* Print implementations of public constructors for class "clazz" to
+ * "os".
+ */
+ void print_public_constructors_impl(ostream &os,
+ const isl_class &clazz);
+
+ /* Print implementation of copy assignment operator for class "clazz"
+ * to "os".
+ */
+ void print_copy_assignment_impl(ostream &os, const isl_class &clazz);
+
+ /* Print implementation of destructor for class "clazz" to "os".
+ */
+ void print_destructor_impl(ostream &os, const isl_class &clazz);
+
+ /* Print implementation of ptr() function for class "clazz" to "os".
+ */
+ void print_ptr_impl(ostream &os, const isl_class &clazz);
+
+ /* Translate the isl type name into its corresponding C++ type.
+ *
+ * To obtain the C++ type name, the 'isl_' prefix is removed and the
+ * remainder * is translated to CamelCase.
+ *
+ * isl_basic_set -> BasicSet
+ *
+ */
+ string type2cpp(string name);
+
+ /* Convert a string "input" to CamelCase.
+ */
+ string to_camel_case(const string &input);
+
+ /* Check if a given class "clazz" is supported by these bindings.
+ *
+ * Only classes that have been tested and reviewed are supported.
+ */
+ bool is_supported_class(string name);
+};
diff --git a/interface/extract_interface.cc b/interface/extract_interface.cc
index 1a05520..562a7e1 100644
--- a/interface/extract_interface.cc
+++ b/interface/extract_interface.cc
@@ -76,6 +76,7 @@
#include "extract_interface.h"
#include "generator.h"
#include "python.h"
+#include "cpp.h"

using namespace std;
using namespace clang;
@@ -432,6 +433,9 @@ int main(int argc, char *argv[])
if (Language.compare("python") == 0)
gen = new python_generator(consumer.exported_types,
consumer.exported_functions, consumer.functions);
+ else if (Language.compare("cpp") == 0)
+ gen = new cpp_generator(consumer.exported_types,
+ consumer.exported_functions, consumer.functions);
else
cerr << "Language '" << Language << "' not recognized." << endl
<< "Not generating bindings." << endl;
diff --git a/interface/isl.h.top b/interface/isl.h.top
new file mode 100644
index 0000000..bc76c99
--- /dev/null
+++ b/interface/isl.h.top
@@ -0,0 +1,4 @@
+#include <isl/set.h>
+#include <isl/map.h>
+#include <assert.h>
+#include <utility>
--
2.7.4

Tobias Grosser

unread,
Oct 19, 2016, 3:34:58 PM10/19/16
to Tobias Grosser, isl-dev...@googlegroups.com, Armin Groesslinger, Andreas Simbuerger, Sven Verdoolaege, Michael Kruse, Jakob Leben, Alexandre Isoard
Hi all,

to give some context. These patches implement the isl C++ smart pointer
interface that Jakob, Michael, Alexandre and me developed throughout the
last weeks, using the accumulated experience from earlier existing isl
bindings generators available.

The first three patches are just a resubmission of Armin's original
patches which I rebased to isl trunk. The last patch implements a
minimal C++ generator. The interface it implements is sufficient enough
to be useful for Polly and we are planning to grow a more complete C++
interface around it. At the same time, it is still minimalistic enough
to allow for a detailed review without getting distracted by a lot of
side-discussions.

For an _extensive_ list of choices that we evaluated including their
individual benefits and drawbacks see the following
https://docs.google.com/document/d/1h89T_wKRFSimY8bn9estBI4p0pJZkj7Zu5ptSE7nybI/edit
list. A more brief explanation of why certain choices have been taken is
in the commit message of patch 4.

I attached the generated C++ bindings (isl.h) as well as an example file
that illustrates their use (test_isl_smartptr.cpp)

I am looking forward to receive your comments on these patches.

Best,
Tobias
test_isl_smartptr.cpp
isl.h

Jakob Leben

unread,
Oct 19, 2016, 4:24:37 PM10/19/16
to Tobias Grosser, isl Development, Armin Groesslinger, Andreas Simbuerger, Sven Verdoolaege, Michael Kruse, Alexandre Isoard
Hi,

I noticed the patches do not include generation of isValid (or isNull, or operator bool). Since the classes can be default constructed with a null pointer, I think these methods are crucial.

Sven Verdoolaege

unread,
Oct 20, 2016, 10:00:56 AM10/20/16
to Tobias Grosser, isl-dev...@googlegroups.com, Armin Groesslinger, Andreas Simbuerger
On Wed, Oct 19, 2016 at 08:38:59PM +0200, Tobias Grosser wrote:
> From: Armin Groesslinger <armin.gro...@uni-passau.de>
>
> To support bindings for other languages than Python in the
> future, move the functions for querying information about
> isl functions and types (e.g., takes(), is_subclass()) and
> the struct isl_class to generator.h and generator.cc. The
> generator routines for Python remain in python.h and python.cc.
>
> This change does not change current behavior. The generated isl.py files
> are identical.
>
> Signed-off-by: Armin Groesslinger <armin.gro...@uni-passau.de>
> Signed-off-by: Andreas Simbuerger <simb...@fim.uni-passau.de>
> Signed-off-by: Sven Verdoolaege <sk...@kotnet.org>
> Signed-off-by: Tobias Grosser <tob...@grosser.es>

Did you modify this patch after my sign-off was added?

skimo

Tobias Grosser

unread,
Oct 20, 2016, 10:16:18 AM10/20/16
to sven.ver...@gmail.com, isl-dev...@googlegroups.com, Armin Groesslinger, Andreas Simbuerger
As written in the other email, I took this patch from Andreas' git
branch and rebased it onto isl trunk. As part of this rebase I resolved
merge conflicts and moved newly added functions that belong to
generator.cc into generator.cc.

I attached you the new patch again as well as the patch from Andreas git
branch in case you want to compare the difference.

Best,
Tobias
old.patch
new.patch

Sven Verdoolaege

unread,
Oct 20, 2016, 10:40:57 AM10/20/16
to Tobias Grosser, isl-dev...@googlegroups.com, Armin Groesslinger, Andreas Simbuerger
On Thu, Oct 20, 2016 at 04:16:17PM +0200, Tobias Grosser wrote:
> On Thu, Oct 20, 2016, at 04:00 PM, Sven Verdoolaege wrote:
> > On Wed, Oct 19, 2016 at 08:38:59PM +0200, Tobias Grosser wrote:
> > > From: Armin Groesslinger <armin.gro...@uni-passau.de>
> > >
> > > To support bindings for other languages than Python in the
> > > future, move the functions for querying information about
> > > isl functions and types (e.g., takes(), is_subclass()) and
> > > the struct isl_class to generator.h and generator.cc. The
> > > generator routines for Python remain in python.h and python.cc.
> > >
> > > This change does not change current behavior. The generated isl.py files
> > > are identical.
> > >
> > > Signed-off-by: Armin Groesslinger <armin.gro...@uni-passau.de>
> > > Signed-off-by: Andreas Simbuerger <simb...@fim.uni-passau.de>
> > > Signed-off-by: Sven Verdoolaege <sk...@kotnet.org>
> > > Signed-off-by: Tobias Grosser <tob...@grosser.es>
> >
> > Did you modify this patch after my sign-off was added?
>
> As written in the other email, I took this patch from Andreas' git
> branch and rebased it onto isl trunk. As part of this rebase I resolved
> merge conflicts and moved newly added functions that belong to
> generator.cc into generator.cc.

I'll take that as a "yes". Then please remove my sign-off
so that I know I have to look at it again.

skimo

Tobias Grosser

unread,
Oct 20, 2016, 10:53:59 AM10/20/16
to isl-dev...@googlegroups.com, Armin Größlinger, Andreas Simbuerger, Tobias Grosser
From: Armin Größlinger <armin.gro...@uni-passau.de>

To support bindings for other languages than Python in the
future, move the functions for querying information about
isl functions and types (e.g., takes(), is_subclass()) and
the struct isl_class to generator.h and generator.cc. The
generator routines for Python remain in python.h and python.cc.

This change does not change current behavior. The generated isl.py files
are identical.

Signed-off-by: Armin Größlinger <armin.gro...@uni-passau.de>
Signed-off-by: Andreas Simbuerger <simb...@fim.uni-passau.de>
Signed-off-by: Tobias Grosser <tob...@grosser.es>
---

Changes to v1:

- Drop Sven's Signed-off-by line.

interface/Makefile.am | 2 +
interface/extract_interface.cc | 1 +
interface/generator.cc | 260 +++++++++++++++++++++++++++++++++++++++++
interface/generator.h | 58 +++++++++
interface/python.cc | 251 ---------------------------------------
interface/python.h | 3 +-
6 files changed, 322 insertions(+), 253 deletions(-)
create mode 100644 interface/generator.cc
create mode 100644 interface/generator.h

diff --git a/interface/Makefile.am b/interface/Makefile.am
index 242294e..a37cb18 100644
--- a/interface/Makefile.am
+++ b/interface/Makefile.am
@@ -10,6 +10,8 @@ includes = -I$(top_builddir) -I$(top_srcdir) \

extract_interface_CPPFLAGS = $(includes)
extract_interface_SOURCES = \
+ generator.h \
+ generator.cc \
python.h \
python.cc \
extract_interface.h \
diff --git a/interface/extract_interface.cc b/interface/extract_interface.cc
index 331ec44..9013f66 100644
--- a/interface/extract_interface.cc
+++ b/interface/extract_interface.cc
@@ -74,6 +74,7 @@
#include <clang/Sema/Sema.h>

#include "extract_interface.h"
+#include "generator.h"
#include "python.h"

using namespace std;
diff --git a/interface/generator.cc b/interface/generator.cc
new file mode 100644
index 0000000..63b21f9
--- /dev/null
+++ b/interface/generator.cc
@@ -0,0 +1,260 @@
+/*
+ * Copyright 2011,2015 Sven Verdoolaege. All rights reserved.
+ }
+ }
+
+/* Return the class that has a name that matches the initial part
+ * of the name of function "fd" or NULL if no such class could be found.
+ */
+isl_class *method2class(map<string, isl_class> &classes,
+ FunctionDecl *fd)
+{
+ string best;
+ map<string, isl_class>::iterator ci;
+ string name = fd->getNameAsString();
+
+ for (ci = classes.begin(); ci != classes.end(); ++ci) {
+ if (name.substr(0, ci->first.length()) == ci->first)
+ best = ci->first;
+ }
+
+ if (classes.find(best) == classes.end()) {
+ cerr << "Unable to find class of " << name << endl;
+ return NULL;
+ }
+
+ return &classes[best];
+}
+
+/* Is "type" the type "isl_ctx *"?
+ */
+bool is_isl_ctx(QualType type)
+{
+ if (!type->isPointerType())
+ return 0;
+ type = type->getPointeeType();
+ if (type.getAsString() != "isl_ctx")
+ return false;
+
+ return true;
+}
+
+ }
+
+ return false;
+}
+ }
+
+ return false;
+}
+
+/* Is "type" that of a pointer to a function?
+ */
+bool is_callback(QualType type)
+{
+ if (!type->isPointerType())
+ return false;
+ type = type->getPointeeType();
+ return type->isFunctionType();
+}
+
+/* Is "type" that of "char *" of "const char *"?
+ */
+bool is_string(QualType type)
+{
+ if (type->isPointerType()) {
+ string s = type->getPointeeType().getAsString();
+ return s == "const char" || s == "char";
+ }
+
+ return false;
+}
+
+/* Return the name of the type that "type" points to.
+ * The input "type" is assumed to be a pointer type.
+ */
+string extract_type(QualType type)
+{
+ if (type->isPointerType())
+ return type->getPointeeType().getAsString();
+ die("Cannot extract type from non-pointer type");
+}
diff --git a/interface/generator.h b/interface/generator.h
new file mode 100644
index 0000000..d577b4d
--- /dev/null
+++ b/interface/generator.h
@@ -0,0 +1,58 @@
+#ifndef ISL_INTERFACE_GENERATOR_H
+#define ISL_INTERFACE_GENERATOR_H
+
+#include <clang/AST/Decl.h>
+#include <map>
+#include <set>
+#include <string>
+
+
+using namespace std;
+using namespace clang;
+
+/* isl_class collects all constructors and methods for an isl "class".
+ * "name" is the name of the class.
+ * "type" is the declaration that introduces the type.
+ * "methods" contains the set of methods, grouped by method name.
+ * "fn_to_str" is a reference to the *_to_str method of this class, if any.
+ * "fn_free" is a reference to the *_free method of this class, if any.
+ */
+struct isl_class {
+ string name;
+ RecordDecl *type;
+ set<FunctionDecl *> constructors;
+ map<string, set<FunctionDecl *> > methods;
+ FunctionDecl *fn_to_str;
+ FunctionDecl *fn_free;
+
+ bool is_static(FunctionDecl *method);
+
+ void print(map<string, isl_class> &classes, set<string> &done);
+ void print_constructor(FunctionDecl *method);
+ void print_representation(const string &python_name);
+ void print_method_type(FunctionDecl *fd);
+ void print_method_types();
+ void print_method(FunctionDecl *method, vector<string> super);
+ void print_method_overload(FunctionDecl *method, vector<string> super);
+ void print_method(const string &fullname,
+ const set<FunctionDecl *> &methods, vector<string> super);
+};
+
+void die(const char *msg) __attribute__((noreturn));
+string drop_type_suffix(string name, FunctionDecl *method);
+vector<string> find_superclasses(RecordDecl *decl);
+bool is_overload(Decl *decl);
+bool is_constructor(Decl *decl);
+bool takes(Decl *decl);
+bool gives(Decl *decl);
+isl_class *method2class(map<string, isl_class> &classes, FunctionDecl *fd);
+bool is_isl_ctx(QualType type);
+bool first_arg_is_isl_ctx(FunctionDecl *fd);
+bool is_isl_type(QualType type);
+bool is_isl_bool(QualType type);
+bool is_string_type(QualType type);
+bool is_callback(QualType type);
+bool is_string(QualType type);
+string extract_type(QualType type);
+
+#endif /* ISL_INTERFACE_GENERATOR_H */
diff --git a/interface/python.cc b/interface/python.cc
index 80977e7..4ec166f 100644
--- a/interface/python.cc
+++ b/interface/python.cc
- FunctionDecl *fn_free;
-
- bool is_static(FunctionDecl *method);
-
- void print(map<string, isl_class> &classes, set<string> &done);
- void print_constructor(FunctionDecl *method);
- void print_representation(const string &python_name);
- void print_method_type(FunctionDecl *fd);
- void print_method_types();
- void print_method(FunctionDecl *method, vector<string> super);
- void print_method_overload(FunctionDecl *method, vector<string> super);
- void print_method(const string &fullname,
- const set<FunctionDecl *> &methods, vector<string> super);
/* Drop the "isl_" initial part of the type name "name".
*/
/* Should "method" be considered to be a static method?
* That is, is the first argument something other than
* an instance of the class?
diff --git a/interface/python.h b/interface/python.h
index 5dff126..1706dd7 100644
--- a/interface/python.h
+++ b/interface/python.h
@@ -1,5 +1,4 @@
-#include <set>
-#include <clang/AST/Decl.h>
+#include "generator.h"

using namespace std;
using namespace clang;
--
2.9.3

Tobias Grosser

unread,
Oct 20, 2016, 10:54:00 AM10/20/16
to isl-dev...@googlegroups.com, Armin Größlinger, Andreas Simbuerger, Tobias Grosser
From: Armin Größlinger <armin.gro...@uni-passau.de>

Add option "--language" to specify for which language (currently only
"python") to generate bindings.

Signed-off-by: Armin Größlinger <armin.gro...@uni-passau.de>
Signed-off-by: Andreas Simbuerger <simb...@fim.uni-passau.de>
Signed-off-by: Tobias Grosser <tob...@grosser.es>
---
interface/Makefile.am | 3 ++-
interface/extract_interface.cc | 18 +++++++++++++++---
2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/interface/Makefile.am b/interface/Makefile.am
index a37cb18..4d2b733 100644
--- a/interface/Makefile.am
+++ b/interface/Makefile.am
@@ -29,7 +29,8 @@ test: extract_interface

isl.py: extract_interface isl.py.top
(cat $(srcdir)/isl.py.top; \
- ./extract_interface$(EXEEXT) $(includes) $(srcdir)/all.h) \
+ ./extract_interface$(EXEEXT) --language=python $(includes) \
+ $(srcdir)/all.h) \
> isl.py

dist-hook: isl.py
diff --git a/interface/extract_interface.cc b/interface/extract_interface.cc
index ddca368..1a05520 100644
--- a/interface/extract_interface.cc
+++ b/interface/extract_interface.cc
2.9.3

Tobias Grosser

unread,
Oct 20, 2016, 10:54:00 AM10/20/16
to isl-dev...@googlegroups.com, Armin Größlinger, Andreas Simbuerger, Tobias Grosser
From: Armin Größlinger <armin.gro...@uni-passau.de>

This change is large, but mostly mechanical. In generator.cc the prefix
'generator::' is added to turn static functions into class methods. In
python.cc functions are moved into the newly introduced
python_generator. This sometimes requires the introduction of a new
isl_class argument.

The only non-trivial change is the separation of generate_python into
generator::generator() and python_generator::generate(). The former now
constructs the set of isl_classses which we want to generate bindings
for, whereas the latter prints for each class the corresponding bindings.

This change does not change current behavior. The generated isl.py files
are identical.

Signed-off-by: Armin Größlinger <armin.gro...@uni-passau.de>
Signed-off-by: Andreas Simbuerger <simb...@fim.uni-passau.de>
Signed-off-by: Tobias Grosser <tob...@grosser.es>
---
interface/extract_interface.cc | 3 +-
interface/generator.cc | 100 +++++++++++++++++----
interface/generator.h | 65 ++++++++------
interface/python.cc | 191 +++++++++++++++--------------------------
interface/python.h | 39 ++++++++-
5 files changed, 231 insertions(+), 167 deletions(-)

diff --git a/interface/extract_interface.cc b/interface/extract_interface.cc
index 9013f66..ddca368 100644
--- a/interface/extract_interface.cc
+++ b/interface/extract_interface.cc
@@ -423,8 +423,9 @@ int main(int argc, char *argv[])
ParseAST(*sema);
Diags.getClient()->EndSourceFile();
+generator::generator(set<RecordDecl *> &exported_types,
+ set<FunctionDecl *> exported_functions, set<FunctionDecl *> functions)
+{
+ map<string, isl_class>::iterator ci;
+
/* Print error message "msg" and abort.
*/
-void die(const char *msg)
+void generator::die(const char *msg)
{
fprintf(stderr, "%s", msg);
abort();
@@ -53,7 +121,7 @@ void die(const char *msg)
* return the modified name (or the original name if
* no modifications were made).
*/
-string drop_type_suffix(string name, FunctionDecl *method)
+string generator::drop_type_suffix(string name, FunctionDecl *method)
{
int num_params;
ParmVarDecl *param;
@@ -79,7 +147,7 @@ string drop_type_suffix(string name, FunctionDecl *method)
/* Return a sequence of the types of which the given type declaration is
* marked as being a subtype.
*/
-std::vector<string> find_superclasses(RecordDecl *decl)
+std::vector<string> generator::find_superclasses(RecordDecl *decl)
{
vector<string> super;

@@ -105,28 +173,28 @@ std::vector<string> find_superclasses(RecordDecl *decl)

/* Is decl marked as being part of an overloaded method?
*/
-bool is_overload(Decl *decl)
+bool generator::is_overload(Decl *decl)
{
return has_annotation(decl, "isl_overload");
}

/* Is decl marked as a constructor?
*/
-bool is_constructor(Decl *decl)
+bool generator::is_constructor(Decl *decl)
{
return has_annotation(decl, "isl_constructor");
}

/* Is decl marked as consuming a reference?
*/
-bool takes(Decl *decl)
+bool generator::takes(Decl *decl)
{
return has_annotation(decl, "isl_take");
}

/* Is decl marked as returning a reference that is required to be freed.
*/
-bool gives(Decl *decl)
+bool generator::gives(Decl *decl)
{
return has_annotation(decl, "isl_give");
}
@@ -134,7 +202,7 @@ bool gives(Decl *decl)
/* Return the class that has a name that matches the initial part
* of the name of function "fd" or NULL if no such class could be found.
*/
-isl_class *method2class(map<string, isl_class> &classes,
+isl_class *generator::method2class(map<string, isl_class> &classes,
FunctionDecl *fd)
{
string best;
@@ -156,7 +224,7 @@ isl_class *method2class(map<string, isl_class> &classes,

/* Is "type" the type "isl_ctx *"?
*/
-bool is_isl_ctx(QualType type)
+bool generator::is_isl_ctx(QualType type)
{
if (!type->isPointerType())
return 0;
@@ -169,7 +237,7 @@ bool is_isl_ctx(QualType type)

/* Is the first argument of "fd" of type "isl_ctx *"?
*/
-bool first_arg_is_isl_ctx(FunctionDecl *fd)
+bool generator::first_arg_is_isl_ctx(FunctionDecl *fd)
{
ParmVarDecl *param;

@@ -182,7 +250,7 @@ bool first_arg_is_isl_ctx(FunctionDecl *fd)

/* Is "type" that of a pointer to an isl_* structure?
*/
-bool is_isl_type(QualType type)
+bool generator::is_isl_type(QualType type)
{
if (type->isPointerType()) {
string s;
@@ -199,7 +267,7 @@ bool is_isl_type(QualType type)

/* Is "type" the type isl_bool?
*/
-bool is_isl_bool(QualType type)
+bool generator::is_isl_bool(QualType type)
{
string s;

@@ -212,7 +280,7 @@ bool is_isl_bool(QualType type)

/* Is "type" that of a pointer to char.
*/
-bool is_string_type(QualType type)
+bool generator::is_string_type(QualType type)
{
if (type->isPointerType()) {
string s;
@@ -229,7 +297,7 @@ bool is_string_type(QualType type)

/* Is "type" that of a pointer to a function?
*/
-bool is_callback(QualType type)
+bool generator::is_callback(QualType type)
{
if (!type->isPointerType())
return false;
@@ -239,7 +307,7 @@ bool is_callback(QualType type)

/* Is "type" that of "char *" of "const char *"?
*/
-bool is_string(QualType type)
+bool generator::is_string(QualType type)
{
if (type->isPointerType()) {
string s = type->getPointeeType().getAsString();
@@ -252,7 +320,7 @@ bool is_string(QualType type)
/* Return the name of the type that "type" points to.
* The input "type" is assumed to be a pointer type.
*/
-string extract_type(QualType type)
+string generator::extract_type(QualType type)
{
if (type->isPointerType())
return type->getPointeeType().getAsString();
diff --git a/interface/generator.h b/interface/generator.h
index d577b4d..b55de99 100644
--- a/interface/generator.h
+++ b/interface/generator.h
@@ -24,35 +24,44 @@ struct isl_class {
map<string, set<FunctionDecl *> > methods;
FunctionDecl *fn_to_str;
FunctionDecl *fn_free;
-
- bool is_static(FunctionDecl *method);
-
- void print(map<string, isl_class> &classes, set<string> &done);
- void print_constructor(FunctionDecl *method);
- void print_representation(const string &python_name);
- void print_method_type(FunctionDecl *fd);
- void print_method_types();
- void print_method(FunctionDecl *method, vector<string> super);
- void print_method_overload(FunctionDecl *method, vector<string> super);
- void print_method(const string &fullname,
- const set<FunctionDecl *> &methods, vector<string> super);
};

-void die(const char *msg) __attribute__((noreturn));
-string drop_type_suffix(string name, FunctionDecl *method);
-vector<string> find_superclasses(RecordDecl *decl);
-bool is_overload(Decl *decl);
-bool is_constructor(Decl *decl);
-bool takes(Decl *decl);
-bool gives(Decl *decl);
-isl_class *method2class(map<string, isl_class> &classes, FunctionDecl *fd);
-bool is_isl_ctx(QualType type);
-bool first_arg_is_isl_ctx(FunctionDecl *fd);
-bool is_isl_type(QualType type);
-bool is_isl_bool(QualType type);
-bool is_string_type(QualType type);
-bool is_callback(QualType type);
-bool is_string(QualType type);
-string extract_type(QualType type);
+/* Base class for interface generators.
+ */
+class generator {
+protected:
+ map<string,isl_class> classes;
+ map<string, FunctionDecl *> functions_by_name;
+
+public:
+ generator(set<RecordDecl *> &exported_types,
+ set<FunctionDecl *> exported_functions,
+ set<FunctionDecl *> functions);
+
#endif /* ISL_INTERFACE_GENERATOR_H */
diff --git a/interface/python.cc b/interface/python.cc
index 4ec166f..3b23c79 100644
--- a/interface/python.cc
+++ b/interface/python.cc
@@ -39,25 +39,17 @@
#include <vector>
#include "python.h"

-/* Drop the "isl_" initial part of the type name "name".
- */
-static string type2python(string name)
+python_generator::python_generator( set<RecordDecl *> &exported_types,
+ set<FunctionDecl *> exported_functions, set<FunctionDecl *> functions)
+ : generator(exported_types, exported_functions, functions)
{
- return name.substr(4);
}

-/* Should "method" be considered to be a static method?
- * That is, is the first argument something other than
- * an instance of the class?
+/* Drop the "isl_" initial part of the type name "name".
*/
-void isl_class::print_method(const string &fullname,
- const set<FunctionDecl *> &methods, vector<string> super)
map<string, isl_class>::iterator ci;
- }
- }

for (ci = classes.begin(); ci != classes.end(); ++ci) {
if (done.find(ci->first) == done.end())
- ci->second.print(classes, done);
+ print(ci->second);
}
}
diff --git a/interface/python.h b/interface/python.h
index 1706dd7..ea361b0 100644
--- a/interface/python.h
+++ b/interface/python.h
@@ -3,5 +3,40 @@
using namespace std;
using namespace clang;

-void generate_python(set<RecordDecl *> &exported_types,
- set<FunctionDecl *> exported_functions, set<FunctionDecl *> functions);
+class python_generator : public generator {
+private:
+ set<string> done;
+
+public:
+ python_generator(set<RecordDecl *> &exported_types,
+ set<FunctionDecl *> exported_functions,
+ set<FunctionDecl *> functions);
+
+ virtual void generate() override;
+
+private:
+ void print(const isl_class &clazz);
+ void print_method_header(bool is_static, const string &name, int n_arg);
+ void print_class_header(const isl_class &clazz, const string &name,
+ const vector<string> &super);
+ void print_type_check(const string &type, int pos, bool upcast,
+ const string &super, const string &name, int n);
+ void print_callback(QualType type, int arg);
+ void print_arg_in_call(FunctionDecl *fd, int arg, int skip);
+ void print_argtypes(FunctionDecl *fd);
+ void print_method_return(FunctionDecl *method);
+ void print_restype(FunctionDecl *fd);
+ void print(map<string, isl_class> &classes, set<string> &done);
+ void print_constructor(const isl_class &clazz, FunctionDecl *method);
+ void print_representation(const isl_class &clazz,
+ const string &python_name);
+ void print_method_type(FunctionDecl *fd);
+ void print_method_types(const isl_class &clazz);
+ void print_method(const isl_class &clazz, FunctionDecl *method,
+ vector<string> super);
+ void print_method_overload(const isl_class &clazz,
+ FunctionDecl *method, vector<string> super);
+ void print_method(const isl_class &clazz, const string &fullname,
+ const set<FunctionDecl *> &methods, vector<string> super);
+
+};
--
2.9.3

Tobias Grosser

unread,
Oct 20, 2016, 10:54:01 AM10/20/16
to isl-dev...@googlegroups.com, Tobias Grosser
.. which generates for each exported isl object a smart pointer class of
the following form:

class Set;
inline Set manage(__isl_take isl_set *Set);

class Set {
friend inline Set manage(__isl_take isl_set *Set);

isl_set *Ptr = nullptr;

inline explicit Set(__isl_take isl_set *Ptr);

public:
inline Set();
inline Set(const Set &Obj);
inline Set& operator=(Set Obj);
inline ~Set();
inline __isl_give isl_set *copy() const &;
inline __isl_give isl_set *copy() && = delete;
inline isl_set *get() const;
inline __isl_give isl_set *release();
inline bool isNull() const;
};

The interface and functionality resembles C++11 std::unique_ptr. The
isl smart pointer keeps unique ownership of an object. The currently
held pointer can be observed by calling get(). When calling release(),
both the current pointer is returned and ownership is released. Names
and semantics of get() and release() match the corresponding methods in
std::unique_ptr. In addition, a copy() method is provided, which returns
a pointer to a copy of the managed object. These three methods allow the
isl C++ objects to be used together with the isl C functions. On top of
this an isNull method is provided to check if the object is currently
NULL.
Signed-off-by: Tobias Grosser <tob...@grosser.es>
---

Changes to v1:

- added isNull method

interface/Makefile.am | 12 +-
interface/cpp.cc | 353 +++++++++++++++++++++++++++++++++++++++++
interface/cpp.h | 179 +++++++++++++++++++++
interface/extract_interface.cc | 4 +
interface/isl.h.top | 4 +
5 files changed, 550 insertions(+), 2 deletions(-)
create mode 100644 interface/cpp.cc
create mode 100644 interface/cpp.h
create mode 100644 interface/isl.h.top

diff --git a/interface/Makefile.am b/interface/Makefile.am
index 4d2b733..3d7642b 100644
--- a/interface/Makefile.am
+++ b/interface/Makefile.am
@@ -14,6 +14,8 @@ extract_interface_SOURCES = \
generator.cc \
python.h \
python.cc \
+ cpp.h \
+ cpp.cc \
extract_interface.h \
extract_interface.cc
extract_interface_LDADD = \
@@ -33,5 +35,11 @@ isl.py: extract_interface isl.py.top
$(srcdir)/all.h) \
> isl.py

-dist-hook: isl.py
- cp isl.py $(distdir)/
+isl.h: extract_interface all.h isl.h.top
+ (cat $(srcdir)/isl.h.top; \
+ ./extract_interface$(EXEEXT) --language=cpp $(includes) \
+ $(srcdir)/all.h) \
+ > isl.h
+
+dist-hook: isl.py isl.hh
+ cp isl.py $(distdir)/ ; cp isl.hh $(distdir)
diff --git a/interface/cpp.cc b/interface/cpp.cc
new file mode 100644
index 0000000..9173581
--- /dev/null
+++ b/interface/cpp.cc
@@ -0,0 +1,353 @@
+/*
+ * Copyright 2016 Tobias Grosser. All rights reserved.
+cpp_generator::cpp_generator( set<RecordDecl *> &exported_types,
+ set<FunctionDecl *> exported_functions, set<FunctionDecl *> functions)
+ : generator(exported_types, exported_functions, functions)
+{
+}
+
+void cpp_generator::generate()
+{
+ ostream &os = cout;
+
+ fprintf(os, "\n");
+ fprintf(os, "#ifndef ISL_CPP_ALL\n");
+ fprintf(os, "#define ISL_CPP_ALL\n\n");
+ fprintf(os, "namespace isl {\n\n");
+
+ print_forward_declarations(os);
+ print_declarations(os);
+ print_implementations(os);
+
+ fprintf(os, "};\n\n");
+ fprintf(os, "#endif /* ISL_CPP_ALL */\n");
+}
+
+void cpp_generator::print_forward_declarations(ostream &os)
+{
+ map<string, isl_class>::iterator ci;
+
+ fprintf(os, "// forward declarations\n");
+
+ for (ci = classes.begin(); ci != classes.end(); ++ci)
+ if (is_supported_class(ci->second.name))
+ print_class_forward_decl(os, ci->second);
+ fprintf(os, "\n");
+}
+
+void cpp_generator::print_declarations(ostream &os)
+{
+ map<string, isl_class>::iterator ci;
+
+ for (ci = classes.begin(); ci != classes.end(); ++ci)
+ print_class(os, ci->second);
+}
+
+void cpp_generator::print_implementations(ostream &os)
+{
+ map<string, isl_class>::iterator ci;
+
+ for (ci = classes.begin(); ci != classes.end(); ++ci)
+ fprintf(os, " inline bool isNull();\n");
+}
+
+ fprintf(os, "bool %s::isNull() {\n", cppname);
+ fprintf(os, " return Ptr == nullptr;\n");
+ fprintf(os, "}\n\n");
+}
+
+string cpp_generator::to_camel_case(const string &input)
+{
+ string output;
+
+ bool uppercase = true;
+
+ for (const char &character : input) {
+
+ if (character == '_') {
+ uppercase = true;
+ continue;
+ }
+ if (uppercase) {
+ output.append(1, toupper(character));
+ uppercase = false;
+ } else {
+ output.append(1, character);
+ }
+ }
+
+ return output;
+}
+
+string cpp_generator::type2cpp(string name)
+{
+ return to_camel_case(name.substr(4));
+}
+
+std::vector<const char*> supported_classes = {
+ "isl_set",
+ "isl_map",
+};
+
+bool cpp_generator::is_supported_class(string name)
+{
+ for (size_t i = 0; i < supported_classes.size(); i++) {
+ if (name == supported_classes[i])
+ return true;
+ }
+
+ return false;
+}
diff --git a/interface/cpp.h b/interface/cpp.h
new file mode 100644
index 0000000..476086a
--- /dev/null
+++ b/interface/cpp.h
@@ -0,0 +1,179 @@
+#include "generator.h"
+
+using namespace std;
+using namespace clang;
+
+class cpp_generator : public generator {
+public:
+ cpp_generator(set<RecordDecl *> &exported_types,
+ set<FunctionDecl *> exported_functions,
+ set<FunctionDecl *> functions);
+
+ virtual void generate() override;
+private:
+
+ /* Print forward declarations for all classes to "os".
+ */
+ void print_forward_declarations(ostream &os);
+
+ /* Print all declarations to "os".
+ */
+ void print_declarations(ostream &os);
+
+ /* Print declarations for class "clazz" to "os".
+ */
+ void print_class(ostream &os, const isl_class &clazz);
+
+ /* Print forward declaration of class "clazz" to "os".
+ */
+ void print_class_forward_decl(ostream &os, const isl_class &clazz);
+
+ /* Print global constructor method to "os".
+ *
+ * Each class has one global constructor:
+ *
+ * Set manage(__isl_take isl_set *Ptr);
+ *
+ * The only public way to construct isl C++ objects from a raw pointer
+ * is through this global constructor method. This ensures isl object
+ * constructions is very explicit and pointers do not converted by
+ * accident. Due to overloading manage() can be called on any isl
+ * raw pointer and the corresponding object is automatically
+ * constructure, without the user having to choose the right isl object
+ * type.
+ *
+ */
+ void print_class_global_constructor(ostream &os,
+ const isl_class &clazz);
+
+ /* Print declarations of private constructors for class "clazz" to "os".
+ *
+ * Each class currently as one private constructor:
+ *
+ * 1) Constructor from a plain isl_* C pointer
+ *
+ * Example:
+ *
+ * Set(__isl_take isl_set *Ptr);
+ *
+ * 4) bool isNull()
+ *
+ * Check if the current object is a null pointer.
+ *
diff --git a/interface/extract_interface.cc b/interface/extract_interface.cc
index 1a05520..562a7e1 100644
--- a/interface/extract_interface.cc
+++ b/interface/extract_interface.cc
@@ -76,6 +76,7 @@
#include "extract_interface.h"
#include "generator.h"
#include "python.h"
+#include "cpp.h"

using namespace std;
using namespace clang;
@@ -432,6 +433,9 @@ int main(int argc, char *argv[])
if (Language.compare("python") == 0)
gen = new python_generator(consumer.exported_types,
consumer.exported_functions, consumer.functions);
+ else if (Language.compare("cpp") == 0)
+ gen = new cpp_generator(consumer.exported_types,
+ consumer.exported_functions, consumer.functions);
else
cerr << "Language '" << Language << "' not recognized." << endl
<< "Not generating bindings." << endl;
diff --git a/interface/isl.h.top b/interface/isl.h.top
new file mode 100644
index 0000000..bc76c99
--- /dev/null
+++ b/interface/isl.h.top
@@ -0,0 +1,4 @@
+#include <isl/set.h>
+#include <isl/map.h>
+#include <assert.h>
+#include <utility>
--
2.9.3

Tobias Grosser

unread,
Oct 20, 2016, 10:54:28 AM10/20/16
to sven.ver...@gmail.com, isl-dev...@googlegroups.com, Armin Groesslinger, Andreas Simbuerger
Sure. I sent out an updated patchset with your sign-off line removed.

Best,
Tobias

Sven Verdoolaege

unread,
Oct 20, 2016, 11:37:45 AM10/20/16
to Tobias Grosser, isl-dev...@googlegroups.com
Nice and extensive commit message.

I assume you are sending this to get early feedback.
Clearly it's way too early for them to get included in
the main repository. They would need to get extensively
tested first, preferably in more than one project.

On Wed, Oct 19, 2016 at 08:39:02PM +0200, Tobias Grosser wrote:
> .. which generates for each exported isl object a smart pointer class of
> the following form:

Please use complete sentences. (Not "..").

> class Set;
> inline Set manage(__isl_take isl_set *Set);
>
> class Set {
> friend inline Set manage(__isl_take isl_set *Set);
>
> isl_set *Ptr = nullptr;
>
> inline explicit Set(__isl_take isl_set *Ptr);
>
> public:
> inline Set();
> inline Set(const Set &Obj);
> inline Set& operator=(Set Obj);
> inline ~Set();
> inline __isl_give isl_set *copy() const &;
> inline __isl_give isl_set *copy() && = delete;

Why is this needed?

> inline isl_set *get() const;

Missing __isl_keep.

> inline __isl_give isl_set *release() const;
> };
>
> The interface and functionality resembles C++11 std::unique_ptr. The
> isl smart pointer keeps unique ownership of an object. The currently
> held pointer can be observed by calling get(). When calling release(),
> both the current pointer is returned and ownership is released. Names
> and semantics of get() and release() match the corresponding methods in
> std::unique_ptr.

Looking at the implementation of get(), isn't there a risk that
the isl object may get destroyed before you have finished using it
if you call get() on a temporary object?
Or does std::unique_ptr have the same issue such that users
should be used to having to think about that?

> In addition, a copy() method is provided, which returns
> a pointer to a copy of the managed object. These three methods allow the
> isl C++ objects to be used together with the isl C functions.
>
> Example:
>
> void foo(Set S) {
>
> isl_set_is_universe(S.get());
>
> isl_set_free(S.copy());
>
> isl_set_free(S.release());
> }
>
> An alternative choice to name get() and release() would have been keep()
> and take(). The name 'take' is for example used in llvm::OwningPtr. For
> consistency with std::unique_ptr, this alternative naming has not been
> choosen.

chosen

> isl C++ objects can be constructed either by copying existing isl
> objects of the same type or by passing an isl C pointer to a global
> constructor method isl::manage(isl_type *ptr). Constructing an
> isl C++ object from an isl C pointer using an explicit class constructor
> is not allowed. This interface has been choosen in correspondence with

chosen

> std::make_unique. Besides making object construction and especially
> pointer consumption explicit, global constructor methods also make
> specifying the needed object type unnecessary. We also allow default
> construction of empty isl objects, as default-constructabity is

constructability

> important when using isl objects in C++ containers and especially
> when exploiting C++ move semantics. To complete the "Big Three" we also
> define the copy assignment operator. The remaining two functions needed
> to complete the "Big Five", move constructor and move assignment, are not
> needed for an initial implementation and are left for a subsequent
> commit.
>
> Example:
>
> void foo (__isl_take *s1, __isl_take *s2) {
>
> isl::Set S1 = isl::manage(s1);
>
> auto S = isl::manage(s2);
>
> auto S2 = S1;
>
> isl::Set EmptySet;
> }
>
> The currently generated library is header-only, which resembles the
> already existing python bindings and is the best choice for a
> lightweight interface library. In the future, implementations for
> larger functions or debugging methods might require the addition
> of implementations as part of a compiled (not-header-only) library.
>
> The currently generated library is generated in a single unified header
> file, which matches the generated python bindings. In the future,
> this might evolve to a combination of per-class header files and a
> unifying header file, as there is interest in both. However, for
> an initial binding a unified header has been choosen.

chosen

>
> No class hierarchy is constructed, as there is currently no obvious
> benefit and a hierarchy can always be introduced later. However,
> most conversions are probably better implemented through implicit
> constructors from related isl objects. Such constructors are planned
> to be added in the future.
>
> All functions are declared 'inline' to not cause link errors when
> included in multiple translation units.
>
> The public interfaces have currently no defined behavior in case isl
> ptrs are NULL, but instead assert when used on a NULL pointer. We
> explicitly do not support isl's ability to handle NULL pointers
> gracefully to ensure no software relies on this feature, which could
> block the possible introduction of a more C++ style error handling
> approach later on.

What is this C++ style error handling that you may want
to introduce later?
Presumably you will want to extend the interface to also
include some methods. How will you handle isl functions
returning NULL?
In any case, don't use assert.

> This change currently only enables C++ interface generation for sets
> and maps, as only this part has been throughly tested. Other isl

Do you mean "truly" or "thoroughly" ?

> datatypes will be exposed in future commits.
>
> This change intentionally only introduces a minimalistic interface. The
> interface established with this change is already sufficient to manage
> isl pointers. Polly today uses an isl C++ interface that has about the
> same functionality as this change and already the simplified memory
> management has proven to facilitate software development a lot. The
> infrastructur established with this change will serve as basis for a
> future function call interface that makes isl C functions conveniently
> accessible through the C++ interface.

I would feel a lot more comfortable if the patch series would
include at least some function call interface.
Otherwise, it's difficult to tell if it will work out.

skimo

Michael Kruse

unread,
Oct 20, 2016, 12:03:21 PM10/20/16
to Sven Verdoolaege, Tobias Grosser, isl Development
2016-10-20 17:37 GMT+02:00 Sven Verdoolaege <skim...@kotnet.org>:
>> inline __isl_give isl_set *copy() const &;
>> inline __isl_give isl_set *copy() && = delete;
>
> Why is this needed?

It is to avoid copy() being called on r-values. The dtor of r-values
(isl_*_free) will be called soon(-ish) after the call returns. Copying
the object and free'ing the old one directly after is kind of useless.
One should use the release() function instead, which hands over the
ownership instead of copying it.

We had a discussion on the "isl C++ bindings -- Core design choices"
thread about it.


>> inline __isl_give isl_set *release() const;
>> };
>>
>> The interface and functionality resembles C++11 std::unique_ptr. The
>> isl smart pointer keeps unique ownership of an object. The currently
>> held pointer can be observed by calling get(). When calling release(),
>> both the current pointer is returned and ownership is released. Names
>> and semantics of get() and release() match the corresponding methods in
>> std::unique_ptr.
>
> Looking at the implementation of get(), isn't there a risk that
> the isl object may get destroyed before you have finished using it
> if you call get() on a temporary object?

The C++ standard guarantees the temporary objects (nameless variables)
are only destroyed at the end of the statement. Anything using it must
have finished by then (unless a copy is created/assigned to a named
variable)

The idea of get() is to be used by __isl_keep function parameters.


> Or does std::unique_ptr have the same issue such that users
> should be used to having to think about that?

As with unique_ptr, users should know whether they want to take the
ownership of the pointer or not.


>> The public interfaces have currently no defined behavior in case isl
>> ptrs are NULL, but instead assert when used on a NULL pointer. We
>> explicitly do not support isl's ability to handle NULL pointers
>> gracefully to ensure no software relies on this feature, which could
>> block the possible introduction of a more C++ style error handling
>> approach later on.
>
> What is this C++ style error handling that you may want
> to introduce later?
> Presumably you will want to extend the interface to also
> include some methods. How will you handle isl functions
> returning NULL?

The goal is, to notify the user early (at least in debug builds) when
an ISL function returns null, and passes it on without checking for
NULL. That is, the methods would check whether one of the arguments is
invalid/NULL.


> In any case, don't use assert.

What do you suggest instead of assert? And why?


Michael

--
Tardyzentrismus verboten!

Tobias Grosser

unread,
Oct 21, 2016, 3:30:30 AM10/21/16
to sven.ver...@gmail.com, isl-dev...@googlegroups.com
On Thu, Oct 20, 2016, at 05:37 PM, Sven Verdoolaege wrote:
> Nice and extensive commit message.

Thank you for the positive feedback. Really appreciated.

> I assume you are sending this to get early feedback.
> Clearly it's way too early for them to get included in
> the main repository. They would need to get extensively
> tested first, preferably in more than one project.

I would like to reach two things:

1) Reach agreement over patch 1 & 2 as well as how patch 4 connects
with generator.cc

Independently of what interface we generate, the way we use generator.cc
can remain the same and splitting the general bindings generator from
the python specific one seems beneficial just by itself. Hence, I would
like to get these changes fully reviewed (sign-off). If we all agree on
these changes, it would be great if patches 1 & 2 could already be
committed, as otherwise any changes to the python bindings necessarily
will require modifications to these patches and consequently a new
review. Patch 4 serves for this cause as an example that tests patch 1
and 2 and shows they are sufficient to enable the development of a
multi-language generator.

2) Get a general review of patch 4

Patch four is the result of three weeks of discussions of analyzing and
discussing all existing isl bindings and identifying a common base from
which we can grow a more complete set of bindings. Here it would be
great to get:

a) feedback on the smartptr interface we developed

We completed the design of the smartpointer interface, after
extensively evaluating a large range of choices
and making sure we believe this smartpointer interface can serve as a
base for all the enhancements we have
in mind. As this design will be the base for future discussions and
developments, it would be good to
get your opinion on the choices we have taken and to make sure our
future discussions start off from sth
we all agree with.

b) feedback on implementation of C++ generator

I am interested to get feedback on the way I implemented the C++
generator. I will develop future extensions
using the same style. By making sure the implementation choices I have
taken match your preferred style,
future patches hopefully can adopt this style and will require less
refactoring when pushed for review.

When to upstream patch 4 is certainly a separate discussion, that can be
held after 1) and 2).

I dropped a couple of questions Michael already answered nicely, but
otherwise gave inline answers:

> On Wed, Oct 19, 2016 at 08:39:02PM +0200, Tobias Grosser wrote:
> > .. which generates for each exported isl object a smart pointer class of
> > the following form:
>
> Please use complete sentences. (Not "..").

Fixed.

> > class Set;
> > inline Set manage(__isl_take isl_set *Set);
> >
> > class Set {
> > friend inline Set manage(__isl_take isl_set *Set);
> >
> > isl_set *Ptr = nullptr;
> >
> > inline explicit Set(__isl_take isl_set *Ptr);
> >
> > public:
> > inline Set();
> > inline Set(const Set &Obj);
> > inline Set& operator=(Set Obj);
> > inline ~Set();
> > inline __isl_give isl_set *copy() const &;
> > inline __isl_give isl_set *copy() && = delete;
>
> Why is this needed?

See comment in cpp.h:

* Also generate a declaration to delete copy() for r-values.
For
* r-values release() should be used to avoid unnecessary
copies.

I added this longer explanation to the commit message:

The r-value version of the copy() function is not needed as coping an
object that will be freed right after is inefficient. In such a
situation, the use of release() is more efficient as it avoids the
unnecessary object copy. By deleting the r-value version of copy we
ensure that the user does not accidentally use the copy() in situations
where it is preferable to call release().

> > inline isl_set *get() const;
>
> Missing __isl_keep.

Added.

> > In addition, a copy() method is provided, which returns
> > a pointer to a copy of the managed object. These three methods allow the
> > isl C++ objects to be used together with the isl C functions.
> >
> > Example:
> >
> > void foo(Set S) {
> >
> > isl_set_is_universe(S.get());
> >
> > isl_set_free(S.copy());
> >
> > isl_set_free(S.release());
> > }
> >
> > An alternative choice to name get() and release() would have been keep()
> > and take(). The name 'take' is for example used in llvm::OwningPtr. For
> > consistency with std::unique_ptr, this alternative naming has not been
> > choosen.
>
> chosen

Fixed.

> > isl C++ objects can be constructed either by copying existing isl
> > objects of the same type or by passing an isl C pointer to a global
> > constructor method isl::manage(isl_type *ptr). Constructing an
> > isl C++ object from an isl C pointer using an explicit class constructor
> > is not allowed. This interface has been choosen in correspondence with
>
> chosen

Fixed.

> > std::make_unique. Besides making object construction and especially
> > pointer consumption explicit, global constructor methods also make
> > specifying the needed object type unnecessary. We also allow default
> > construction of empty isl objects, as default-constructabity is
>
> constructability

Fixed.

> > important when using isl objects in C++ containers and especially
> > when exploiting C++ move semantics. To complete the "Big Three" we also
> > define the copy assignment operator. The remaining two functions needed
> > to complete the "Big Five", move constructor and move assignment, are not
> > needed for an initial implementation and are left for a subsequent
> > commit.
> >
> > Example:
> >
> > void foo (__isl_take *s1, __isl_take *s2) {
> >
> > isl::Set S1 = isl::manage(s1);
> >
> > auto S = isl::manage(s2);
> >
> > auto S2 = S1;
> >
> > isl::Set EmptySet;
> > }
> >
> > The currently generated library is header-only, which resembles the
> > already existing python bindings and is the best choice for a
> > lightweight interface library. In the future, implementations for
> > larger functions or debugging methods might require the addition
> > of implementations as part of a compiled (not-header-only) library.
> >
> > The currently generated library is generated in a single unified header
> > file, which matches the generated python bindings. In the future,
> > this might evolve to a combination of per-class header files and a
> > unifying header file, as there is interest in both. However, for
> > an initial binding a unified header has been choosen.
>
> chosen

Fixed.

> > This change currently only enables C++ interface generation for sets
> > and maps, as only this part has been throughly tested. Other isl
>
> Do you mean "truly" or "thoroughly" ?

thoroughly

> > datatypes will be exposed in future commits.
> >
> > This change intentionally only introduces a minimalistic interface. The
> > interface established with this change is already sufficient to manage
> > isl pointers. Polly today uses an isl C++ interface that has about the
> > same functionality as this change and already the simplified memory
> > management has proven to facilitate software development a lot. The
> > infrastructur established with this change will serve as basis for a
> > future function call interface that makes isl C functions conveniently
> > accessible through the C++ interface.
>
> I would feel a lot more comfortable if the patch series would
> include at least some function call interface.
> Otherwise, it's difficult to tell if it will work out.

The optimal function call interface is still something that we need to
work out . I already implemented some prototypes [1] (see attachments),
but to get full confidence we most likely will play with different
implementations to reach a conclusion. The different options we consider
are described in the google document we used to agree on this interface
[0]. Most of them have already been extensively evaluated in existing
bindings, such that only the question remains which function call
interface is the most intuitive / maintainable / efficient.

However, what all of these function calls interfaces have in common is a
smart pointer interface similar in nature to the one we propose here.
Hence, if we do not come up with something that nobody of us has been
thinking of before, I expect these patches to not be affected by the
function call interface choices we take.

In terms of how much this patch is tested. I already ported non-trivial
parts of Polly to use these bindings (plus some small extensions to
support to_str, operator bool(), and llvm's raw_ostream) and all Polly
tests pass [2,3]

Best,
Tobias

[0]
https://docs.google.com/document/d/1h89T_wKRFSimY8bn9estBI4p0pJZkj7Zu5ptSE7nybI/edit
[1] https://github.com/tobig/isl/tree/cpp-generator-with-functions
[2] https://github.com/tobig/isl/tree/cpp-generator-polly
[3]
https://github.com/tobig/polly/commit/e1580c2580eb05fe38962437483e7975026f3250
isl-with-functions.h
test_isl_smartptr_functions.cpp

Sven Verdoolaege

unread,
Oct 21, 2016, 10:03:30 AM10/21/16
to Tobias Grosser, isl-dev...@googlegroups.com
On Wed, Oct 19, 2016 at 08:39:02PM +0200, Tobias Grosser wrote:
> +string cpp_generator::to_camel_case(const string &input)
> +{
> + string output;
> +

drop empty line

> + bool uppercase = true;
> +
> + for (const char &character : input) {
> +
drop empty line

> + if (character == '_') {
> + uppercase = true;
> + continue;
> + }
> + if (uppercase) {
> + output.append(1, toupper(character));
> + uppercase = false;
> + } else {
> + output.append(1, character);
> + }
> + }
> +
> + return output;

Use tabs for indentation (entire function).

> +bool cpp_generator::is_supported_class(string name)
> +{
> + for (size_t i = 0; i < supported_classes.size(); i++) {
> + if (name == supported_classes[i])
> + return true;
> + }
> +
> + return false;

Use tabs for indentation (entire function).

skimo

Sven Verdoolaege

unread,
Oct 21, 2016, 10:09:26 AM10/21/16
to re...@meinersbur.de, Tobias Grosser, isl Development
Thanks for the explanations.
I don't understand. isl function are _designed_ to pass along NULL values.
(Maybe I failed to parse your sentence properly.)

> > In any case, don't use assert.
>
> What do you suggest instead of assert? And why?

What do you want assert to do?
Depending on the phase of the moon (or something equally obscure
such as a macro being defined), it either does nothing or
prints a cryptic error message.
Choose which of these two you want and if you go for the error
message, then print a proper one.

skimo

Sven Verdoolaege

unread,
Oct 21, 2016, 10:57:28 AM10/21/16
to Tobias Grosser, isl-dev...@googlegroups.com
On Fri, Oct 21, 2016 at 09:30:27AM +0200, Tobias Grosser wrote:
> 1) Reach agreement over patch 1 & 2 as well as how patch 4 connects
> with generator.cc
>
> Independently of what interface we generate, the way we use generator.cc
> can remain the same and splitting the general bindings generator from
> the python specific one seems beneficial just by itself. Hence, I would
> like to get these changes fully reviewed (sign-off). If we all agree on
> these changes, it would be great if patches 1 & 2 could already be
> committed, as otherwise any changes to the python bindings necessarily
> will require modifications to these patches and consequently a new
> review.

That's a valid concern. I've put these two patches into my internal
queue to prevent this from happening.

> Patch 4 serves for this cause as an example that tests patch 1
> and 2 and shows they are sufficient to enable the development of a
> multi-language generator.

I disagree. Patch 4 in itself is not sufficient demonstrator.
AFAIU, it only prints information related to classes and
it even has a hard-coded list of classes.
If Patch 4 was all you wanted to achieve, then you could
do that in a much simpler way.

> 2) Get a general review of patch 4
>
> Patch four is the result of three weeks of discussions of analyzing and
> discussing all existing isl bindings and identifying a common base from
> which we can grow a more complete set of bindings. Here it would be
> great to get:
>
> a) feedback on the smartptr interface we developed
>
> We completed the design of the smartpointer interface, after
> extensively evaluating a large range of choices
> and making sure we believe this smartpointer interface can serve as a
> base for all the enhancements we have
> in mind. As this design will be the base for future discussions and
> developments, it would be good to
> get your opinion on the choices we have taken and to make sure our
> future discussions start off from sth
> we all agree with.

I believe I did this in the review you are replying to.

> b) feedback on implementation of C++ generator
>
> I am interested to get feedback on the way I implemented the C++
> generator. I will develop future extensions
> using the same style. By making sure the implementation choices I have
> taken match your preferred style,
> future patches hopefully can adopt this style and will require less
> refactoring when pushed for review.

I'm a bit swamped at the moment, but I had a quick look and sent out
a second review.

> I added this longer explanation to the commit message:
>
> The r-value version of the copy() function is not needed as coping an

copying

> object that will be freed right after is inefficient. In such a
> situation, the use of release() is more efficient as it avoids the
> unnecessary object copy. By deleting the r-value version of copy we
> ensure that the user does not accidentally use the copy() in situations
> where it is preferable to call release().

ok

skimo

Michael Kruse

unread,
Oct 21, 2016, 11:14:03 AM10/21/16
to Sven Verdoolaege, Michael Kruse, Tobias Grosser, isl Development
2016-10-21 16:09 GMT+02:00 Sven Verdoolaege <skim...@kotnet.org>:
>> The goal is, to notify the user early (at least in debug builds) when
>> an ISL function returns null, and passes it on without checking for
>> NULL. That is, the methods would check whether one of the arguments is
>> invalid/NULL.
>
> I don't understand. isl function are _designed_ to pass along NULL values.
> (Maybe I failed to parse your sentence properly.)

I think you correctly got what I want to explain and I understand your
objection.

ISL is designed to pass on NULL values, but for C++ this rather
untypical. Calling a method on nullptr is undefined behavior. In this
case the nullptr is encapsulated in an object, so it could be allowed
to. It still might not be what a C++ programmers might expect as other
smart pointer implementations also cannot be used on nullptr (eg.
operator->()). We essentially define use of nullptr objects as
undefined behavior.

There is also the goal to catch programmer errors early. Example:

Set s = ...;
... = isl_set_unwrap(s.release());
s.getCtx();

Here, the error is that the ownership of s has been taken away, but s
is still be used. This is a bug that we would like to catch early,
instead to pass on. If it is eventually discovered, it will be later
in the program, but at that point it might be difficult to see where
the error originated.

However, there is at least one occasion where ISL might return NULL
even if the input was correct. That is, when max_operations is set.
With these asserts, we force the programmer to check for NULL after
each operation. We could for instance only fail assertions if
max_operation is not set. Because this happens in assert(), this would
have no impact to release builds.

We could also throw exceptions, which would be the most C++-ish thing
to do. Unfortunately, exceptions are switched off in the LLVM code
base so we need at least an alternative behavior.

Because we preferred a minimal implementation to start with, an we
could always change the undefined behavior to a defined one if
necessary.


>> > In any case, don't use assert.
>>
>> What do you suggest instead of assert? And why?
>
> What do you want assert to do?
> Depending on the phase of the moon (or something equally obscure
> such as a macro being defined), it either does nothing or
> prints a cryptic error message.
> Choose which of these two you want and if you go for the error
> message, then print a proper one.

As explained above, we essential say using the wrapper in NULL state
is undefined behavior. It is not supposed to happen and if it still
does, the implementation allowed to do anything that it thinks is
appropriate.

In debug builds, it notifies the user that they have a bug in their code.

In release builds, they are ignored to not influence execution time.

Using assert() is very commonplace in C++ programming. Eg. [1]


[1] http://llvm.org/docs/CodingStandards.html#assert-liberally

--
Tardyzentrismus verboten!

Tobias Grosser

unread,
Oct 21, 2016, 5:34:18 PM10/21/16
to sven.ver...@gmail.com, isl-dev...@googlegroups.com
Thank you. I addresses these issue and will repost when this review
thread has been completed.

Best,
Tobias

Tobias Grosser

unread,
Oct 21, 2016, 5:55:18 PM10/21/16
to sven.ver...@gmail.com, isl-dev...@googlegroups.com
On Fri, 2016-10-21 at 16:56 +0200, Sven Verdoolaege wrote:
> On Fri, Oct 21, 2016 at 09:30:27AM +0200, Tobias Grosser wrote:
> > 1) Reach agreement over patch 1 & 2 as well as  how patch 4
> > connects
> > with generator.cc
> >
> > Independently of what interface we generate, the way we use
> > generator.cc
> > can remain the same and splitting the general bindings generator
> > from
> > the python specific one seems beneficial just by itself. Hence, I
> > would
> > like to get these changes fully reviewed (sign-off). If we all
> > agree on
> > these changes, it would be great if patches 1 & 2 could already be
> > committed, as otherwise any changes to the python bindings
> > necessarily
> > will require modifications to these patches and consequently a new
> > review.
>
>
> That's a valid concern.  I've put these two patches into my internal
> queue to prevent this from happening.

Interesting. Does this mean you agree with these patches? How can
external users who maintain patches that have not yet been upstreamed
get access to these patches for example when the python bindings are
extended and the patches need to be updated?

> > Patch 4 serves for this cause as an example that tests patch 1
> > and 2 and shows they are sufficient to enable the development of a
> > multi-language generator.
>
>
> I disagree.  Patch 4 in itself is not sufficient demonstrator.
> AFAIU, it only prints information related to classes and
> it even has a hard-coded list of classes.

The hardcoded list is only to limit the output of the generator to
focus the initial discussion.

> If Patch 4 was all you wanted to achieve, then you could
> do that in a much simpler way.

Sure. Now, what do you think is the minimal set of patches that would
be sufficient to justify these changes. Do I understand that a minimal
function call interface is what you are looking for?

> > 2) Get a general review of patch 4
> >
> > Patch four is the result of three weeks of discussions of analyzing
> > and
> > discussing all existing isl bindings and identifying a common base
> > from
> > which we can grow a more complete set of bindings. Here it would be
> > great to get:
> >
> >    a) feedback on the smartptr interface we developed
> >
> >    We completed the design of the smartpointer interface, after
> >    extensively evaluating a large range of choices 
> >    and making sure we believe this smartpointer interface can serve
> > as a
> >    base for all the enhancements we have
> >    in mind. As this design will be the base for future discussions
> > and
> >    developments, it would be good to 
> >    get your opinion on the choices we have taken and to make sure
> > our
> >    future discussions start off from sth
> >    we all agree with.
>
>
> I believe I did this in the review you are replying to.

Yes, that is already very helpful. Thank you.

> >   b) feedback on implementation of C++ generator
> >
> >   I am interested to get feedback on the way I implemented the C++
> >   generator. I will develop future extensions
> >   using the same style. By making sure the implementation choices I
> > have
> >   taken match your preferred style,
> >   future patches hopefully can adopt this style and will require
> > less
> >   refactoring when pushed for review.
>
>
> I'm a bit swamped at the moment, but I had a quick look and sent out
> a second review.

Thank you.

Best,
Tobias

Sven Verdoolaege

unread,
Oct 24, 2016, 10:13:43 AM10/24/16
to re...@meinersbur.de, Tobias Grosser, isl Development
On Fri, Oct 21, 2016 at 05:13:22PM +0200, Michael Kruse wrote:
> 2016-10-21 16:09 GMT+02:00 Sven Verdoolaege <skim...@kotnet.org>:
> >> The goal is, to notify the user early (at least in debug builds) when
> >> an ISL function returns null, and passes it on without checking for
> >> NULL. That is, the methods would check whether one of the arguments is
> >> invalid/NULL.
> >
> > I don't understand. isl function are _designed_ to pass along NULL values.
> > (Maybe I failed to parse your sentence properly.)
>
> I think you correctly got what I want to explain and I understand your
> objection.
>
> ISL is designed to pass on NULL values, but for C++ this rather
> untypical.

It's not typical for C either, but it is how isl is designed to work.
Since the C++ bindings are apparently meant to be used together
with the C interface, I think it would be confusing to users
that one would behave differently from the other with respect
to NULL pointers.

> There is also the goal to catch programmer errors early. Example:
>
> Set s = ...;
> ... = isl_set_unwrap(s.release());
> s.getCtx();
>
> Here, the error is that the ownership of s has been taken away, but s
> is still be used. This is a bug that we would like to catch early,
> instead to pass on. If it is eventually discovered, it will be later
> in the program, but at that point it might be difficult to see where
> the error originated.

This could be handled by having release() invalidate the "Set"
object (in some way that is different from just setting a pointer
to NULL).

> However, there is at least one occasion where ISL might return NULL
> even if the input was correct. That is, when max_operations is set.
> With these asserts, we force the programmer to check for NULL after
> each operation. We could for instance only fail assertions if
> max_operation is not set. Because this happens in assert(), this would
> have no impact to release builds.

As I explained in my previous message, I don't want any asserts.
I also don't want any special behavior based on options.
Different users may care about different sets of options.
They should handle them themselves, outside of the bindings.

> We could also throw exceptions, which would be the most C++-ish thing
> to do. Unfortunately, exceptions are switched off in the LLVM code
> base so we need at least an alternative behavior.

For pure C++ bindings, I would indeed expect an exception to be thrown.
If this is not possible or not appropriate, then this should be mentioned
in the commit message.

The reason for accepting NULL inputs (but considering them to signal
an error condition) is exactly so that you wouldn't have to check
the result after every single operation.
If you don't have an alternative for that (exceptions would be such
an alternative), then I think you should stick to this behavior.

skimo

Sven Verdoolaege

unread,
Oct 24, 2016, 10:45:50 AM10/24/16
to Tobias Grosser, isl-dev...@googlegroups.com
On Fri, Oct 21, 2016 at 11:55:15PM +0200, Tobias Grosser wrote:
> On Fri, 2016-10-21 at 16:56 +0200, Sven Verdoolaege wrote:
> > On Fri, Oct 21, 2016 at 09:30:27AM +0200, Tobias Grosser wrote:
> > > 1) Reach agreement over patch 1 & 2 as well as  how patch 4
> > > connects
> > > with generator.cc
> > >
> > > Independently of what interface we generate, the way we use
> > > generator.cc
> > > can remain the same and splitting the general bindings generator
> > > from
> > > the python specific one seems beneficial just by itself. Hence, I
> > > would
> > > like to get these changes fully reviewed (sign-off). If we all
> > > agree on
> > > these changes, it would be great if patches 1 & 2 could already be
> > > committed, as otherwise any changes to the python bindings
> > > necessarily
> > > will require modifications to these patches and consequently a new
> > > review.
> >
> >
> > That's a valid concern.  I've put these two patches into my internal
> > queue to prevent this from happening.
>
> Interesting. Does this mean you agree with these patches?

See below for what it means.

> How can
> external users who maintain patches that have not yet been upstreamed
> get access to these patches for example when the python bindings are
> extended and the patches need to be updated?

They can't. The purpose of having them in my internal queue is that
I will notice when this happens and then I will notify you (except
in case you are the one who is changing the python bindings).
Note that this is a special favor to you. I'm not planning on doing
this for anyone else.

> > If Patch 4 was all you wanted to achieve, then you could
> > do that in a much simpler way.
>
> Sure. Now, what do you think is the minimal set of patches that would
> be sufficient to justify these changes. Do I understand that a minimal
> function call interface is what you are looking for?

By default, I would expect the C++ bindings to match the Python bindings.
The minimal C++ bindings at this point would then be those that match
the current Python bindings.
Of course, there may be some divergence in special cases.
If there are any such special cases for the C++ bindings,
then we can discuss them.

skimo

Michael Kruse

unread,
Oct 25, 2016, 7:49:56 AM10/25/16
to Sven Verdoolaege, Michael Kruse, Tobias Grosser, isl Development
2016-10-24 16:13 GMT+02:00 Sven Verdoolaege <skim...@kotnet.org>:
>> We could also throw exceptions, which would be the most C++-ish thing
>> to do. Unfortunately, exceptions are switched off in the LLVM code
>> base so we need at least an alternative behavior.
>
> For pure C++ bindings, I would indeed expect an exception to be thrown.
> If this is not possible or not appropriate, then this should be mentioned
> in the commit message.
>
> The reason for accepting NULL inputs (but considering them to signal
> an error condition) is exactly so that you wouldn't have to check
> the result after every single operation.
> If you don't have an alternative for that (exceptions would be such
> an alternative), then I think you should stick to this behavior.

Would it be ok to have a flag in isl_ctx that controls whether to
throw exceptions?

Michael

--
Tardyzentrismus verboten!

Tobias Grosser

unread,
Oct 25, 2016, 8:43:09 AM10/25/16
to isl-dev...@googlegroups.com
Thank you. ;-)

I wonder, how does this avoid the need for rebasing/re-reviewing. As far
as I understand,
I would still need to rebase the patches in case of changes and then go
through
the review process again, right?

> > > If Patch 4 was all you wanted to achieve, then you could
> > > do that in a much simpler way.
> >
> > Sure. Now, what do you think is the minimal set of patches that would
> > be sufficient to justify these changes. Do I understand that a minimal
> > function call interface is what you are looking for?
>
> By default, I would expect the C++ bindings to match the Python bindings.
> The minimal C++ bindings at this point would then be those that match
> the current Python bindings.
> Of course, there may be some divergence in special cases.
> If there are any such special cases for the C++ bindings,
> then we can discuss them.

This was not the response I expected. I believe we are not so far apart,
even though this reply makes me think we are. So maybe discussing things
just makes this unnecessary complicated -- or works better in person /
over the phone?

The reason for my question was to find a strategy that helps to avoid
running in similar issues as we found with the last C++ bindings. The
need to build up a huge patchset which results in a long and unfocused
discussion. Your suggestion sounds as if you prefer a huge patchset. In
an earlier discussion you suggested to start off with a sufficiently
functional subset. Probably this partial patchset and especially me
limiting the code just to isl_set/basic_set made you think I wanted to
push for something incomplete. That's not the case. I will include in a
next iteration all classes and also a basic function interface to show
that the memory interface works.

Best,
Tobias

jakob...@gmail.com

unread,
Oct 25, 2016, 9:15:05 PM10/25/16
to isl Development, sven.ver...@gmail.com, re...@meinersbur.de, tob...@grosser.es
I do not like the idea of throwing an exception whenever a function is invoked on a null pointer, mainly because it implies the overhead of additional checking whether the pointer is null.

I suggest we decide to either:

A. Preserve the C interface semantics:
- When calling a method of "invalid" object, and forwarding the null pointer to the C interface would not abort, then just silently return a (possibly invalid) object.
- Make sure to not cause any null pointer dereferencing in the C++ wrapper code.
- When passing the null pointer to the C interface would cause an abort, rather check the pointer and throw an exception if it's null.

B. Simply declare any call on an invalid object (one with a null pointer) undefined behavior, and take no precaution for user doing so (which would liberate us from development and maintenance work related to A)

As I mentioned in our earlier discussions, I agree with Sven that asserts are not a reliable way of reporting errors to the user. If Sven prefers to completely avoid them, then I am OK with that. In any case, no one should *rely* on assertions.

Jakob

Sven Verdoolaege

unread,
Oct 26, 2016, 5:20:29 AM10/26/16
to re...@meinersbur.de, Tobias Grosser, isl Development
What problem would be solved by the introduction of this flag?

skimo

Sven Verdoolaege

unread,
Oct 26, 2016, 5:23:50 AM10/26/16
to jakob...@gmail.com, isl Development, re...@meinersbur.de, tob...@grosser.es
On Tue, Oct 25, 2016 at 06:15:05PM -0700, jakob...@gmail.com wrote:
> - When passing the null pointer to the C interface would cause an abort,
> rather check the pointer and throw an exception if it's null.

If isl aborts on a NULL pointer input, then isl should be fixed.

skimo

Sven Verdoolaege

unread,
Oct 26, 2016, 5:50:51 AM10/26/16
to Tobias Grosser, isl-dev...@googlegroups.com
It allows the potential conflict to be _detected_ (prior to the conflict
appearing in the public repo). If and when that happens, we can decide
how to handle the conflict.

skimo

Michael Kruse

unread,
Oct 26, 2016, 6:56:57 AM10/26/16
to Sven Verdoolaege, Michael Kruse, Tobias Grosser, isl Development
The problem that some users cannot use exceptions (-> LLVM), while for
others they would be an elegant programming model. They could be
switched on by default, and then switched off
(isl_ctx_set_disable_exceptions(0)) for users that do not want
exceptions. Python also has exceptions, so this flag could be used in
the python bindings as well.

I have have exceeding the max_operations quota in mind. It would allow
to directly 'jump' to the error-handling code instead to pass NULL
through several ISL functions and/or explicit handling for functions
that do not return a pointer such as isl_set_is_empty.

Michael

--
Tardyzentrismus verboten!

Sven Verdoolaege

unread,
Oct 27, 2016, 6:36:56 AM10/27/16
to re...@meinersbur.de, Tobias Grosser, isl Development
I don't know. I'm not a C++ expert, but whether or not exceptions
should be thrown doesn't sound like something you should configure
at run-time. Wouldn't a user have to be prepared to handle exceptions
if she is calling a library that can throw exceptions?
Maybe it would be better to have two separate variants of
the C++ bindings?
Is there any demand for exceptions throwing C++ bindings?

This proposal also begs the question as to what happens when
the option is turned off.

skimo

jakob...@gmail.com

unread,
Oct 27, 2016, 12:45:19 PM10/27/16
to isl Development, re...@meinersbur.de, tob...@grosser.es, sven.ver...@gmail.com
On Thursday, October 27, 2016 at 3:36:56 AM UTC-7, skimo wrote:
Maybe it would be better to have two separate variants of
the C++ bindings?
Is there any demand for exceptions throwing C++ bindings?

This proposal also begs the question as to what happens when
the option is turned off.

skimo

It is apparent in this thread that several people have different opinions regarding whether exceptions should be used at all, and how in particular they should be introduced.

I propose to start by declaring the use of C++ classes with null pointers undefined behavior. This does not preclude possibly adding exceptions later, but it allows us to move forward now.

Alexandre Isoard

unread,
Oct 28, 2016, 6:12:48 AM10/28/16
to Jakob Leben, isl Development, re...@meinersbur.de, Tobias Grosser, Sven Verdoolaege
I think we agreed to have the C++ bindings exception safe but not throwing exceptions, isn't it?

In case of null pointers, a class should be able to be destroyed or assigned to safely (that is, no assert for those).
For the other operations, if isl is expected to have a well defined behavior on null pointers (as Sven suggest), then we should not assert that it is non-null and rely on isl to properly cope with it.
In which case, we cannot declare the use of C++ classes with null pointers as undefined behavior. It is a isl defined behavior.

There is a good argument for adding assert: that is that in most cases a null object is unwanted by the user and is usually due to a mistake.

--

---
You received this message because you are subscribed to the Google Groups "isl Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isl-development+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--
Alexandre Isoard

jakob...@gmail.com

unread,
Oct 28, 2016, 12:46:45 PM10/28/16
to isl Development, jakob...@gmail.com, re...@meinersbur.de, tob...@grosser.es, sven.ver...@gmail.com
On Friday, October 28, 2016 at 3:12:48 AM UTC-7, Alexandre Isoard wrote:
I think we agreed to have the C++ bindings exception safe but not throwing exceptions, isn't it?

Yes, I think that was a consensus for the first implementation, but it was due to the idea that asserts can somewhat make up for the lack of exceptions. Since Sven rejected asserts, the discussion about exceptions was reheated by Michael. So I don't yet see a consensus here at the moment.
 
In case of null pointers, a class should be able to be destroyed or assigned to safely (that is, no assert for those).

Of course, that's a given.
 
For the other operations, if isl is expected to have a well defined behavior on null pointers (as Sven suggest), then we should not assert that it is non-null and rely on isl to properly cope with it.
In which case, we cannot declare the use of C++ classes with null pointers as undefined behavior. It is a isl defined behavior.

I disagree. On the level of C++ bindings, we can define whatever we want undefined behavior - even though the behavior is actually deterministic. Afaik, "undefined behavior" just means dependence on a particular implementation or dynamic factors that can change without notice, and hence "not to be relied upon":

One valid argument for promoting isl's null pointer semantics to the level of C++ may be familiarity to the C user. However, in this particular case, the fact that the pointer is wrapped in a C++ class and hidden from the user, and the syntactic difference between method and free function calls, as well as common C++ practices and expectations would favor not promoting the null pointer semantics.


Michael Kruse

unread,
Oct 28, 2016, 4:46:52 PM10/28/16
to Sven Verdoolaege, Michael Kruse, Tobias Grosser, isl Development
2016-10-27 12:36 GMT+02:00 Sven Verdoolaege <skim...@kotnet.org>:
>> The problem that some users cannot use exceptions (-> LLVM), while for
>> others they would be an elegant programming model. They could be
>> switched on by default, and then switched off
>> (isl_ctx_set_disable_exceptions(0)) for users that do not want
>> exceptions. Python also has exceptions, so this flag could be used in
>> the python bindings as well.
>>
>> I have have exceeding the max_operations quota in mind. It would allow
>> to directly 'jump' to the error-handling code instead to pass NULL
>> through several ISL functions and/or explicit handling for functions
>> that do not return a pointer such as isl_set_is_empty.
>
> I don't know. I'm not a C++ expert, but whether or not exceptions
> should be thrown doesn't sound like something you should configure
> at run-time. Wouldn't a user have to be prepared to handle exceptions
> if she is calling a library that can throw exceptions?
> Maybe it would be better to have two separate variants of
> the C++ bindings?
> Is there any demand for exceptions throwing C++ bindings?
>
> This proposal also begs the question as to what happens when
> the option is turned off.


The switch would change between C++-style exceptions and ISL-style
propagation of NULL.

Because of LLVM, we cannot have an exceptions-only version. At the
same time it would be nicer for other users to actually use the
facilities offered by C++ (otherwise one could just continue to use
the C interface).

I can think of three methods to choose between exceptions and NULL-propagation.
0) No exceptions at all (option for completeness)
1) Setting a macro at compile-time
2) Flag in isl_ctx
3) #including different headers.
4) Different class names
5) Different method/function names

From your past messages I assume that you would not accept solution
1). For 2) I was asking you opinion on.

I previously did not think about option 3-5). This might be
alternatives, but with their own drawbacks. One will see problems with
3) if headers include the other version.

Michael

--
Tardyzentrismus verboten!

Michael Kruse

unread,
Oct 28, 2016, 5:01:07 PM10/28/16
to Jakob Leben, isl Development, Michael Kruse, Tobias Grosser, Sven Verdoolaege
2016-10-28 18:46 GMT+02:00 <jakob...@gmail.com>:
> On Friday, October 28, 2016 at 3:12:48 AM UTC-7, Alexandre Isoard wrote:
>>
>> I think we agreed to have the C++ bindings exception safe but not throwing
>> exceptions, isn't it?
>
>
> Yes, I think that was a consensus for the first implementation, but it was
> due to the idea that asserts can somewhat make up for the lack of
> exceptions. Since Sven rejected asserts, the discussion about exceptions was
> reheated by Michael. So I don't yet see a consensus here at the moment.
>
>>
>> In case of null pointers, a class should be able to be destroyed or
>> assigned to safely (that is, no assert for those).
>
>
> Of course, that's a given.
>
>>
>> For the other operations, if isl is expected to have a well defined
>> behavior on null pointers (as Sven suggest), then we should not assert that
>> it is non-null and rely on isl to properly cope with it.
>> In which case, we cannot declare the use of C++ classes with null pointers
>> as undefined behavior. It is a isl defined behavior.
>
>
> I disagree. On the level of C++ bindings, we can define whatever we want
> undefined behavior - even though the behavior is actually deterministic.
> Afaik, "undefined behavior" just means dependence on a particular
> implementation or dynamic factors that can change without notice, and hence
> "not to be relied upon":

"Undefined behavior" would allow us to make decisions about the actual
behavior at a later point to keep the scope of discussion smaller, as
Tobias intended to.

However, given that we cannot use assert(), any implementation would
de-facto define a behavior, which we would be stuck at if users start
relying on it. This is why I think we should already start such a
fundamental discussion now.


> One valid argument for promoting isl's null pointer semantics to the level
> of C++ may be familiarity to the C user. However, in this particular case,
> the fact that the pointer is wrapped in a C++ class and hidden from the
> user, and the syntactic difference between method and free function calls,
> as well as common C++ practices and expectations would favor not promoting
> the null pointer semantics.
>

+1

I think it would be kind of sad to not be able to use exceptions for
OutOfQuota situations.

Michael

--
Tardyzentrismus verboten!

Michael Kruse

unread,
Oct 28, 2016, 5:10:14 PM10/28/16
to Sven Verdoolaege, Michael Kruse, Tobias Grosser, isl Development
2016-10-27 12:36 GMT+02:00 Sven Verdoolaege <skim...@kotnet.org>:
> I don't know. I'm not a C++ expert, but whether or not exceptions
> should be thrown doesn't sound like something you should configure
> at run-time. Wouldn't a user have to be prepared to handle exceptions
> if she is calling a library that can throw exceptions?

Isn't this already true for isl_ctx_set_error? Code might expect the
ISL_ON_ERROR_ABORT setting, yet it can even be set with command-line
options.

Exceptions will still allow handling errors of called functions that
don't have handling code for them. If even the program doesn't catch
the exception, the behavior is the same as with ISL_ON_ERROR_ABORT:
program termination.

Michael

--
Tardyzentrismus verboten!

Tobias Grosser

unread,
Nov 1, 2016, 6:23:20 PM11/1/16
to jakob...@gmail.com, isl Development, re...@meinersbur.de, sven.ver...@gmail.com
I would agree with Jakob's suggestion. Let's not force assertions on the
user for now, we can add a warning that a more elaborate exception
handling interface might be added later on, sucht that he/she should be
prepared to adapt his/her code.

Best,
Tobias

Sven Verdoolaege

unread,
Nov 2, 2016, 7:48:56 AM11/2/16
to re...@meinersbur.de, Tobias Grosser, isl Development
On Fri, Oct 28, 2016 at 11:09:33PM +0200, Michael Kruse wrote:
> 2016-10-27 12:36 GMT+02:00 Sven Verdoolaege <skim...@kotnet.org>:
> > I don't know. I'm not a C++ expert, but whether or not exceptions
> > should be thrown doesn't sound like something you should configure
> > at run-time. Wouldn't a user have to be prepared to handle exceptions
> > if she is calling a library that can throw exceptions?
>
> Isn't this already true for isl_ctx_set_error? Code might expect the
> ISL_ON_ERROR_ABORT setting, yet it can even be set with command-line
> options.

Indeed, user code shouldn't depend on the ISL_ON_ERROR_ABORT setting and
user code doesn't need to depend on this setting.
In the case of exceptions, however, AFAIU you are saying that you
wouldn't be able to handle the exceptions.

skimo

Sven Verdoolaege

unread,
Nov 2, 2016, 10:05:59 AM11/2/16
to re...@meinersbur.de, Tobias Grosser, isl Development
On Fri, Oct 28, 2016 at 10:46:11PM +0200, Michael Kruse wrote:
> 2016-10-27 12:36 GMT+02:00 Sven Verdoolaege <skim...@kotnet.org>:
> >> The problem that some users cannot use exceptions (-> LLVM), while for
> >> others they would be an elegant programming model. They could be
> >> switched on by default, and then switched off
> >> (isl_ctx_set_disable_exceptions(0)) for users that do not want
> >> exceptions. Python also has exceptions, so this flag could be used in
> >> the python bindings as well.
> >>
> >> I have have exceeding the max_operations quota in mind. It would allow
> >> to directly 'jump' to the error-handling code instead to pass NULL
> >> through several ISL functions and/or explicit handling for functions
> >> that do not return a pointer such as isl_set_is_empty.
> >
> > I don't know. I'm not a C++ expert, but whether or not exceptions
> > should be thrown doesn't sound like something you should configure
> > at run-time. Wouldn't a user have to be prepared to handle exceptions
> > if she is calling a library that can throw exceptions?
> > Maybe it would be better to have two separate variants of
> > the C++ bindings?
> > Is there any demand for exceptions throwing C++ bindings?
> >
> > This proposal also begs the question as to what happens when
> > the option is turned off.
>
> The switch would change between C++-style exceptions and ISL-style
> propagation of NULL.

Wouldn't that just force users to support both?

> Because of LLVM, we cannot have an exceptions-only version. At the
> same time it would be nicer for other users to actually use the
> facilities offered by C++ (otherwise one could just continue to use
> the C interface).
>
> I can think of three methods to choose between exceptions and NULL-propagation.
> 0) No exceptions at all (option for completeness)
> 1) Setting a macro at compile-time
> 2) Flag in isl_ctx
> 3) #including different headers.
> 4) Different class names
> 5) Different method/function names
>
> From your past messages I assume that you would not accept solution
> 1). For 2) I was asking you opinion on.
>
> I previously did not think about option 3-5). This might be
> alternatives, but with their own drawbacks. One will see problems with
> 3) if headers include the other version.

In the simplest case, there is only one mechanism for handling
exceptional cases.
It's still not entirely clear to me if there is anyone who wants
an exceptions based mechanism. I may have missed something, but
it seems that in this thread you are the only one who requested such,
but you are also the one who said you couldn't use it, so I'm a bit confused.

In any case, if you want to support two mechanisms and if you want to
make it possible to use both from the same application (which makes the
isl_ctx flag impossible), then I guess you would need to have differently
named classes, maybe just by using different name spaces.

skimo

Tobias Grosser

unread,
Nov 2, 2016, 10:13:38 AM11/2/16
to sven.ver...@gmail.com, re...@meinersbur.de, isl Development
I believe Michael is mainly concerned about backward compatibility, as
he does not want to end up in a situation where we did not discuss this
point today and then future changes might become impossible due to the
very strong backward compatibility guarantees we are giving for the isl
C interface (which Michael assumes will also hold for the C++
interface).

I personally would suggest to just document clearly that the C++ is
experimental for the next two releases and that we do not provide
backward compatibility guarantees for it just yet.

Tobias Grosser

unread,
Nov 16, 2016, 4:32:25 PM11/16/16
to sven.ver...@gmail.com, re...@meinersbur.de, isl Development
On Wed, Nov 2, 2016, at 03:05 PM, Sven Verdoolaege wrote:
We just discussed this topic on the Polly phone call and Alexandre,
Michael, and me are OK to not change the error handling strategy in
comparison to the C interface. Michael still pointed out that there may
be good reasons for a more C++ error handling strategy, but we probably
should wait for actual use cases to push and design this further. As
Jakob also was in favor of an exception free interface, I updated my
patchset and removed the assertions.

Best,
Tobias

Tobias Grosser

unread,
Nov 17, 2016, 3:46:36 AM11/17/16
to isl-dev...@googlegroups.com
I updated my patchset now:

https://github.com/tobig/isl/tree/cpp-generator

and also attached the latest versions of the bindings. As requested by
Sven, I do not artificially limit them to just sets and maps.

As a next step, I will re-start the discussion on the function call
interface.

Best,
Tobias
isl.h

Jakob Leben

unread,
Nov 17, 2016, 4:05:46 PM11/17/16
to Tobias Grosser, isl Development
Hi Tobias,

Is there a reason why the operator= does not operate on const & argument?

Jakob


Best,
Tobias

--

---
You received this message because you are subscribed to a topic in the Google Groups "isl Development" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/isl-development/ECQsgrDuSNc/unsubscribe.
To unsubscribe from this group and all its topics, send an email to isl-development+unsubscribe@googlegroups.com.

Tobias Grosser

unread,
Nov 17, 2016, 4:47:06 PM11/17/16
to isl-dev...@googlegroups.com
On Thu, Nov 17, 2016, at 10:05 PM, Jakob Leben wrote:
> Hi Tobias,
>
> Is there a reason why the operator= does not operate on const & argument?

http://en.cppreference.com/w/cpp/language/copy_assignment

I use case (1), with the copy and swap idiom as discussed here:

http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom

Best,
Tobias
> > isl-developme...@googlegroups.com.
> > For more options, visit https://groups.google.com/d/optout.
> >
>
> --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "isl Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to isl-developme...@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages