Renaming bool_t
18 views
Skip to first unread message
Field Van Zee
Jul 13, 2020, 2:52:19 PM7/13/20
to blis-devel
Friends of BLIS,
A contributor on GitHub has found that arm_sve.h defines a bool_t type [1]. Obviously, this conflicts with the bool_t used in blis.h. Both sides can point fingers at the other and ask why didn't they use a different name, but this would not be productive as we are equally justified (or unjustified) in using the name.
At a deeper level, BLIS has always committed a minor sin by using _t in its typedefs -- a suffix which is reserved by POSIX. I pressed forward with the names simply because there were no conflicts that we knew of *at the time*. In retrospect, this was somewhat shortsighted. But on the other hand, we made it a *long* time (seven years) without any conflicts between our namespace and those of the rest of the world. The "right" solution today (as it would have been back then) would be to rename all types with a bli_ or blis_ prefix, a la pthreads. However, I don't like how this convention would significantly lengthen many type names (blis_dim_t, blis_trans_t, blis_diag_t, etc), which, of course, is one of the reasons I used _t to begin with. If we were going to rename all the types today, I would much rather add another letter to the _t suffix (e.g. _tt, _ty, etc).
As a short-term fix, I will likely rename bool_t to something else such as boole_t. However, this will unfortunately "break" any existing user code that uses bool_t. I don't see any way around this, though. On the bright side, nothing in our public user-level API uses bool_t [2], so this should have minimal practical impact.
I am posting this message to start a dialogue specifically on the bool_t rename and the topic of type names in BLIS more generally. If you have any comments, please feel free to share them.
Field
[1] https://github.com/flame/blis/issues/420
[2] https://github.com/flame/blis/blob/master/docs/BLISTypedAPI.md
A contributor on GitHub has found that arm_sve.h defines a bool_t type [1]. Obviously, this conflicts with the bool_t used in blis.h. Both sides can point fingers at the other and ask why didn't they use a different name, but this would not be productive as we are equally justified (or unjustified) in using the name.
At a deeper level, BLIS has always committed a minor sin by using _t in its typedefs -- a suffix which is reserved by POSIX. I pressed forward with the names simply because there were no conflicts that we knew of *at the time*. In retrospect, this was somewhat shortsighted. But on the other hand, we made it a *long* time (seven years) without any conflicts between our namespace and those of the rest of the world. The "right" solution today (as it would have been back then) would be to rename all types with a bli_ or blis_ prefix, a la pthreads. However, I don't like how this convention would significantly lengthen many type names (blis_dim_t, blis_trans_t, blis_diag_t, etc), which, of course, is one of the reasons I used _t to begin with. If we were going to rename all the types today, I would much rather add another letter to the _t suffix (e.g. _tt, _ty, etc).
As a short-term fix, I will likely rename bool_t to something else such as boole_t. However, this will unfortunately "break" any existing user code that uses bool_t. I don't see any way around this, though. On the bright side, nothing in our public user-level API uses bool_t [2], so this should have minimal practical impact.
I am posting this message to start a dialogue specifically on the bool_t rename and the topic of type names in BLIS more generally. If you have any comments, please feel free to share them.
Field
[1] https://github.com/flame/blis/issues/420
[2] https://github.com/flame/blis/blob/master/docs/BLISTypedAPI.md
Devin Matthews
Jul 13, 2020, 3:21:10 PM7/13/20
to blis-...@googlegroups.com
Field,
I am of the opinion that types, like functions, global variables, and macros should be namespaced. However, I don't think the _t is necessary, thus bool_t -> bli_bool which is only a 2-letter increase. Indeed this is a breaking change, but you could include a comparability option to enable the old typedefs as well (and which would only affect the public API and user code so that there still wouldn't be a conflict with arm_sve.h).
Devin
I am of the opinion that types, like functions, global variables, and macros should be namespaced. However, I don't think the _t is necessary, thus bool_t -> bli_bool which is only a 2-letter increase. Indeed this is a breaking change, but you could include a comparability option to enable the old typedefs as well (and which would only affect the public API and user code so that there still wouldn't be a conflict with arm_sve.h).
Devin
--
You received this message because you are subscribed to the Google Groups "blis-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blis-devel+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/blis-devel/0f77a0db-738d-4b07-bc96-dbaec0b334f5o%40googlegroups.com.
Devin Matthews
Jul 13, 2020, 3:24:46 PM7/13/20
to blis-...@googlegroups.com
Although I have to say for bool_t in particular, I don't see the
need for its existence. C99 bool is semantically superior (although
not as good as C++ bool!).
Devin
Devin
To view this discussion on the web visit https://groups.google.com/d/msgid/blis-devel/b4737142-e640-9563-14e8-c3b730f87066%40smu.edu.
Jeff Hammond
Jul 13, 2020, 4:49:28 PM7/13/20
to Field Van Zee, blis-devel
The reservation on thing_t is benign. POSIX isn't a compiler so they can't abuse this rule to compile your code to "rm -rf /" the way the ISO C/C++ people can (but don't, obviously).
What you should do is namespace all your types. Don't use a confusing name like boole_t. Use bli_bool_t.
Jeff
What you should do is namespace all your types. Don't use a confusing name like boole_t. Use bli_bool_t.
Jeff
--
You received this message because you are subscribed to the Google Groups "blis-devel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blis-devel+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/blis-devel/0f77a0db-738d-4b07-bc96-dbaec0b334f5o%40googlegroups.com.
Jeff Hammond
Jul 13, 2020, 4:51:05 PM7/13/20
to Devin Matthews, blis-discuss
I strongly agree here that in the specific case of bool, you should use the built-in language type, unless you care about a compiler that doesn't support C99 _Bool.
Jeff
To view this discussion on the web visit https://groups.google.com/d/msgid/blis-devel/63837da6-8b8b-779f-6261-694c2bdf7f4b%40smu.edu.
Jed Brown
Jul 13, 2020, 4:56:11 PM7/13/20
to Jeff Hammond, Field Van Zee, blis-devel
I agree on bli_bool or bli_bool_t. Namespacing is important and _t is benign if namespaced.
>> <https://groups.google.com/d/msgid/blis-devel/0f77a0db-738d-4b07-bc96-dbaec0b334f5o%40googlegroups.com?utm_medium=email&utm_source=footer>
>> .
>> .
> --
> You received this message because you are subscribed to the Google Groups "blis-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to blis-devel+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/blis-devel/CAGKz%3DuKJwVP3eRHSKDhDrmmepbvASkKOiugo6Nqv0qOmx4pGpA%40mail.gmail.com.
> You received this message because you are subscribed to the Google Groups "blis-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to blis-devel+...@googlegroups.com.
Jeff Diamond
Jul 13, 2020, 5:26:36 PM7/13/20
to blis-...@googlegroups.com
I'd also vote for using the actual bool type unless C99 breaks
blis.
To view this discussion on the web visit https://groups.google.com/d/msgid/blis-devel/CAGKz%3DuKgjiKSWZ9ggCocLLASDMj1VaJSt1qNdqYbWXg9HVibhQ%40mail.gmail.com.
Minh Quan HO
Jul 14, 2020, 9:53:01 AM7/14/20
to blis-devel
For boolean, I vote using C99's `bool`.
For other types, it is not bad (re-)defining BLIS owns neither (for
the expert API for example). For example, being known worldwide as
`int`, OpenCL still defines its own `cl_int`, and so on.
Quan
For other types, it is not bad (re-)defining BLIS owns neither (for
the expert API for example). For example, being known worldwide as
`int`, OpenCL still defines its own `cl_int`, and so on.
Quan
Field G. Van Zee
Jul 22, 2020, 5:40:53 PM7/22/20
to blis-...@googlegroups.com
Thank you everyone for all your comments.
For those in favor of proper namespacing of the typenames, consider that
renaming all the types in BLIS will take a *lot* longer than fixing the
bool_t issue for RuQing Xu, whose feedback prompted me to begin this
dialogue [1]. Also consider that the namespace solution would be quite
disruptive to the API (no matter when it is done, now or later), and the
longer I can put that off the better.
I haven't ruled out taking the namespacing route eventually. But for now
I will move forward with the minimally disruptive transition of bool_t
-> bool.
I've already set the stage by fixing a slew of sloppy/missing typecasts
in a69a4d7. That's phase 1.
Phase 2 will consist of updating the bool_t typedef so that bool_t is
type-defined in terms of C99's bool instead of a BLIS signed integer
(gint_t).
Phase 3 will be textually ripping out all of the instances of "bool_t"
and replacing them with "bool" (and removing the bool_t typedef
altogether). Once this is done, the bool_t type collision from arm_sve.h
should finally be circumvented.
Field
[1] https://github.com/flame/blis/issues/420
>>>> <mailto:blis-devel+...@googlegroups.com>.
>>> <https://groups.google.com/d/msgid/blis-devel/b4737142-e640-9563-14e8-c3b730f87066%40smu.edu?utm_medium=email&utm_source=footer>.
>> <https://groups.google.com/d/msgid/blis-devel/63837da6-8b8b-779f-6261-694c2bdf7f4b%40smu.edu?utm_medium=email&utm_source=footer>.
>>
>>
>>
>> --
>> Jeff Hammond
>> jeff.s...@gmail.com <mailto:jeff.s...@gmail.com>
>> <https://groups.google.com/d/msgid/blis-devel/CAGKz%3DuKgjiKSWZ9ggCocLLASDMj1VaJSt1qNdqYbWXg9HVibhQ%40mail.gmail.com?utm_medium=email&utm_source=footer>.
> <https://groups.google.com/d/msgid/blis-devel/0f131ecb-f4d4-2d6f-ca5c-89f71110300a%40fastmail.com?utm_medium=email&utm_source=footer>.
For those in favor of proper namespacing of the typenames, consider that
renaming all the types in BLIS will take a *lot* longer than fixing the
bool_t issue for RuQing Xu, whose feedback prompted me to begin this
dialogue [1]. Also consider that the namespace solution would be quite
disruptive to the API (no matter when it is done, now or later), and the
longer I can put that off the better.
I haven't ruled out taking the namespacing route eventually. But for now
I will move forward with the minimally disruptive transition of bool_t
-> bool.
I've already set the stage by fixing a slew of sloppy/missing typecasts
in a69a4d7. That's phase 1.
Phase 2 will consist of updating the bool_t typedef so that bool_t is
type-defined in terms of C99's bool instead of a BLIS signed integer
(gint_t).
Phase 3 will be textually ripping out all of the instances of "bool_t"
and replacing them with "bool" (and removing the bool_t typedef
altogether). Once this is done, the bool_t type collision from arm_sve.h
should finally be circumvented.
Field
[1] https://github.com/flame/blis/issues/420
>>>> To view this discussion on the web visit
>>>> https://groups.google.com/d/msgid/blis-devel/0f77a0db-738d-4b07-bc96-dbaec0b334f5o%40googlegroups.com
>>>> <https://groups.google.com/d/msgid/blis-devel/0f77a0db-738d-4b07-bc96-dbaec0b334f5o%40googlegroups.com?utm_medium=email&utm_source=footer>.
>>>> https://groups.google.com/d/msgid/blis-devel/0f77a0db-738d-4b07-bc96-dbaec0b334f5o%40googlegroups.com
>>>
>>> --
>>> You received this message because you are subscribed to the
>>> Google Groups "blis-devel" group.
>>> To unsubscribe from this group and stop receiving emails from it,
>>> send an email to blis-devel+...@googlegroups.com
>>> <mailto:blis-devel+...@googlegroups.com>.
>>> --
>>> You received this message because you are subscribed to the
>>> Google Groups "blis-devel" group.
>>> To unsubscribe from this group and stop receiving emails from it,
>>> send an email to blis-devel+...@googlegroups.com
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/blis-devel/b4737142-e640-9563-14e8-c3b730f87066%40smu.edu
>>> <https://groups.google.com/d/msgid/blis-devel/b4737142-e640-9563-14e8-c3b730f87066%40smu.edu?utm_medium=email&utm_source=footer>.
>>
>> --
>> You received this message because you are subscribed to the Google
>> Groups "blis-devel" group.
>> To unsubscribe from this group and stop receiving emails from it,
>> send an email to blis-devel+...@googlegroups.com
>> <mailto:blis-devel+...@googlegroups.com>.
>> --
>> You received this message because you are subscribed to the Google
>> Groups "blis-devel" group.
>> To unsubscribe from this group and stop receiving emails from it,
>> send an email to blis-devel+...@googlegroups.com
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/blis-devel/63837da6-8b8b-779f-6261-694c2bdf7f4b%40smu.edu
>> <https://groups.google.com/d/msgid/blis-devel/63837da6-8b8b-779f-6261-694c2bdf7f4b%40smu.edu?utm_medium=email&utm_source=footer>.
>>
>>
>>
>> --
>> Jeff Hammond
>> jeff.s...@gmail.com <mailto:jeff.s...@gmail.com>
>> http://jeffhammond.github.io/
>> --
>> You received this message because you are subscribed to the Google
>> Groups "blis-devel" group.
>> To unsubscribe from this group and stop receiving emails from it, send
>> an email to blis-devel+...@googlegroups.com
>> <mailto:blis-devel+...@googlegroups.com>.
>> --
>> You received this message because you are subscribed to the Google
>> Groups "blis-devel" group.
>> To unsubscribe from this group and stop receiving emails from it, send
>> an email to blis-devel+...@googlegroups.com
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/blis-devel/CAGKz%3DuKgjiKSWZ9ggCocLLASDMj1VaJSt1qNdqYbWXg9HVibhQ%40mail.gmail.com
>> <https://groups.google.com/d/msgid/blis-devel/CAGKz%3DuKgjiKSWZ9ggCocLLASDMj1VaJSt1qNdqYbWXg9HVibhQ%40mail.gmail.com?utm_medium=email&utm_source=footer>.
>
> --
> You received this message because you are subscribed to the Google
> Groups "blis-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to blis-devel+...@googlegroups.com
> <mailto:blis-devel+...@googlegroups.com>.
> --
> You received this message because you are subscribed to the Google
> Groups "blis-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to blis-devel+...@googlegroups.com
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/blis-devel/0f131ecb-f4d4-2d6f-ca5c-89f71110300a%40fastmail.com
> <https://groups.google.com/d/msgid/blis-devel/0f131ecb-f4d4-2d6f-ca5c-89f71110300a%40fastmail.com?utm_medium=email&utm_source=footer>.
Field G. Van Zee
Jul 29, 2020, 4:46:19 PM7/29/20
to blis-...@googlegroups.com
A quick follow-up.
I've now implemented the aforementioned three phases (in a69a4d7,
2c554c2, and 00e14cb). Everything looks good so far in my own testing
and on Travis CI.
Please open an issue on github if you encounter any trouble with BLIS's
usage of C99 bool.
Field
I've now implemented the aforementioned three phases (in a69a4d7,
2c554c2, and 00e14cb). Everything looks good so far in my own testing
and on Travis CI.
Please open an issue on github if you encounter any trouble with BLIS's
usage of C99 bool.
Field
Reply all
Reply to author
Forward
0 new messages