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

static operator in headers and modules can't be merged well #61807

Open
rnikander opened this issue Mar 29, 2023 · 6 comments
Open

static operator in headers and modules can't be merged well #61807

rnikander opened this issue Mar 29, 2023 · 6 comments
Labels
clang:modules C++20 modules and Clang Header Modules

Comments

@rnikander
Copy link

rnikander commented Mar 29, 2023

Started in this issue here: #61493. Per request I'm filing another one with headers stripped down.

The types in Apple's <simd/simd.h> header do not seem to work well with C++ modules. Below you will see that there are operators defined, like operator *, but they become unavailable when you import the module.

I replaced a function implementation with throw 1 to avoid pulling on the thread and including a lot more stuff.

header.h

// This file is #include <simd/simd.h> stripped down to the just enough to reproduce the issue.

typedef __attribute__((__ext_vector_type__(2))) float simd_float2;

typedef struct { simd_float2 columns[2]; } simd_float2x2;

extern "C" {
static  simd_float2 simd_mul( simd_float2x2 __x,  simd_float2 __y) { throw 1; }
}

namespace simd {

typedef ::simd_float2 float2;
  
struct float2x2 : ::simd_float2x2 {
    float2x2() { columns[0] = 0; columns[1] = 0; }
    float2x2(float2 v) { columns[0] = (float2){v.x,0}; columns[1] = (float2){0,v.y}; }
    float2x2(float2 c0, float2 c1) { columns[0] = c0; columns[1] = c1; }
    float2x2(::simd_float2x2 m) : ::simd_float2x2(m) { }
};

static float2 operator*(const float2x2 x, const float2 y) { return ::simd_mul(x, y); }
}

ModWithVectors_min.cc

module;
#include "header.h"
export module ModWithVectors;

main_min.cc

#include "header.h"

import ModWithVectors; // Comment this out and it compiles.

int main(int argc, char *argv[])
{
    //  ⎡ 1 2 ⎤ * ⎡1⎤ = ⎡5⎤
    //  ⎣ 3 4 ⎦   ⎣2⎦   ⎣1⎦
    simd::float2 col1{1, 3};
    simd::float2 col2{2, 4};    
    simd::float2x2 mat(col1, col2);
    simd::float2 vec{1, 2};
    auto v = mat * vec;
    return 0;
}

CMakeLists.txt

add_executable(mod_bug3_min)
target_sources(mod_bug3_min
  PUBLIC  main_min.cc
  PUBLIC FILE_SET cxx_modules TYPE CXX_MODULES FILES ModWithVectors_min.cc
)

The error:

/ModBug3/main_min.cc:14:18: error: cannot convert between vector and non-scalar values ('simd::float2x2' and 'simd::float2' (aka 'simd_float2'))
    auto v = mat * vec;
             ~~~ ^ ~~~
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 29, 2023

@llvm/issue-subscribers-clang-modules

@ChuanqiXu9
Copy link
Member

It works if we remove the "static" identifier from operator*. So the problem is that we didn't handle the entities with internal linkage well.

And it looks complex than it shows. For example, in main_min.cc, should the deserializer to merge the static operator*? If yes, it is odd since the entities with internal linkage shouldn't be visible outside the modules. If no, I can image there will be more ODR-violation problems.

Maybe is it a good idea to emit an error directly here? I am not sure.

@ChuanqiXu9 ChuanqiXu9 changed the title Importing a C++ module causes simd matrix operators to disappear static operator in headers and modules can't be merged well Apr 6, 2023
@iains
Copy link
Contributor

iains commented Apr 6, 2023

what is the content of "header_here.h" (a copy of "header.h")?

[in C++20 modules] The content of the GMF is not visible to an importer of the module - however it is reachable (so that any decls in the GMF used in the exported named module purview are available).

Actually, in the specific example (once we have completed GMF unused decl elision) the ModWithVectors would essentially be empty since none of the GMF content has any uses in the module purview.

so is this an interaction between 'clang modules' and 'C++20 modules' rules?

@ChuanqiXu9
Copy link
Member

what is the content of "header_here.h" (a copy of "header.h")?

I think so.

so is this an interaction between 'clang modules' and 'C++20 modules' rules?

I don't think so. It is easy to add a use for the static operator in ModWithVectors_min.cc. The key point for the issue should be how should we handle entities with internal linkage. But I think the issue will be much easier to discuss after we implement P1815 actually.

@rnikander
Copy link
Author

what is the content of "header_here.h" (a copy of "header.h")?

Yes. Typo on my part. I just edited the original to fix that.

@masx200
Copy link

masx200 commented May 17, 2023

[ 37%]: compiling.module.test leetcode_treenode_cpp.freeTreeNode
"C:\\Program Files\\LLVM\\bin\\clang" -c -x c++-module -fmodule-output=build\.gens\leetcode-treenode-cpp\windows\x64\test\rules\modules\cache\ec4a91a6\leetcode_treenode_cpp.freeTreeNode.pcm -fmodule-file=leetcode_treenode_cpp.TreeNode=build\.gens\leetcode-treenode-cpp\windows\x64\test\rules\modules\cache\ec4a91a6\leetcode_treenode_cpp.TreeNode.pcm -Qunused-arguments -m64 -std=c++20 -D__TEST__ -fexceptions -fcxx-exceptions -isystem C:\Users\Administrator\Documents\vcpkg-master\installed\x64-windows-static\include -fmodules-cache-path=build\.gens\leetcode-treenode-cpp\windows\x64\test\rules\modules\cache -fmodule-file=leetcode_treenode_cpp.TreeNode=build\.gens\leetcode-treenode-cpp\windows\x64\test\rules\modules\cache\ec4a91a6\leetcode_treenode_cpp.TreeNode.pcm -o build\.objs\leetcode-treenode-cpp\windows\x64\test\freeTreeNode.ixx.obj freeTreeNode.ixx
error: @programdir\modules\private\async\runjobs.lua:256: @programdir\rules\c++\modules\modules_support\clang.lua:270: @programdir\modules\core\tools\gcc.lua:721: In file included from traversalTreeNode.ixx:2:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\include\vector:11:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\include\xmemory:14:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\include\new:11:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\include\exception:12:
C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\include\type_traits:2186:19: error: 'std::hash' has different definitions in different modules; first difference is defined here found method '_Do_hash' with no body
    static size_t _Do_hash(const _Kty& _Keyval) noexcept {
    ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\include\type_traits:2186:19: note: but in 'leetcode_treenode_cpp.TreeNode.<global>' found method '_Do_hash' with body
    static size_t _Do_hash(const _Kty& _Keyval) noexcept {
    ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from traversalTreeNode.ixx:2:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\include\vector:11:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\include\xmemory:14:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\include\new:11:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\include\exception:12:
C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\include\type_traits:2167:23: error: 'std::_Conditionally_enabled_hash' has different definitions in different modules; first difference is defined here found method 'operator()' with no body
    _NODISCARD size_t operator()(const _Kty& _Keyval) const
               ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\include\type_traits:2167:23: note: but in 'leetcode_treenode_cpp.TreeNode.<global>' found method 'operator()' with body
    _NODISCARD size_t operator()(const _Kty& _Keyval) const
               ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.

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

6 participants