Explicitly instantiate function return values that are template classes

[scovich]

Following the ideas discussed in #402, search the set of referenced types for templates that are function return values and emit them all as fields of a dummy struct. This silences compiler warnings (clang/gcc) and errors (MSVC) about potential ABI compatibility issues caused by returning template classes by value from extern "C" functions.

This behavior is implemented as a new pass (similar to the existing instantiate_monomorphs pass) that is controlled by a new export config, instantiate_return_value_monomorphs. The name of the dummy struct defaults to __cbindgen_return_value_monomorphs but can be overridden by a second config.

NOTE: We take the dummy struct approach because explicit instantiation in a header file violates the One Definition Rule (ODR) if multiple compilation units of a project include the header, leading to undefined behavior that may or may not include a linker error about multiple definitions. For example, according to https://en.cppreference.com/w/cpp/language/class_template:

An explicit instantiation definition forces instantiation of the class, struct, or union they refer to. It may appear in the program anywhere after the template definition, and for a given argument-list, is only allowed to appear once in the entire program, no diagnostic required.

The dummy struct performs implicit instantiation of the template classes, which as a special case does not violate the ODR (otherwise every header-defined class would violate ODR).

NOTE: The PR is large because of changes to tests and expected test output. Without that, the diff is much smaller:

10 files changed, 221 insertions(+), 19 deletions(-)

Fixes #402

[scovich]

No longer needed -- All tests that used to trigger this warning have been updated to use the new feature.

[scovich]

There's no way to compile C++ code that uses (opaque) Box<T>. At least, not without additional hacks that would trigger undefined behavior (such as manually providing a definition for the class).

This new feature ignores opaque items, which causes the test to fail compilation because of -Wreturn-type-c-linkage -Werror. If I change the feature to emit opaque items, compilation fails because the type lacks a definition.

I don't believe this test was specifically trying to test Box handling in the first place (and probably not even C++), so I updated it to use a transparent struct that will have the same behavior as Box in C code.

Or, we could just add the .skip_warning_as_error suffix to this test, if that's preferred.

[scovich]

This change wasn't strictly necessary, but it sure made debugging broken tests easier

[scovich]

@emilio -- would it be possible to get a review of this PR?

[emilio]

Thanks, I think this is worth fixing, but mind clarifying on why you went for a wrapper struct rather than just explicit instantiation?

That doesn't hurt I guess, but it is a bit weird when you can just instantiate it. You could even output it at the function level without an extra global pass (though that'd be a bit noisy perhaps? Seems fine if you track the paths you've declared).

[emilio]

FWIW I think this is kind of a workaround... But yeah it's not pretty. I don't think you need a struct or so tho, you can just explicitly instantiate them, right?

See here for example.

[scovich]

Sorry for the delay responding... misrouted github notification email...

From the PR description:

We take the dummy struct approach because explicit instantiation changes semantics of C++ templates and can cause linker errors if multiple compilation units do it -- not good for a general header file.

The approach linked above would violate the One Definition Rule (ODR), if included by multiple different compilation units of the same project. For example, according to https://en.cppreference.com/w/cpp/language/class_template:

An explicit instantiation definition forces instantiation of the class, struct, or union they refer to. It may appear in the program anywhere after the template definition, and for a given argument-list, is only allowed to appear once in the entire program, no diagnostic required.

The "no diagnostic required" part is annoying -- the compiler/linker isn't required to say anything, the resulting binary just has undefined behavior. See e.g.
https://stackoverflow.com/questions/45120323/why-c-linker-is-silent-about-odr-violation
https://stackoverflow.com/questions/21534435/separate-compilation-and-template-explicit-instantiation

C++11 added support for "explicit instantiation declarations" by prepending the extern keyword, but that just pushes the problem somewhere else:

An explicit instantiation declaration (an extern template) skips implicit instantiation step: the code that would otherwise cause an implicit instantiation instead uses the explicit instantiation definition provided elsewhere (resulting in link errors if no such instantiation exists).

By "using" a specific template instantiation inside a struct definition, the compiler implicitly instantiates the template instead, which makes its definition available without violating the ODR -- a special case that would otherwise cause header file class definitions to violate ODR.

[scovich]

(also updated the PR description with some of this info, since it wasn't clear before)