FMT_ATTACH_TO_GLOBAL_MODULE does not compile with clang

[kamrann]

There appear to be two independent issues:

1. os.h:386:16: error: declaration of 'buffer_size' with internal linkage cannot be exported. constexpr implying internal linkage has an exception for module interface units, but it seems when enclosed within extern "C++" clang ceases to apply this exception. I suspect clang is probably wrong here and fmt/MSVC are correct, but not 100% sure. I filed an issue with LLVM.
2. error: declaration of 'assert_fail' in module fmt follows declaration in the global module. This one I suspect is more likely to be clang being correct and fmt/MSVC wrong to permit it, but again unsure. clang seems to want the definitions of things previously declared within extern "C++" to also be within such a block.

So it's possible there is no error here on fmt's side, but from experience, with two discrepancies between MSVC and clang, MSVC being on the correct side of both is unlikely!

[vitaut]

Thanks for reporting. It seems that the second issue has an easy workaround of adding extern "C++". If this is the case a PR to implement it would be welcome. Not sure about the first one. cc @DanielaE

[DanielaE]

Comments without studying the code or its changes in detail:

• constexpr does not imply internal linkage, at all. Please look up the related sections in the standard.
• extern "C++" within the purview of a module has the effect that all affected entities are no longer attached to the named module ('fmt' in this case), but rather to the global module. This changes the generated symbols and linker records, plus the compiler-internal entity tables. Please look up the related sections in the standard.
• the application of FMT_ATTACH_TO_GLOBAL_MODULE is meant to enable using the same definitions regardless of their origin, be it either from an #include or from an import. A simple test:

#include <fmt/*> // definitions from this TU
import fmt; // definitions made reachable from the module interface TU

must work then because of [basic.def.odr]/15

Unfortunately, Clang 18 is still getting this wrong. To its excuse, it doesn't claim conformance to the standard.

[kamrann]

It seems that the second issue has an easy workaround of adding extern "C++". If this is the case a PR to implement it would be welcome.

Happy to provide a PR if this looks like what is needed. It does appear to fix it, I'm just not familiar enough with the standard and modules to be certain that fmt is wrong here.

constexpr does not imply internal linkage, at all. Please look up the related sections in the standard

I spent quite a bit of time looking this stuff up before posting the issues. Perhaps my interpretation of the standard is off, that's why I needed some input/confirmation.

From [dcl.constexpr]:

A constexpr specifier used in an object declaration declares the object as const

And from [basic.link]:

The name of an entity that belongs to a namespace scope has internal linkage if it is the name of
  - a non-template variable of non-volatile const-qualified type, unless
    - it is declared in the purview of a module interface unit (outside the private-module-fragment, if any) or module partition

To me this says that constexpr variable definitions at namespace scope have internal linkage. And indeed outside the extern "C++" case all compilers appear to confirm this.

extern "C++" within the purview of a module has the effect that all affected entities are no longer attached to the named module ('fmt' in this case), but rather to the global module. This changes the generated symbols and linker records, plus the compiler-internal entity tables.

Yep, I'm aware it changes the attachment, but I don't follow how this relates to the question of linkage.

One option I believe would be to change the variable to be inline constexpr; I think this would safely fix the issue regardless of whether it turns out that clang is correct or not. I'll test this.

[kamrann]

PRs submitted for each change.

With these two combined I can now build fmt successfully with FMT_ATTACH_TO_GLOBAL_MODULE, however the particular usage example @DanielaE gives above does indeed fail, giving errors on clang and an ICE on MSVC.

[DanielaE]

You were speaking about constexpr at namespace scope within the purview of a module interface unit, right?
Otherwise, the whole discussion is moot.

This is from my tests in a different project where I check the coexistence of #include and import within the same TU, without compromising name lookup of the exported entities. In fact, there's more to it: the module is built using the modularized standard library, while doctest brings its own set of required standard library header #includes. Clang fails, MSVC passes:

#include <doctest.hpp>

// this must work according to [basic.def.odr]/15
// the same definitions come from *two different* TUs:
//   - this TU by means of #include <argparse.hpp>
//   - another TU that's creating module 'argparse'
//
// the definitions must *both* be attached to the global module!

#if defined(__clang__)
// the test fails to compile on Clang 18
constexpr bool included = false;
#else
constexpr bool included = true;
#include <argparse/argparse.hpp>
#endif

import argparse;

using doctest::skip;
using doctest::test_suite;

TEST_CASE("Module is usable in mixed mode" * test_suite("module") *
          skip(not included)) {
  REQUIRE(requires {
    typename argparse::nargs_pattern;
    {argparse::nargs_pattern::optional};
    {argparse::nargs_pattern::any};
    {argparse::nargs_pattern::at_least_one};
    typename argparse::default_arguments;
    {argparse::default_arguments::none};
    {argparse::default_arguments::all};
    {argparse::default_arguments::help};
    {argparse::default_arguments::version};
    {argparse::default_arguments::none & argparse::default_arguments::all};
    typename argparse::ArgumentParser;
    typename argparse::Argument;
  });
}

[kamrann]

You were speaking about constexpr at namespace scope within the purview of a module interface unit, right? Otherwise, the whole discussion is moot.

Correct.

As mentioned above, trying this with fmt (with the changes from the PRs) fails on both MSVC and clang, with endless redefinition errors. Merely

#include <fmt/format.h>
import fmt;

is sufficient to break it on both compilers. So it seems there is some way to go before this can be used fully in a way that permits diamond dependency graphs within a TU without worrying about a shared dependency being consumed as both modules and includes. Even so, it still has value now I think, because it permits linking together TUs that consumed fmt in different ways.