[C++20] [Modules] False Positive ODR violation diagnostic due to using inconsistent qualified but the same type

[ashley-hawkins]

Clang versions I tested with, from https://apt.llvm.org/ (taken from --version):

• Debian clang version 18.0.0 (++20240119100743+cd05ade13a66-1~exp1~20240119220916.1830) cd05ade
• Debian clang version 17.0.6 (5)

Current clang version on compiler explorer: clang version 18.0.0git (https://github.com/llvm/llvm-project.git 4684507)

I'm still not sure whether this is a problem with libstdc++ or a problem with clang so I apologise if this is the wrong place to report, but I get this error when including just <chrono> in one module partition and <memory> in another, here is an example on Compiler Explorer: https://godbolt.org/z/Wvv1h5aYa

module.cc

module;
#include <chrono>
export module repro;
export import :part;

part.cc

module;
#include <memory>
export module repro:part;

In this example I am getting the error 'std::align' has different definitions in different modules

full compiler output

In module 'repro:part' imported from /app/module.cc:4:
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/14.0.1/../../../../include/c++/14.0.1/bits/align.h:61:1: error: 'std::align' has different definitions in different modules; definition in module 'repro:part.<global>' first difference is function body
   60 | inline void*
      | ~~~~~~~~~~~~
   61 | align(size_t __align, size_t __size, void*& __ptr, size_t& __space) noexcept
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   62 | {
      | ~
   63 |   if (__space < __size)
      |   ~~~~~~~~~~~~~~~~~~~~~
   64 |     return nullptr;
      |     ~~~~~~~~~~~~~~~
   65 |   const auto __intptr = reinterpret_cast<uintptr_t>(__ptr);
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   66 |   const auto __aligned = (__intptr - 1u + __align) & -__align;
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   67 |   const auto __diff = __aligned - __intptr;
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   68 |   if (__diff > (__space - __size))
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   69 |     return nullptr;
      |     ~~~~~~~~~~~~~~~
   70 |   else
      |   ~~~~
   71 |     {
      |     ~
   72 |       __space -= __diff;
      |       ~~~~~~~~~~~~~~~~~~
   73 |       return __ptr = reinterpret_cast<void*>(__aligned);
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   74 |     }
      |     ~
   75 | }
      | ~
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/14.0.1/../../../../include/c++/14.0.1/bits/align.h:61:1: note: but in '' found a different body
   60 | inline void*
      | ~~~~~~~~~~~~
   61 | align(size_t __align, size_t __size, void*& __ptr, size_t& __space) noexcept
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   62 | {
      | ~
   63 |   if (__space < __size)
      |   ~~~~~~~~~~~~~~~~~~~~~
   64 |     return nullptr;
      |     ~~~~~~~~~~~~~~~
   65 |   const auto __intptr = reinterpret_cast<uintptr_t>(__ptr);
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   66 |   const auto __aligned = (__intptr - 1u + __align) & -__align;
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   67 |   const auto __diff = __aligned - __intptr;
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   68 |   if (__diff > (__space - __size))
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   69 |     return nullptr;
      |     ~~~~~~~~~~~~~~~
   70 |   else
      |   ~~~~
   71 |     {
      |     ~
   72 |       __space -= __diff;
      |       ~~~~~~~~~~~~~~~~~~
   73 |       return __ptr = reinterpret_cast<void*>(__aligned);
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   74 |     }
      |     ~
   75 | }
      | ~
1 error generated.

I looked at the preprocessed source and saw that the problem causing the function definitions to be considered different was one of them was getting uintptr_t from the global namespace, the other was getting uintptr_t from a using in the std namespace.

I could reproduce the same situation without those included headers to demonstrate the problem:

module.cc

module;
typedef long T;
namespace ns {
    using ::T;
}
namespace ns {
inline void fun() {
    (void)(T)0;
}
}
export module repro;
export import :part;

part.cc

module;
typedef long T;
namespace ns {
inline void fun() {
    (void)(T)0;
}
}
export module repro:part;

CMakeLists.txt

cmake_minimum_required(VERSION 3.28)
project(repro)

set(CMAKE_CXX_STANDARD 20)

add_library(repro)
target_sources(repro PUBLIC FILE_SET CXX_MODULES FILES module.cc part.cc)

And here is the above on compiler explorer: https://godbolt.org/z/Ke6eoGnan and the compiler output from that compiler explorer example:

full compiler output

In file included from /app/module.cc:12:
part.cc:4:13: error: 'ns::fun' has different definitions in different modules; definition in module 'repro:part.<global>' first difference is function body
    4 | inline void fun() {
      | ~~~~~~~~~~~~^~~~~~~
    5 |     (void)(T)0;
      |     ~~~~~~~~~~~
    6 | }
      | ~
module.cc:7:13: note: but in '' found a different body
    7 | inline void fun() {
      | ~~~~~~~~~~~~^~~~~~~
    8 |     (void)(T)0;
      |     ~~~~~~~~~~~
    9 | }
      | ~
1 error generated.

[llvmbot]

@llvm/issue-subscribers-clang-modules

Author: None (ashley-hawkins)

Clang versions I tested with, from <https://apt.llvm.org/> (taken from `--version`): - `Debian clang version 18.0.0 (++20240119100743+cd05ade13a66-1~exp1~20240119220916.1830)` cd05ade - `Debian clang version 17.0.6 (5)`

Current clang version on compiler explorer: clang version 18.0.0git (https://github.com/llvm/llvm-project.git 4684507)

I'm still not sure whether this is a problem with libstdc++ or a problem with clang so I apologise if this is the wrong place to report, but I get this error when including just <chrono> in one module partition and <memory> in another, here is an example on Compiler Explorer: https://godbolt.org/z/Wvv1h5aYa
<details>
<summary>module.cc</summary>

module;
#include <chrono>
export module repro;
export import :part;

</details>
<details>
<summary>part.cc</summary>

module;
#include <memory>
export module repro:part;

</details>

In this example I am getting the error 'std::align' has different definitions in different modules
<details>
<summary>full compiler output</summary>

In module 'repro:part' imported from /app/module.cc:4:
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/14.0.1/../../../../include/c++/14.0.1/bits/align.h:61:1: error: 'std::align' has different definitions in different modules; definition in module 'repro:part.<global>' first difference is function body
   60 | inline void*
      | ~~~~~~~~~~~~
   61 | align(size_t __align, size_t __size, void*& __ptr, size_t& __space) noexcept
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   62 | {
      | ~
   63 |   if (__space < __size)
      |   ~~~~~~~~~~~~~~~~~~~~~
   64 |     return nullptr;
      |     ~~~~~~~~~~~~~~~
   65 |   const auto __intptr = reinterpret_cast<uintptr_t>(__ptr);
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   66 |   const auto __aligned = (__intptr - 1u + __align) & -__align;
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   67 |   const auto __diff = __aligned - __intptr;
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   68 |   if (__diff > (__space - __size))
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   69 |     return nullptr;
      |     ~~~~~~~~~~~~~~~
   70 |   else
      |   ~~~~
   71 |     {
      |     ~
   72 |       __space -= __diff;
      |       ~~~~~~~~~~~~~~~~~~
   73 |       return __ptr = reinterpret_cast<void*>(__aligned);
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   74 |     }
      |     ~
   75 | }
      | ~
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/14.0.1/../../../../include/c++/14.0.1/bits/align.h:61:1: note: but in '' found a different body
   60 | inline void*
      | ~~~~~~~~~~~~
   61 | align(size_t __align, size_t __size, void*& __ptr, size_t& __space) noexcept
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   62 | {
      | ~
   63 |   if (__space < __size)
      |   ~~~~~~~~~~~~~~~~~~~~~
   64 |     return nullptr;
      |     ~~~~~~~~~~~~~~~
   65 |   const auto __intptr = reinterpret_cast<uintptr_t>(__ptr);
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   66 |   const auto __aligned = (__intptr - 1u + __align) & -__align;
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   67 |   const auto __diff = __aligned - __intptr;
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   68 |   if (__diff > (__space - __size))
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   69 |     return nullptr;
      |     ~~~~~~~~~~~~~~~
   70 |   else
      |   ~~~~
   71 |     {
      |     ~
   72 |       __space -= __diff;
      |       ~~~~~~~~~~~~~~~~~~
   73 |       return __ptr = reinterpret_cast<void*>(__aligned);
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   74 |     }
      |     ~
   75 | }
      | ~
1 error generated.

</details>

I looked at the preprocessed source and saw that the problem causing the function definitions to be considered different was one of them was getting uintptr_t from the global namespace, the other was getting uintptr_t from a using in the std namespace.

I could reproduce the same situation without those included headers to demonstrate the problem:
<details>
<summary>module.cc</summary>

module;
typedef long T;
namespace ns {
    using ::T;
}
namespace ns {
inline void fun() {
    (void)(T)0;
}
}
export module repro;
export import :part;

</details>
<details>
<summary>part.cc</summary>

module;
typedef long T;
namespace ns {
inline void fun() {
    (void)(T)0;
}
}
export module repro:part;

</details>
<details>
<summary>CMakeLists.txt</summary>

cmake_minimum_required(VERSION 3.28)
project(repro)

set(CMAKE_CXX_STANDARD 20)

add_library(repro)
target_sources(repro PUBLIC FILE_SET CXX_MODULES FILES module.cc part.cc)

</details>

And here is the above on compiler explorer: https://godbolt.org/z/Ke6eoGnan and the compiler output from that compiler explorer example:
<details>
<summary>full compiler output</summary>

In file included from /app/module.cc:12:
part.cc:4:13: error: 'ns::fun' has different definitions in different modules; definition in module 'repro:part.<global>' first difference is function body
    4 | inline void fun() {
      | ~~~~~~~~~~~~^~~~~~~
    5 |     (void)(T)0;
      |     ~~~~~~~~~~~
    6 | }
      | ~
module.cc:7:13: note: but in '' found a different body
    7 | inline void fun() {
      | ~~~~~~~~~~~~^~~~~~~
    8 |     (void)(T)0;
      |     ~~~~~~~~~~~
    9 | }
      | ~
1 error generated.

</details>

[dwblaikie]

Hmm, seems like the same as #76638, which claims to be fixed - but this repro shows the failure on godbolt at ToT... so, not sure.

[ChuanqiXu9]

Hmm, seems like the same as #76638, which claims to be fixed - but this repro shows the failure on godbolt at ToT... so, not sure.

#76638 is a simple workaround for the enum type only. And this issue has the same fundamental reason with #76638 but its direct issue is not the same with #76638. I commented this in #76638. I don't have an idea about how to proceed now : (

[dwblaikie]

First thing would be to establish whether the example is conforming, I guess?

If the alias had a different name it'd obviously be non-conforming/ODR violation, right? (because definitions must have the same sequence of tokens)

The (however it's phrased in the spec) "lookups must find the same names" - if that applies to aliases (like you must find the same alias) - then the code is non-conforming, I think? If it applies to the underlying entity (making the alias transparent) then perhaps the code is conforming and we need to canonicalize earlier in some way.

[ChuanqiXu9]

First thing would be to establish whether the example is conforming, I guess?

If the alias had a different name it'd obviously be non-conforming/ODR violation, right? (because definitions must have the same sequence of tokens)

Yes.

The (however it's phrased in the spec) "lookups must find the same names" - if that applies to aliases (like you must find the same alias) - then the code is non-conforming, I think? If it applies to the underlying entity (making the alias transparent) then perhaps the code is conforming and we need to canonicalize earlier in some way.

It should be the later case according to the comment of https://reviews.llvm.org/D156210#4540980

[ChuanqiXu9]

The reproducer can be workaround by #79240. However, I suspect the same issue exist with the declarations in the module purview. But given there is almost no legacy code in module purview, it won't be so hurting any more.