"static inline" in a header file is stupid, right?

[Benoit Jacob]

Hello,

Short version: "inline" alone is enough to take care of multiple
function definitions. Next time you're about to write "static inline"
in a header file, seriously consider doing "inline" instead.

Long version:

This command,

$ find mozilla-central -name '*.h' | xargs grep -n 'static inline'

finds 1851 matches here.

The "inline" keyword does two things that I know of:
1) it suggests inlining, and
2) it allows multiple definitions of a function, regardless of
whether inlining happens.

That second thing means that in most cases, defining a "static inline"
function in a header is useless, and just "inline" is what is really
wanted in most cases.

The "static" keyword in "static inline" is harmful in the situation
where a "static inline" function gets included and compiled in N
different files and does not get inlined. In this situation, one would
typically want the N copies of this function to get merged into one,
to avoid code bloat. But the "static" keyword prevents that, forcing
the linker to not merge these redundant functions.

Cheers,
Benoit

[Kyle Huey]

That's true for things at file scope, yes. My not-very-random sample of
MXR indicates that this is often used on static things on structs/classes,
which has an entirely different meaning of course.

- Kyle

[Joshua Cranmer]

On 4/3/2012 3:38 PM, Benoit Jacob wrote:
> Hello,
>
> Short version: "inline" alone is enough to take care of multiple
> function definitions. Next time you're about to write "static inline"
> in a header file, seriously consider doing "inline" instead.
>
> Long version:
>
> This command,
>
> $ find mozilla-central -name '*.h' | xargs grep -n 'static inline'
>
> finds 1851 matches here.
>
> The "inline" keyword does two things that I know of:
> 1) it suggests inlining, and
> 2) it allows multiple definitions of a function, regardless of
> whether inlining happens.
>
> That second thing means that in most cases, defining a "static inline"
> function in a header is useless, and just "inline" is what is really
> wanted in most cases.

No, no it is not. If you don't have the static keyword, you get multiple
definitions of the function, and the linker will say "you have multiple
definitions of it." A simple test case:

jcranmer@xochiquetzal /tmp $ cat test.h
inline int foo() { return 1; }
jcranmer@xochiquetzal /tmp $ cat test1.c
#include "test.h"
int bar() {
return foo() + 1;
}
jcranmer@xochiquetzal /tmp $ cat test2.c
#include "test.h"
int main() {
return foo() + 1;
}
jcranmer@xochiquetzal /tmp $ gcc test1.c test2.c
/usr/bin/ld: error: /tmp/ccJNRLKW.o: multiple definition of 'foo'
/usr/bin/ld: /tmp/ccjlFFST.o: previous definition here
collect2: ld returned 1 exit status

If I change the definition of foo to "static inline", the linker error
goes away.

[Jeff Muizelaar]

On 2012-04-03, at 4:44 PM, Benoit Jacob wrote:

> Right, I meant file scope of course.
>
> This slightly different command should give only file scope matches,
> assuming sane indentation and given that it's not allowed to repeat
> the in-class static keyword in out-of-class definitions:
>
> $ find mozilla-central -name '*.h' | xargs grep -n '^static inline'
>
> That's still 1160 matches.

'static inline' is need for C to avoid multiple definitions.

-Jeff

[Benoit Jacob]

2012/4/3 Joshua Cranmer <Pidg...@verizon.net>:

> On 4/3/2012 3:38 PM, Benoit Jacob wrote:
>>
>> Hello,
>>
>> Short version: "inline" alone is enough to take care of multiple
>> function definitions. Next time you're about to write "static inline"
>> in a header file, seriously consider doing "inline" instead.
>>
>> Long version:
>>
>> This command,
>>
>>   $ find mozilla-central -name '*.h' | xargs grep -n 'static inline'
>>
>> finds 1851 matches here.
>>
>> The "inline" keyword does two things that I know of:
>>  1) it suggests inlining, and
>>  2) it allows multiple definitions of a function, regardless of
>> whether inlining happens.
>>
>> That second thing means that in most cases, defining a "static inline"
>> function in a header is useless, and just "inline" is what is really
>> wanted in most cases.
>
> No, no it is not. If you don't have the static keyword, you get multiple
> definitions of the function, and the linker will say "you have multiple
> definitions of it." A simple test case:

