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

Improve opaque pointers. #66

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@shintaro-iwasaki
Copy link
Contributor

commented Sep 14, 2018

This PR makes opaque types unique, and fixes several problems revealed by this.
The definition is changed as follows:

// Previous
typedef void *ABT_thread;
// New
struct ABT_thread_opaque;
typedef struct ABT_thread_opaque *ABT_thread;

This technique is very common. See https://en.wikipedia.org/wiki/Opaque_pointer for details.
This improvement revealed several problems (see the second commit).

For users, this change is beneficial in the following situation:

// ABT_thread_join(ABT_thread thread);
// ABT_thread_free(ABT_thread *thread);
ABT_thread thread = [...];
ABT_thread_join(&thread); // ERROR, but no warning previously
ABT_thread_free(&thread); // OK

It is because any pointer type (i.e., &thread = void **) can be converted to void *.
After the improvement, the type of &thread becomes struct ABT_thread_opaque **, which is different from struct ABT_thread_opaque *. The compiler therefore warns at the line of ABT_thread_join.

shintaro-iwasaki added some commits Sep 14, 2018

Make opaque types unique.
Make opaque ABT_xxx types unique by introducing opaque struct types, which is
helpful to find a potential bugs.

For example, the previous definition is as follows:
  typedef void *ABT_thread;
The new definition is as follows:
  struct ABT_thread_opaque; // no member variables.
  typedef struct ABT_thread_opaque *ABT_thread;

There are several benefits for development. For example, passing a variable of
ABT_thread* type to an argument of ABT_thread type is now warned by compiler.

Search for "Opaque Pointer" for details.
Fix type bugs.
This commit fix bugs revealed by improving implementation of opaque types.
@shintaro-iwasaki

This comment has been minimized.

Copy link
Contributor Author

commented Sep 14, 2018

test:jenkins

@carns

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2018

Thanks @shintaro-iwasaki, I am a big fan of this change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.