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

Remove C99 style inline declaration #731

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

chris-liddell
Copy link
Contributor

Trivial patch.

@stweil
Copy link
Collaborator

stweil commented Feb 1, 2024

That code was added in commit df0fe14.

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).

src/adaptmap.c Outdated Show resolved Hide resolved
@DanBloomberg
Copy link
Owner

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 df0fe14 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
Copy link
Collaborator

stweil commented Feb 2, 2024

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
Copy link
Owner

DanBloomberg commented Feb 2, 2024

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
Copy link
Contributor Author

chris-liddell commented Feb 2, 2024

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!)

Copy link
Collaborator

@stweil stweil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now, thank you!

@DanBloomberg DanBloomberg merged commit cb91576 into DanBloomberg:master Feb 2, 2024
@DanBloomberg
Copy link
Owner

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

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.

3 participants