Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++20][Modules] error: 'std::align_val_t' has different definitions in different modules #76638

Closed
Ivan171 opened this issue Dec 30, 2023 · 10 comments
Assignees
Labels
clang:modules C++20 modules and Clang Header Modules

Comments

@Ivan171
Copy link

Ivan171 commented Dec 30, 2023

I could not create a minimal reproducer, but I've attached the preprocessed files, which I think is enough to reproduce the issue.

Environment

Windows 10
Clang 18.0.0git (https://github.com/llvm/llvm-project 85c3953)
MSVC 2022 (17.8.1)

Reproducer

// mod2.cpp

module;

#include <utility>

export module mod2;
// mod1.cpp

module;

#include <memory>

export module mod1;

import mod2;
$ clang++.exe -std=c++20 -fno-delayed-template-parsing -fno-ms-compatibility -x c++-module mod2.cpp --precompile
$ clang++.exe -std=c++20 -fno-delayed-template-parsing -fno-ms-compatibility -x c++-module mod1.cpp -fprebuilt-module-path=.
In file included from mod1.cpp:3:
In file included from C:\Program Files (x86)\Microsoft Visual Studio 2022\VC\Tools\MSVC\14.38.33130\include\memory:10:
In file included from C:\Program Files (x86)\Microsoft Visual Studio 2022\VC\Tools\MSVC\14.38.33130\include\exception:8:
In file included from C:\Program Files (x86)\Microsoft Visual Studio 2022\VC\Tools\MSVC\14.38.33130\include\yvals.h:20:
In file included from C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt\crtdbg.h:13:
In file included from C:\Program Files (x86)\Microsoft Visual Studio 2022\VC\Tools\MSVC\14.38.33130\include\vcruntime_new_debug.h:10:
[repro.zip](https://github.com/llvm/llvm-project/files/13799238/repro.zip)
[repro.zip](https://github.com/llvm/llvm-project/files/13799246/repro.zip)

C:\Program Files (x86)\Microsoft Visual Studio 2022\VC\Tools\MSVC\14.38.33130\include\vcruntime_new.h:27:16: error: 'std::align_val_t'
      has different definitions in different modules; defined here first difference is enum with specified type 'size_t'
      (aka 'unsigned long long')
   27 |     enum class align_val_t : size_t {};
      |     ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files (x86)\Microsoft Visual Studio 2022\VC\Tools\MSVC\14.38.33130\include\vcruntime_new.h:27:16: note: but in
      'mod2.<global>' found enum with specified type 'size_t' (aka 'unsigned long long')
   27 |     enum class align_val_t : size_t {};
      |     ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
@Ivan171
Copy link
Author

Ivan171 commented Dec 30, 2023

Preprocessed files

@EugeneZelenko EugeneZelenko added clang:modules C++20 modules and Clang Header Modules and removed new issue labels Dec 30, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 30, 2023

@llvm/issue-subscribers-clang-modules

Author: Ivan171 (Ivan171)

I could not create a minimal reproducer, but I've attached the preprocessed files, which I think is enough to reproduce the issue.

Environment

Windows 10
Clang 18.0.0git (https://github.com/llvm/llvm-project 85c3953)
MSVC 2022 (17.8.1)

Reproducer

// mod2.cpp

module;

#include &lt;utility&gt;

export module mod2;
// mod1.cpp

module;

#include &lt;memory&gt;

export module mod1;

import mod2;
$ clang++.exe -std=c++20 -fno-delayed-template-parsing -fno-ms-compatibility -x c++-module mod2.cpp --precompile
$ clang++.exe -std=c++20 -fno-delayed-template-parsing -fno-ms-compatibility -x c++-module mod1.cpp -fprebuilt-module-path=.
In file included from mod1.cpp:3:
In file included from C:\Program Files (x86)\Microsoft Visual Studio 2022\VC\Tools\MSVC\14.38.33130\include\memory:10:
In file included from C:\Program Files (x86)\Microsoft Visual Studio 2022\VC\Tools\MSVC\14.38.33130\include\exception:8:
In file included from C:\Program Files (x86)\Microsoft Visual Studio 2022\VC\Tools\MSVC\14.38.33130\include\yvals.h:20:
In file included from C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt\crtdbg.h:13:
In file included from C:\Program Files (x86)\Microsoft Visual Studio 2022\VC\Tools\MSVC\14.38.33130\include\vcruntime_new_debug.h:10:
[repro.zip](https://github.com/llvm/llvm-project/files/13799238/repro.zip)
[repro.zip](https://github.com/llvm/llvm-project/files/13799246/repro.zip)

C:\Program Files (x86)\Microsoft Visual Studio 2022\VC\Tools\MSVC\14.38.33130\include\vcruntime_new.h:27:16: error: 'std::align_val_t'
      has different definitions in different modules; defined here first difference is enum with specified type 'size_t'
      (aka 'unsigned long long')
   27 |     enum class align_val_t : size_t {};
      |     ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files (x86)\Microsoft Visual Studio 2022\VC\Tools\MSVC\14.38.33130\include\vcruntime_new.h:27:16: note: but in
      'mod2.&lt;global&gt;' found enum with specified type 'size_t' (aka 'unsigned long long')
   27 |     enum class align_val_t : size_t {};
      |     ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

@koplas
Copy link

koplas commented Jan 1, 2024

Does compiling with -fms-compatibility yield the same error?

@Ivan171
Copy link
Author

Ivan171 commented Jan 2, 2024

Does compiling with -fms-compatibility yield the same error?

Yes

@ChuanqiXu9
Copy link
Member

Oh, sad. This has the same error message with #60027...

@koplas
Copy link

koplas commented Jan 2, 2024

Here is a reduced reproducer:

// mod1.cpp
module;

extern "C" {
    typedef unsigned __int64 size_t;
}


namespace std {
    enum class align_val_t : size_t {};
}


export module mod1;

import mod2;
// mod2.cpp
module;

extern "C" {
    typedef unsigned __int64 size_t;
 }


namespace std {
            using :: size_t;
}


extern "C++" {
    namespace std {
        enum class align_val_t : size_t {};
    }
}

export module mod2;

Seems similar to the reproducer provided by @davidstone: #60027 (comment)

@ChuanqiXu9 ChuanqiXu9 self-assigned this Jan 3, 2024
@ChuanqiXu9
Copy link
Member

This is slightly more tricky than I thought.

The reduced example can be accepted by using the canonical decl for the integral type for enum decl here:

if (Enum->getIntegerTypeSourceInfo())
AddQualType(Enum->getIntegerType());

However, this may introduce false-negative issues. Now the following case will be accepted:

// mod1.cpp
module;

extern "C" {
    typedef unsigned __int64 size_t;
}


namespace std {
    enum class align_val_t : size_t {};
}
export module mod1;

import mod2;
// mod2.cpp
module;

extern "C" {
    typedef unsigned __int64 size_t;
 }


namespace std {
            using :: size_t;
}


extern "C++" {
    namespace std {
        enum class align_val_t : std::size_t {};
    }
}

export module mod2;

Now the spellings of the definition of align_val_t in two different modules are different. This violates http://eel.is/c++draft/basic.def.odr#14.4:

Each such definition shall consist of the same sequence of tokens


Also my "fix" only works for the reported enum case, but I believe the similar problem can occur in other places.

@zygoloid Can you take a look about how should we proceed here?

@ChuanqiXu9
Copy link
Member

Given the reported issue has a stronger impact (we've seen multiple reports about it) and the down side of the proposed solution may only be a concern, I'd like to land the proposed solution in a few days if no more comments come in.

@Ivan171
Copy link
Author

Ivan171 commented Jan 24, 2024

My original issue is fixed, but now I've ran into the following issue #79386, which seems to have the same underlying problem.

@ChuanqiXu9
Copy link
Member

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

Yeah, you can look at #78850 and #79240

ChuanqiXu9 added a commit that referenced this issue Jan 29, 2024
Previosly we land
085eae6
to workaround the false positive ODR violations in
#76638.

However, we decided to not perform ODR checks for decls from GMF in
#79240 and we land the
corresponding change. So we should be able to remove the workaround now.

The original tests get remained.
ChuanqiXu9 added a commit to ChuanqiXu9/llvm-project that referenced this issue Feb 1, 2024
Previosly we land
llvm@085eae6
to workaround the false positive ODR violations in
llvm#76638.

However, we decided to not perform ODR checks for decls from GMF in
llvm#79240 and we land the
corresponding change. So we should be able to remove the workaround now.

The original tests get remained.
tstellar pushed a commit to ChuanqiXu9/llvm-project that referenced this issue Feb 7, 2024
Previosly we land
llvm@085eae6
to workaround the false positive ODR violations in
llvm#76638.

However, we decided to not perform ODR checks for decls from GMF in
llvm#79240 and we land the
corresponding change. So we should be able to remove the workaround now.

The original tests get remained.
tstellar pushed a commit to tstellar/llvm-project that referenced this issue Feb 14, 2024
Previosly we land
llvm@085eae6
to workaround the false positive ODR violations in
llvm#76638.

However, we decided to not perform ODR checks for decls from GMF in
llvm#79240 and we land the
corresponding change. So we should be able to remove the workaround now.

The original tests get remained.
tstellar pushed a commit to tstellar/llvm-project that referenced this issue Feb 14, 2024
Previosly we land
llvm@085eae6
to workaround the false positive ODR violations in
llvm#76638.

However, we decided to not perform ODR checks for decls from GMF in
llvm#79240 and we land the
corresponding change. So we should be able to remove the workaround now.

The original tests get remained.
tstellar pushed a commit to tstellar/llvm-project that referenced this issue Feb 14, 2024
Previosly we land
llvm@085eae6
to workaround the false positive ODR violations in
llvm#76638.

However, we decided to not perform ODR checks for decls from GMF in
llvm#79240 and we land the
corresponding change. So we should be able to remove the workaround now.

The original tests get remained.
tstellar pushed a commit to tstellar/llvm-project that referenced this issue Feb 14, 2024
Previosly we land
llvm@085eae6
to workaround the false positive ODR violations in
llvm#76638.

However, we decided to not perform ODR checks for decls from GMF in
llvm#79240 and we land the
corresponding change. So we should be able to remove the workaround now.

The original tests get remained.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:modules C++20 modules and Clang Header Modules
Projects
None yet
Development

No branches or pull requests

5 participants