search

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.72k stars 384 forks source link

Remove C99 style inline declaration #731

Closed chris-liddell closed 5 months ago

chris-liddell commented 5 months ago

Trivial patch.

stweil commented 5 months ago

That code was added in commit df0fe14f8.

I am curious: are there still build environments which don't accept such inline declarations (which I personally like because they require less code lines and are easier to review)?

If we want to avoid inline declarations, I suggest to update the commit and format the declaration like most others in the Leptonica code (left aligned, empty line after declaration).

DanBloomberg commented 5 months ago

I agree, Stefan,and prefer the C99 inlining of declarations, for the reasons you give. However, at least until a year ago, my gnu C compiler treated it as an error.

Issue #724 was a failure to build with cmake because -lm wasn't included. When trying to replicate this, I found I couldn't build either because it was trying to use C17. Ubuntu 20.04 comes with gcc9, which doesn't know about C17.

So a month ago I downloaded gcc 13.1.0. However commit https://github.com/DanBloomberg/leptonica/commit/df0fe14f8c6ae1b1969994bed41c878adcc580a5 was done back in August 2023, so my previous gcc also was allowing these inline declarations and didn't flag the problem.

Bottom line: I think we're stuck with avoiding inlines for the indefinite future.

@chris-liddell Look at some functions. One additional rule on these declarations: 2 spaces between the type and the variable; e.g.,

{
l_int32  d;
stweil commented 5 months ago

C99 is now more that 20 years old. "C99 is substantially completely supported as of GCC 4.5". gcc 4.5.0 was released April 14, 2010. I think that compilers are no longer a reason to avoid them.

DanBloomberg commented 5 months ago

You are probably correct, and perhaps we should not sweat the inline declarations going forward.

Perhaps more important than the date that gcc first supported C99 is the reluctant acceptance of C99 by Microsoft

This is from Wikipedia:

Most C compilers provide support for at least some of the features introduced in C99.

Historically, Microsoft has been slow to implement new C features in their Visual C++ tools, instead focusing mainly on supporting developments in the C++ standards.[13] However, with the introduction of Visual C++ 2013 Microsoft implemented a limited subset of C99, which was expanded in Visual C++ 2015.[14]

chris-liddell commented 5 months ago

Oops, sorry, I did miss the other requirements....

This came up because we (Ghostscript) still get used on some older, obscure platforms (like a Unix emulation environment running on IBM s/390 mainframes) where gcc is not the de facto standard. IBM's C compiler lags behind and, due to the astronomical upgrade costs, their customers lag even further behind. Which is why Ghostscript still avoids C99 and later constructs.

(In all fairness, we realise this is ultimately a losing battle!)

DanBloomberg commented 5 months ago

Thank you for explaining. Those legacy issues are always annoying. We'll keep avoiding inline declarations.