You example is C. I had C++ in mind, as our codebase is predominantly
C++. I should have made that clear. My point is that we have lots of
file-scope static inlines in C++ headers.

Benoit

>
> jcranmer@xochiquetzal /tmp $ cat test.h
> inline int foo() { return 1; }
> jcranmer@xochiquetzal /tmp $ cat test1.c
> #include "test.h"
> int bar() {
>        return foo() + 1;
> }
> jcranmer@xochiquetzal /tmp $ cat test2.c
> #include "test.h"
> int main() {
>        return foo() + 1;
> }
> jcranmer@xochiquetzal /tmp $ gcc test1.c test2.c
> /usr/bin/ld: error: /tmp/ccJNRLKW.o: multiple definition of 'foo'
> /usr/bin/ld: /tmp/ccjlFFST.o: previous definition here
> collect2: ld returned 1 exit status
>
> If I change the definition of foo to "static inline", the linker error goes
> away.

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

[Joshua Cranmer]

On 4/3/2012 3:57 PM, Benoit Jacob wrote:
> 2012/4/3 Joshua Cranmer<Pidg...@verizon.net>:
>> On 4/3/2012 3:38 PM, Benoit Jacob wrote:
>>> Hello,
>>>
>>> Short version: "inline" alone is enough to take care of multiple
>>> function definitions. Next time you're about to write "static inline"
>>> in a header file, seriously consider doing "inline" instead.
>>>
>>> Long version:
>>>
>>> This command,
>>>
>>> $ find mozilla-central -name '*.h' | xargs grep -n 'static inline'
>>>
>>> finds 1851 matches here.
>>>
>>> The "inline" keyword does two things that I know of:
>>> 1) it suggests inlining, and
>>> 2) it allows multiple definitions of a function, regardless of
>>> whether inlining happens.
>>>
>>> That second thing means that in most cases, defining a "static inline"
>>> function in a header is useless, and just "inline" is what is really
>>> wanted in most cases.
>> No, no it is not. If you don't have the static keyword, you get multiple
>> definitions of the function, and the linker will say "you have multiple
>> definitions of it." A simple test case:
> You example is C. I had C++ in mind, as our codebase is predominantly
> C++. I should have made that clear. My point is that we have lots of
> file-scope static inlines in C++ headers.

Not quite. The cases I see are the following:
Chromium code (third party)
Snappy (not sure)
NSPR (C)
Skia (I think this is third party)
Cairo (C third party)
QCMS (third party)
crash reporter (third party)
libtremor (third party)
libvorbis (third party)
JSAPI (the headers are designed to be used by C applications)

less than 1/4 of the cases are actually things we can choose.

[Ehren Metcalfe]

On Tue, Apr 3, 2012 at 4:55 PM, Jeff Muizelaar <jmuiz...@mozilla.com> wrote:
>
>
> 'static inline' is need for C to avoid multiple definitions.
>

In C99 you can write extern void foo(); in the implementation file and
then inline void foo { /* stuff */ } in the header. That way only one
standalone definition will be emitted.

(ccing the list this time)

[Rafael Ávila de Espíndola]

> In C99 you can write extern void foo(); in the implementation file and
> then inline void foo { /* stuff */ } in the header. That way only one
> standalone definition will be emitted.
>

We build with c89 with gcc. With clang on OS X we should be using c99.
No idea about msvc.

Cheers,
Rafael

[Rafael Ávila de Espíndola]

On 12-04-03 4:38 PM, Benoit Jacob wrote:
> Hello,
>
> Short version: "inline" alone is enough to take care of multiple
> function definitions. Next time you're about to write "static inline"
> in a header file, seriously consider doing "inline" instead.
>

Depends on the language. I find it easier to just think about the llvm
linkage :-). This testcase gives the mapping:

// c99: available_externally
// gnu89: default
// c++: linkonce_odr
inline void foo1(void) {
}

// c99: default
// gnu89: available_externally
// c++: linkonce_odr
extern inline void foo2(void) {
}

// c99: internal
// gnu89: internal
// c++: internal
static inline void foo3(void) {
}

void bar(void) {
foo1();
foo2();
foo3();
}

We are stuck with having gnu89 inline for C on linux until we upgrade
glibc. "static inline" is probably used because it produces the same
results on all cases.

