non-member static variables defined in header

[Andrew Falanga]

I came across this while doing a code review within the team I work:

someheader.h

#ifndef _SOMEHEADER_H_
#define _SOMEHEADER_H_


namespace stuff {
static const unsigned int var1 = 0x01;
static const unsigned int var2 = 0x02;

// .....
}

#endif

This caught my attention. I didn't think this should be defined in this way and was curious why multiple definition errors hadn't been triggered by the compiler. So, I duplicated this in a small tester program with a single header shared by two source files. I too didn't get multiple definitions. I inspected the binary with nm and there are 2 defined var1 and var2 variables:

0000000000601264 D _ZN3foo4var1E
0000000000601265 D _ZN3foo4var2E
0000000000400b10 r _ZN3fooL4var1E
0000000000400b11 r _ZN3fooL4var2E


I tried some experimentation and I didn't receive multiple definition warnings until I'd removed both the keywords "static" and "const". So, why is this ok? What part of the language spec explains this? (I haven't been able to come across it yet. I have the C++ draft N3242 from Nov. 11, 2011.)

Thanks,

[Paavo Helde]

Andrew Falanga <af30...@gmail.com> wrote in
news:5e07d883-5cae-4f4f...@googlegroups.com:

> I came across this while doing a code review within the team I work:
>

> #ifndef _SOMEHEADER_H_
> #define _SOMEHEADER_H_

Leading underscore followed by a capital letter is reserved for the
implementation, common code should not use such names. This might give
you something to nag about in the code review.

>
>
> namespace stuff {
> static const unsigned int var1 = 0x01;
> static const unsigned int var2 = 0x02;
>

'static' is superfluous here, in C++ 'const' enforces internal linkage by
itself as well here.

3.5/3 [basic.link]:

A name having namespace scope has internal linkage if it is the name of
[...] a variable that is explicitly declared const or constexpr and
neither explicitly declared extern nor previously declared to have
external linkage [...]

[red floyd]

As Paavo said, either static or const in this case provides internal
linkage, so they are not visible outside the translation unit.

The namespace is a red herring, as is the fact that it's in a header.

Consider the C-ish code:

file1.c:

// global namespace
static int x = 0;


file 2:

// global namespace
static int x = 8;

These are perfectly legal as well, because they have internal linkage
and are in different TU's.

[Andrew Falanga]

I see. Thanks.

[Richard]

[Please do not mail me a copy of your followup]

red floyd <no....@its.invalid> spake the secret code
<m4086r$9fv$1...@dont-email.me> thusly:

>As Paavo said, either static or const in this case provides internal
>linkage, so they are not visible outside the translation unit.

Agreed for static. Not so sure about const, but ISTR that there is
some default linkage implications for const.

>The namespace is a red herring, as is the fact that it's in a header.

Here's how I look at it: constants declared in a header are meant to
be referenced from multiple translation units and the constant should
follow ODR. When you declare things static in the header, you end up
with one definition per translation unit and depending on how often
this header is included, you can end up with tons of data bloat.

By declaring constants 'extern const' in a header and defining them in
a single translation unit, I was able to shave a couple megabytes off
the size of an application.

The advice being violated here is that your header contains
DEFINITIONS and not DECLARATIONS.
--
"The Direct3D Graphics Pipeline" free book <http://tinyurl.com/d3d-pipeline>
The Computer Graphics Museum <http://computergraphicsmuseum.org>
The Terminals Wiki <http://terminals.classiccmp.org>
Legalize Adulthood! (my blog) <http://legalizeadulthood.wordpress.com>

[Öö Tiib]

On Thursday, 13 November 2014 01:56:32 UTC+2, Richard wrote:
> [Please do not mail me a copy of your followup]
>
> red floyd <no....@its.invalid> spake the secret code
> <m4086r$9fv$1...@dont-email.me> thusly:
>
> >As Paavo said, either static or const in this case
> >provides internal linkage, so they are not visible
> >outside the translation unit.
>
> Agreed for static. Not so sure about const, but ISTR
> that there is some default linkage implications for const.

Yes, Paavo did quote the wording of standard even.

> >The namespace is a red herring, as is the fact that it's
> >in a header.
>
> Here's how I look at it: constants declared in a header
> are meant to be referenced from multiple translation
> units and the constant should follow ODR.

If that is your policy then you need some extra code
analyzer tool that warns about 'const' or 'constexpr'
without explicit 'extern' in header. C++ does not
check if things are in header or not in header (headers
are entirely business of language-unaware preprocessor)
and all the constants have internal linkage by default.

> When you declare things static in the header, you end
> up with one definition per translation unit and
> depending on how often this header is included, you
> can end up with tons of data bloat.
>
> By declaring constants 'extern const' in a header
> and defining them in a single translation unit, I was
> able to shave a couple megabytes off the size of an
> application.
>
> The advice being violated here is that your header
> contains DEFINITIONS and not DECLARATIONS.

That can not be adviced in modern C++ because
increasing amount of things are templates and increasing amount of code is header-only. Linker has bit more work
to achieve actual ODR with inlines and templates but we
should not care about work-seconds of linker if we can
save work-hours of developers.

[Paavo Helde]

legaliz...@mail.xmission.com (Richard) wrote in news:m40s34$mq3$1
@news.xmission.com:

> Here's how I look at it: constants declared in a header are meant to
> be referenced from multiple translation units and the constant should
> follow ODR.

The constants are meant to be "used", not "referenced". You don't need to
keep around a reference or pointer to a const value as it cannot change
(legally).

> When you declare things static in the header, you end up
> with one definition per translation unit and depending on how often
> this header is included, you can end up with tons of data bloat.

The C++ rules for const namespace scope variables allow to optimize them
completely away in most cases where they are used, not to speak about the
TU-s where they are not used. Also the linker could presumably sometimes
merge different definitions into one.

I guess this works well for scalar types used by value, like int.
Declaring them extern and defining in a single TU would mean a
pessimization, as it would be then harder to inline the constant value
directly in the generated code.

For compound types this becomes more problematic, as these tend to be
used by references (e.g. const std::string&) and then it is probably
harder for the compiler+linker to optimize or merge them.

OTOH, declaring them extern does not really solve the problem either,
because they typically require dynamic initialization, so declaring them
extern could easily lead to "static initialization order fiasco".

Cheers
Paavo