cpp: export id #18
cpp: export id #18
Conversation
|
@TobiG I don't understand how CI works here... |
|
It does not. I should probably just disable it again. |
There was a problem hiding this comment.
Hi Alex,
thanks for this patch. It looks really great and the functions generated make a lot of sense.
Some high-level questions:
What does "disallow unnamed ids by construction, ensure that ids remain value comparable mean"? It seems your third constructor allows to construct an unnamed id?
Just pretty printing the right bindings is a good idea for a first draft of the patch. However, this will prevent these function to exist on the python side of things. Also, I feel it would be slightly more in-style to just generate this stuff instead of pattern matching on the types. For this we should likely ask Sven for feedback. He has strong concerns about this.
| *static_cast<int *>(user) = 0; | ||
| } | ||
|
|
||
| /* Test that user pointers of the ids are not freed as long as the exists at |
| * In a scope, create an id with a flag as a user object and a user_free that | ||
| * resets the flag. Use the id in a set object that lives outside the given | ||
| * scope. Check that flag is still set after the id object went out of the | ||
| * scope. Check that flag is reset after the set object went of of scope. |
| " void (*deleter)(void *) = nullptr);\n" | ||
| " inline id(isl::ctx ctx, void *usr,\n" | ||
| " void (*deleter)(void *) = nullptr);\n" | ||
| " inline bool has_name() const;\n" |
There was a problem hiding this comment.
We could also just add an __isl_bool isl_id_has_name() method and export it.
| isl_bool isl_set_has_tuple_id(__isl_keep isl_set *set); | ||
| __isl_export |
There was a problem hiding this comment.
I feel these could be a separate commit. But let's worry about this when upstreaming.
| " void *usr,\n" | ||
| " void (*deleter)(void *) = nullptr);\n" | ||
| " inline id(isl::ctx ctx, void *usr,\n" | ||
| " void (*deleter)(void *) = nullptr);\n" |
There was a problem hiding this comment.
The functions you choose look good. I am slightly afraid that pretty printing them like this won't fly. At the very least, the first one could be declared as C function and normally exposed. This would make sure that isl ids are also available in python.
Not sure about the other two. Do they make any sense in python? I mean, do we want to pass references to other python objects as user point? (A deleter certainly does not make sense)
There was a problem hiding this comment.
I would rather maintain a dict id->object separately. My recommendation would be to do so in C++, too. The only place where I need the user pointer is interacting with PPCG-generated schedule trees.
As a radical solution, we can hide the user pointer from C++ and python completely. If the user needs it, they can do access it by extracting a C isl_id and be thus "warned" that the underlying data is a C object.
|
|
||
| static void reset_flag(void *user) { | ||
| *static_cast<int *>(user) = 0; | ||
| } |
There was a problem hiding this comment.
Maybe add this as a lambda function into the test function below?
Yes, actually there should be an assert.
I'd like to avoid modifying the existing public API. Changing something like id allocation seems to be a pretty large change. We could just export User-pointers for python do not make sense. I'm not aware of any address guarantees for python objects, which are necessary to ensure uniqueness. So I would suggest to pretty-print the python class only with a name-based constructor. With these arguments, pattern-matching seems a worthy solution. |
Introduce special behavior for the id class with two features: disallow unnamed ids by construction, ensure that ids remain value-comparable. Generally, isl_id behaves like a reference-counted smart pointer to the name string and the user pointer. Additionally, it guarantees that ids with identical names and user pointers are pointer-comparable. An id object can have a "user_free" callback that is called when the reference counter reaches zero. Existing mechanism for callbacks does not apply to "user_free" callbacks as it modifies the user object passed to the callback. In particular, it creates a new object of a custom type in each call of the function that takes a callback and passes it instead of the original user pointer. Therefore, two ids constructed independently from the same user pointer would no longer be pointer-comparable. Therefore, one must pass the user pointer directly. The "user_free" callback must in turn remain a C function pointer. An alternative solution that supports std::function would require maintaining a map between user pointers and custom objects that were passed when constructing isl_ids; however, it would break direct comparability between isl_ids constructed using C and C++ interface. Support void and void * as return and argument types in the generator. Modify the generator to inject custom method declarations and definitions in the class based on the class name. Inject custom constructors, utility methods and comparison operators for isl::id. Custom constructors take either a name or a user pointer, or both. The "user_free" callback can be optionally provided in constructors or set up separately. This callback must be a C function pointer because it will be called from the C code. The user pointer is passed as void *, which can be replaced by template methods in the future, except in the "user_free" callback. The "set_user_free" function is injected so as to avoid handling a special case in callback generation. Signed-off-by: Oleksandr Zinenko <git@ozinenko.com>
|
@TobiG Would be nice to automate the process of pulling from the main repo and merging it into the continuous-integration branch. |
|
Great. We should just commit the CI file to the main isl repo. That would make this work here out-of-the-box. |
Introduce special behavior for the id class with two features: disallow unnamed
ids by construction, ensure that ids remain value-comparable.
Generally, isl_id behaves like a reference-counted smart pointer to the name
string and the user pointer. Additionally, it guarantees that ids with
identical names and user pointers are pointer-comparable. An id object can have
a "user_free" callback that is called when the reference counter reaches zero.
Existing mechanism for callbacks does not apply to "user_free" callbacks as it
modifies the user object passed to the callback. In particular, it creates a
new object of a custom type in each call of the function that takes a callback
and passes it instead of the original user pointer. Therefore, two ids
constructed independently from the same user pointer would no longer be
pointer-comparable. Therefore, one must pass the user pointer directly. The
"user_free" callback must in turn remain a C function pointer. An alternative
solution that supports std::function would require maintaining a map between
user pointers and custom objects that were passed when constructing isl_ids;
however, it would break direct comparability between isl_ids constructed using
C and C++ interface.
Support void and void * as return and argument types in the generator. Modify
the generator to inject custom method declarations and definitions in the class
based on the class name. Inject custom constructors, utility methods and
comparison operators for isl::id. Custom constructors take either a name or a
user pointer, or both. The "user_free" callback can be optionally provided in
constructors or set up separately. This callback must be a C function pointer
because it will be called from the C code. The user pointer is passed as
void *, which can be replaced by template methods in the future, except in the
"user_free" callback. The "set_user_free" function is injected so as to avoid
handling a special case in callback generation.
Closes #8