> Cheers,
> Benoit

Cheers,
Rafael

[Justin Lebar]

MSVC does not have a C99 mode. You have to compile C as C++ if you
want something resembling C99.

[Mike Hommey]

On Wed, Apr 04, 2012 at 09:55:40AM -0400, Justin Lebar wrote:
> MSVC does not have a C99 mode. You have to compile C as C++ if you
> want something resembling C99.

The C code that uses static inline most certainly doesn't compile as C++

Mike

[Mike Hommey]

Because of headers?

Mike

[Rafael Ávila de Espíndola]

>> We are stuck with having gnu89 inline for C on linux until we
>> upgrade glibc.
>
> Because of headers?

Correct. Building with -std=c99 causes duplicated symbols in XUL.

> Mike

Cheers,
Rafael

[perthom...@gmail.com]

Static Inline makes perfectly sense in C++

Consider the following use case: (in principle I think its irrelevant if the function is static or not)
You have a library/DLL for which you in some case wants to link in the library whereas for other occasions you just want to use a small set of functions, and for this you use the inlined versions to avoid the hassle of having to ship also the DLL.

The decision on whether to use the inlined version or the the DLL (the code should be the same) is taken by the compiler. If the "inline" keyword is not used then the implementation will be taken form the DLL wg\hen generating the object files, even if the implementation is present in the header file.

Now, when linking, you will get an error of the form "XYZ allready defined in....." Because the function is implemented multiple places, firstly, in the DLL, the in every place where the header file containing the function is included.

Using "inline" will only be necessary in front of the implementation, however in my opinion it is a good idea to add inline also in form of the function definition in order to show intent, even if the keyword has no effect.

[cpp...@gmail.com]

Hi! I come from the future, it is not in C++17.

[Botond Ballo]

>
> Hi! I come from the future, it is not in C++17.
>

Can you elaborate on what "it" is?

Thanks,
Botond

[cpp...@gmail.com]

Stupid: It is not stupid, it is a feature.

[Botond Ballo]

On Fri, Jul 31, 2020 at 9:00 PM <cpp...@gmail.com> wrote:

> Stupid: It is not stupid, it is a feature.
>

Ah, you mean `static inline` has uses in C++17 that it didn't have in older
versions?

I would appreciate an example (or a link to post etc.) so we can understand
this better.

Thanks,
Botond

[cpp...@gmail.com]

Sure, before 17 you had to initialize static variable in cpp "encapsulated" code. So maybe you want to expose static data, specifically the way you initialized this static data, through your headers cause you want your API users to have access to read them and figure out things.

Now, you can do that in your header file outside the class brackets. But if you included the header, in more than one user cpp file, the linker will complain with duplicated declaration of your static objects.

Now with inline static, you can initialize in the class declaration. Whenever may be the case you want your API users (if an API / Lirbary is what you are writing) to understand how's your static var is initialized.

A link to show what I mean: https://www.tutorialspoint.com/how-do-inline-variables-work-in-cplusplus-cplusplus17#:~:text=The%20static%20value%20is%3A%2010,the%20class%20using%20inline%20variables.

[Simon Giesecke]

Hi,

I think your original message was meant to be about `static inline`
function definitions in a header file. To my understanding, nothing has
changed with C++11/14/17 about that. However, I think it wasn't accurate
before that, since there is a difference between `static inline` member
functions and `static inline` free functions. The former make sense, and in
case of templates may even be required to be in a header file. However, I
think that this is mostly unrelated to the use of the `inline` keyword
here, but applies to any `static` free function declared in a header file:
If it is inline (explicitly or implicitly), it would emit one copy per
compilation unit, and if it were not inline, it wouldn't be callable
outside the compilation unit that contains its definition. Note however,
that whether a function is `inline` in the sense of the C++ standard is not
related to whether the compiler (or, with LTO, the linker) will inline it
during code generation.

But there are now `static inline` member variables as you point out, which
also make sense to be used in a header file.

The simple grep as suggested in the original message does not distinguish
these cases. (OTOH, the order of the keywords could be reversed, since
`static inline` and `inline static` are equivalent, and the grep as such
does not match the latter.)

We already have several static member variables defined in header files,
which are implicitly inline because they are `constexpr`, most of them
within media/webrtc/trunk/webrtc as of now.

Simon