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

Consistently use "static QINLINE" for inline C code #884

Merged
merged 1 commit into from Oct 28, 2016

Conversation

smcv
Copy link
Contributor

@smcv smcv commented Oct 28, 2016

The portable idiom for type-safe macro-like constructs in C is to use
"static inline" where C99 inline is supported, or "static __inline"
on compilers that implement that keyword as a compiler-specific
extension (at least gcc, clang and MSVC do), falling back to just
"static" as a last resort on terrible compilers from the distant past.

Using "static QINLINE" everywhere means there is no point in defining
QINLINE to "static inline" on clang, so stop doing that; QINLINE now
consistently expands to Standard C/C++ inline, or __inline on MSVC,
or to nothing if we don't know how to inline functions on this
compiler.

This silences warnings about redundant qualifiers (static static inline)
for all the functions that were already inline.

There are a couple of uses of non-static QINLINE in C++ code; I've
left those intact, since inline has different (more useful)
semantics in C++, and as far as I'm aware all reasonable C++ compilers
implement it correctly.


As requested by @xycaleth on #869.

The portable idiom for type-safe macro-like constructs in C is to use
"static inline" where C99 inline is supported, or "static __inline"
on compilers that implement that keyword as a compiler-specific
extension (at least gcc, clang and MSVC do), falling back to just
"static" as a last resort on terrible compilers from the distant past.

Using "static QINLINE" everywhere means there is no point in defining
QINLINE to "static inline" on clang, so stop doing that; QINLINE now
consistently expands to Standard C/C++ inline, or __inline on MSVC,
or to nothing if we don't know how to inline functions on this
compiler.

This silences warnings about redundant qualifiers (static static inline)
for all the functions that were already inline.

There are a couple of uses of non-static QINLINE in C++ code; I've
left those intact, since inline has different (more useful)
semantics in C++, and as far as I'm aware all reasonable C++ compilers
implement it correctly.
@ensiform ensiform merged commit ecd38e2 into JACoders:master Oct 28, 2016
@ensiform
Copy link
Member

ensiform commented Oct 28, 2016

Question though: We seem to have QINLINE defined as commented out inline for Mac builds, I wonder why that doesn't work?

Also paging @xycaleth @redsaurus

@smcv
Copy link
Contributor Author

smcv commented Oct 28, 2016

We seem to have QINLINE defined as commented out inline for Mac builds, I wonder why that doesn't work?

My guess would be that it didn't work on an ancient compiler on some now-irrelevant version of Mac OS; but I don't have a non-obsolete Mac to test on, so I'm not touching that bit :-)

@ensiform
Copy link
Member

I don't have any Mac to test on hence paging those two.

@smcv
Copy link
Contributor Author

smcv commented Oct 29, 2016

I believe modern compilers automatically inline static functions if it helps code size or efficiency anyway, whether they're declared static inline or just static (and conversely, they might treat a static inline function as though it was static if that would help code size or efficiency). The main effect of static inline is that compilers won't typically warn about a static inline function that is never called, whereas for plain static they usually do.

eternalcodes pushed a commit to eternalcodes/EternalJK that referenced this pull request Aug 21, 2018
Consistently use "static QINLINE" for inline C code

Former-commit-id: e39dc54
eternalcodes pushed a commit to eternalcodes/EternalJK that referenced this pull request Aug 21, 2018
Consistently use "static QINLINE" for inline C code
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.

None yet

2 participants