Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cpp/python: export isl_id #21

Open
wants to merge 2 commits into
base: continious-integration
Choose a base branch
from

Conversation

ftynse
Copy link
Member

@ftynse ftynse commented Sep 5, 2017

Export isl_id allocation and name management. Disallow unnamed identifiers by
construction and hide user pointers.

Identifiers can have an optional name and an optional user pointer. However,
identifiers without a name and a user pointer are forbidden. The user pointer
is a void * pointer to a C data structure, managed and potentially deallocated
within C code. It is not fully compatible with C++ objects featuring a virtual
destructor and not sufficiently type-safe. The notion of pointer does not exist
in Python. Therefore, it makes sense to hide the presence of a user pointer
from the bindings. This is achieved by providing the "isl_id_from_name"
(constructor) function that only takes a name of the identifier. If necessary,
user pointers may be accessed through C API, but doing so should be
discouraged. As an alternative, a user of C++ or Python bindings may maintain a
mapping between identifier objects and arbitrary data.

The equality of pointer objects is preserved. However, hiding user pointers may
lead to a situation where two identifier objects in C++ or Python are not equal
although their visible attributes (names) are equal. This can only happen if at
least one of the pointers was obtained from C code.

Since identifiers constructed in C are allowed to not have a name, i.e. have a
NULL name, which cannot be expressed as a C++ std::string, provide an
additional function "isl_id_has_name" to check whether the name is not NULL.

Signed-off-by: Oleksandr Zinenko git@ozinenko.com

@ftynse ftynse requested a review from tobiasgrosser September 5, 2017 11:53
@ftynse
Copy link
Member Author

ftynse commented Sep 5, 2017

Here is a minimal Python-friendly version of isl_id. This does not require modifying the generator, but does require modifying the public C API.

I remember @nicolasvasilache wanting to avoid exposing user pointers, you are served :)

@ftynse
Copy link
Member Author

ftynse commented Sep 5, 2017

We could use operator support from #10 and introduce isl_id_eq function to generate a comparison operator and make isl::id objects directly comparable without modifying the generator.

@tobiasgrosser
Copy link
Member

Very cool! This patch implements the essentials of isl::id. It is certainly sufficient for the python bindings, but should also cover the common C++ use cases.

I would pull the extensions of the C interface into a separate commit and motivate there why they are useful.

@ftynse
Copy link
Member Author

ftynse commented Sep 5, 2017 via email

@tobiasgrosser
Copy link
Member

Sure. Still, we commonly commit extensions to the C interface and changes to the exported functions separately. You can just mention that they will be used in the subsequent commit.

Anyway, very cool stuff. Super simple, but as a result also easy to add, I assume.

Introduce utility functions for named identifiers: "isl_id_from_name"
constructs an identifier given a name and "isl_id_has_name" checks whether an
identifier has a name, i.e. whether its "name" field is not NULL. These
functions allow for manipulating identifiers without user pointers. In
particular, they will be used to hide user pointers from C++ and Python
bindings. The "isl_id_has_name" function is necessary because NULL names cannot
be expressed as strings in bindings.

Signed-off-by: Oleksandr Zinenko <git@ozinenko.com>
Export isl_id allocation and name management. Disallow unnamed identifiers by
construction and hide user pointers.

Identifiers can have an optional name and an optional user pointer. However,
identifiers without a name and a user pointer are forbidden. The user pointer
is a void * pointer to a C data structure, managed and potentially deallocated
within C code. It is not fully compatible with C++ objects featuring a virtual
destructor and not sufficiently type-safe. The notion of pointer does not exist
from the bindings. This is achieved by providing the "isl_id_from_name"
(constructor) function that only takes a name of the identifier. If necessary,
user pointers may be accessed through C API, but doing so should be
discouraged. As an alternative, a user of C++ or Python bindings may maintain a
mapping between identifier objects and arbitrary data.

The equality of pointer objects is preserved. However, hiding user pointers may
lead to a situation where two identifier objects in C++ or Python are not equal
although their visible attributes (names) are equal. This can only happen if at
least one of the pointers was obtained from C code.

Signed-off-by: Oleksandr Zinenko <git@ozinenko.com>
@tobiasgrosser
Copy link
Member

Looks great to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants