Nasty gotcha in template code generation

[Vir Campestris]

So I have a header file with some templated code in it. This is a
simplified version

template <typename T> void func() {
std::cout << "in " << __PRETTY_FUNCTION__ << std::endl;
method(T());
std::cout << "leaving " << __PRETTY_FUNCTION__ << std::endl;
}

You'll notice it calls "method".

I also have a couple of cpp files, which use the template

#include <iostream>

template <typename T> void method(const T & param) {
std::cout << "In method a " << __PRETTY_FUNCTION__ << std::endl;
}

#include "template.h"

void a() {
func<int>();
}

they're identical, except in the second one the function at the bottom
is called b, and the message in method says "In method b".

From another file I call a and b.

The messages indicate that BOTH calls to a and b call the same method()
function.

This is because the instantiated template function from both a.cpp and
b.cpp have the same signature, so the linker throws one of them away.

It took me a while to track it down in the real code.

Andy

[Richard Damon]

Doesn't seem that surprising to me. You named two different functions
with the same name so you should expect an issue. Now, being templates,
the system is more apt to not detect the problem and report the issue.

There is a similar issue with just plain global variables, which gcc
just recently changed the behavior from ignoring to reporting and a lot
of people are being caught unawares that it was a problem.

[Öö Tiib]

On Friday, 18 December 2020 at 13:13:36 UTC+2, Vir Campestris wrote:

> This is because the instantiated template function from both a.cpp and
> b.cpp have the same signature, so the linker throws one of them away.
>
> It took me a while to track it down in the real code.

Yes some violations of ODR are required to be detected by standard but
others are that your program is ill formed but no diagnostic is required.

Both the patterns "undefined behavior" and “no diagnostic is required”
are massively represented in C++ standard so it will take years to
learn to avoid those in your own code and even more years to notice
in code of others. But committee releases more every 3 years so the
process of learning C++ should be continuous.

[Vir Campestris]

On 18/12/2020 13:05, Richard Damon wrote:
> Doesn't seem that surprising to me. You named two different functions
> with the same name so you should expect an issue. Now, being templates,
> the system is more apt to not detect the problem and report the issue.

If I named two different ordinary functions the same I'd expect the
linker to object.

With template instantiations it optimises one away - based purely on the
signature, not the content. I suppose it would be too hard to check the
content.

> There is a similar issue with just plain global variables, which gcc
> just recently changed the behavior from ignoring to reporting and a lot
> of people are being caught unawares that it was a problem.

I don't think I follow you.

Andy

[Richard Damon]

GCC has long allowed multiple translation units to define variables by
the same name as globals (and not require that all but one be declared
as extern to make them just declarations and not definitions). It did
not check that they ware of the same type, but just put all of them at
the same memory location.

Defining globals by the same name in different translation units is
undefined behavior (whether a function of a variable), but does not
require a diagnostic.

GCC has had an option that would flag this case as an error, but the
default has been until a very recent version, to allow this to happen
(for variables) by default. The recent version has changed the default
to generate an error, and some programming communities are having fits
due to this, as long working programs are now not building.

The function issue brought up is very much of the same sort of thing.
Normal functions will generate duplicate definition errors if you define
the same function in two translation units. Due to code generation
issues with templates, template functions are normally compiled so that
multiple copies get merged into a single version, from one of the
translation units.

[Keith Thompson]

Richard Damon <Ric...@Damon-Family.org> writes:
> On 12/18/20 4:43 PM, Vir Campestris wrote:
>> On 18/12/2020 13:05, Richard Damon wrote:
>>> Doesn't seem that surprising to me. You named two different functions
>>> with the same name so you should expect an issue. Now, being templates,
>>> the system is more apt to not detect the problem and report the issue.
>>
>> If I named two different ordinary functions the same I'd expect the
>> linker to object.
>>
>> With template instantiations it optimises one away - based purely on the
>> signature, not the content. I suppose it would be too hard to check the
>> content.
>>
>>> There is a similar issue with just plain global variables, which gcc
>>> just recently changed the behavior from ignoring to reporting and a lot
>>> of people are being caught unawares that it was a problem.
>>
>> I don't think I follow you.
>

> GCC has long allowed multiple translation units to define variables by
> the same name as globals (and not require that all but one be declared
> as extern to make them just declarations and not definitions). It did
> not check that they ware of the same type, but just put all of them at
> the same memory location.
>
> Defining globals by the same name in different translation units is
> undefined behavior (whether a function of a variable), but does not
> require a diagnostic.
>
> GCC has had an option that would flag this case as an error, but the
> default has been until a very recent version, to allow this to happen
> (for variables) by default. The recent version has changed the default
> to generate an error, and some programming communities are having fits
> due to this, as long working programs are now not building.
>
> The function issue brought up is very much of the same sort of thing.
> Normal functions will generate duplicate definition errors if you define
> the same function in two translation units. Due to code generation
> issues with templates, template functions are normally compiled so that
> multiple copies get merged into a single version, from one of the
> translation units.

It looks like the change happened between gcc 9.3.0 (2019) and 10.1.0 (2020).

--
Keith Thompson (The_Other_Keith) Keith.S.T...@gmail.com
Working, but not speaking, for Philips Healthcare
void Void(void) { Void(); } /* The recursive call of the void */

[Ian Collins]

On 19/12/2020 12:18, Richard Damon wrote:
> On 12/18/20 4:43 PM, Vir Campestris wrote:
>> On 18/12/2020 13:05, Richard Damon wrote:
>>> Doesn't seem that surprising to me. You named two different functions
>>> with the same name so you should expect an issue. Now, being templates,
>>> the system is more apt to not detect the problem and report the issue.
>>
>> If I named two different ordinary functions the same I'd expect the
>> linker to object.
>>
>> With template instantiations it optimises one away - based purely on the
>> signature, not the content. I suppose it would be too hard to check the
>> content.
>>
>>> There is a similar issue with just plain global variables, which gcc
>>> just recently changed the behavior from ignoring to reporting and a lot
>>> of people are being caught unawares that it was a problem.
>>
>> I don't think I follow you.
>>
>> Andy
>
>
> GCC has long allowed multiple translation units to define variables by
> the same name as globals (and not require that all but one be declared
> as extern to make them just declarations and not definitions). It did
> not check that they ware of the same type, but just put all of them at
> the same memory location.

This has been the cause of many a strange error and a good argument
against forward declaration (rather than header inclusion) for structs.
If the struct layout changes, say by adding a new member a the top, and
every TU that uses it isn't recompiled nasal demons have been known to
appear.

--
Ian.

[Öö Tiib]

On Friday, 18 December 2020 at 23:43:30 UTC+2, Vir Campestris wrote:
> On 18/12/2020 13:05, Richard Damon wrote:
> > Doesn't seem that surprising to me. You named two different functions
> > with the same name so you should expect an issue. Now, being templates,
> > the system is more apt to not detect the problem and report the issue.
> If I named two different ordinary functions the same I'd expect the
> linker to object.

It is groundless expectation as [basic.def.odr] "Every program shall contain
exactly one definition of every non-inline function or variable that is odr-used
in that program outside of a discarded statement, no diagnostic required.
...
An inline function or variable shall be defined in every translation
unit in which it is odr-used outside of a discarded statement."

> With template instantiations it optimises one away - based purely on the
> signature, not the content. I suppose it would be too hard to check the
> content.

Compiler vendors do very lot of things that are hard and not required,
but programmers are still expected to pay attention to what is their
fault and what compilers are not required to diagnose.

[Vir Campestris]

Just FYI the reason this one bit me is that I was cleaning up some code
being built for several platforms. What it used to do (and does again;
I've given up at least for now) is have a base class with one derived
class compiled in for each platform. We've got a company policy of
moving to run-time dynamic behaviour where in cases like this all the
classes exist, and the correct derived one is instantiated at runtime.

The derived class files contain definitions of operator>> to read some
config data in - but not for the class itself, but for one of the member
data types.

That's OK, except that under the hood the way this is called is to use
lexical_cast (not Boost's, but something similar) to convert a config
line into the data type. And internally that calls the operator>> that
is in scope at the time the lexical_cast function was called.

It wasn't obvious that this is what was going wrong until I put trace in
place, and ran it on one of the platforms that was hitting the problem.
The serialisation failed, and it then used default data rather than
complaining.

Andy

[Öö Tiib]

Odd desire. Platform to what we compile is known compile time so
making it run-time dynamic feels absurd waste.

> The derived class files contain definitions of operator>> to read some
> config data in - but not for the class itself, but for one of the member
> data types.

Sometimes dynamic polymorphism is used only to improve
testability. Through polymorphic interface it is easy to inject
dummies, fakes, simulators and mocks for testing. Virtual calls
are fine anywhere but in rare most performance-critical
places.

But that does not mean that all such alternatives may be really
compiled into whole product. That is error if done.
Also the interface of dynamic polymorphism being more elegant
does not mean that static polymorphism should not be used.

> That's OK, except that under the hood the way this is called is to use
> lexical_cast (not Boost's, but something similar) to convert a config
> line into the data type. And internally that calls the operator>> that
> is in scope at the time the lexical_cast function was called.
>
> It wasn't obvious that this is what was going wrong until I put trace in
> place, and ran it on one of the platforms that was hitting the problem.
> The serialisation failed, and it then used default data rather than
> complaining.

Yes added (unneeded) complexity and tricks can always result with
added chances of bugs and other confusion. What is decided compile
time can be also diagnosed compile time. Compile time diagnosed
issues are cheapest to fix. Compiling for all supported platforms
takes time but for that there is continuous integration.