DanBloomberg / leptonica

Leptonica is an open source library containing software that is broadly useful for image processing and image analysis applications. The official github repository for Leptonica is: danbloomberg/leptonica. See leptonica.org for more documentation.
Other
1.76k stars 387 forks source link

Suggestion: Use standard (C99) int types #645

Closed amitdo closed 1 year ago

amitdo commented 1 year ago

https://en.cppreference.com/w/c/types/integer

l_int16 -> int16_t; l_uint16 -> uint16_t
l_int32 -> int32_t; l_uint32 -> uint32_t
l_int64 -> int64_t; l_uint64 -> uint64_t
DanBloomberg commented 1 year ago

We're talking about changing 500 files in src and prog !

Furthermore, the current naming scheme is simple and consistent with both ints and floats. Because the fixed width float typedefs would not be changed, the naming scheme would end up inconsistent between ints and floats.

amitdo commented 1 year ago

We're talking about changing 500 files in src and prog !

This task can be done by a few lines script.

344 also had hundreds of replacements.

Furthermore, the current naming scheme is simple and consistent with both ints and floats. Because the fixed width float typedefs would not be changed, the naming scheme would end up inconsistent between ints and floats.

It's true that it will be inconsistent.

As a note, the upcoming C++23 standard solves this inconstancy. It adds a new header \<stdfloat>. This will probably be adopted by C sometime in the future...

Since you do not like this idea. I'll close this issue.

DanBloomberg commented 1 year ago

Thanks. In a few decades, this may work out ;-)

amitdo commented 1 year ago

I have patience :-)

stweil commented 1 year ago

We're talking about changing 500 files in src and prog !

As Amit has written this can be scripted (that change just took less than 15 minutes including a test build). The good news is that l_int32 and int32_t (and all other integer types) have the same number of characters, so there is no need to reformat any comments.

And for compatibility the current integer types can still be provided in environ.h as typedefs of POSIX types.

DanBloomberg commented 1 year ago

I agree that there are some things we can do easily, however ...

I set this all up in 2000 so we'd never have to make any widespread changes, either in the library or in programs that use it.

See Section 5. Typedefs in README.html for how I viewed compatibility and customization. This was, as stated there, my approach to handling the lack of standardized C typedefs for built-ins. It's interesting that here we are, 22 years later, and that issue is still with us.

In any event, I was expecting that any necessary modification could still use the leptonica l_* typedefs.

I'm probably a bit old-fashioned (hey, leptonica is still in C), but as I see it, the current approach is simple, it will continue to work no matter what future typedef standards (if any) for C are created, the typedefs in environ.h will continue to be necessary to support legacy code using leptonica, and "if it ain't broke, don't fix it" seems appropriate.