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++ modules example fails on LLVM 15.0.7 for 64-bit Windows - delayed template parsing is not supported/broken with C++ modules #61068

Closed
csimon-bambecksystems opened this issue Feb 28, 2023 · 33 comments
Assignees
Labels
clang:modules C++20 modules and Clang Header Modules platform:windows

Comments

@csimon-bambecksystems
Copy link

csimon-bambecksystems commented Feb 28, 2023

The documentation here provides the following Hello-World style example:

// M.cppm
export module M;
export import :interface_part;
import :impl_part;
export void Hello();

// interface_part.cppm
export module M:interface_part;
export void World();

// impl_part.cppm
module;
#include <iostream>
#include <string>
module M:impl_part;
import :interface_part;

std::string W = "World.";
void World() {
  std::cout << W << std::endl;
}

// Impl.cpp
module;
#include <iostream>
module M;
void Hello() {
  std::cout << "Hello ";
}

// User.cpp
import M;
int main() {
  Hello();
  World();
  return 0;
}

Precompiling the module

clang++ -std=c++20 interface_part.cppm --precompile -o M-interface_part.pcm
clang++ -std=c++20 impl_part.cppm --precompile -fprebuilt-module-path=. -o M-impl_part.pcm
clang++ -std=c++20 M.cppm --precompile -fprebuilt-module-path=. -o M.pcm
clang++ -std=c++20 Impl.cpp -fmodule-file=M.pcm -c -o Impl.o

Compiling the user

clang++ -std=c++20 User.cpp -fprebuilt-module-path=. -c -o User.o

Compiling the module and linking it together

clang++ -std=c++20 M-interface_part.pcm -c -o M-interface_part.o
clang++ -std=c++20 M-impl_part.pcm -c -o M-impl_part.o
clang++ -std=c++20 M.pcm -c -o M.o
clang++ User.o M-interface_part.o  M-impl_part.o M.o Impl.o -o a.out

I tried this exact example on LLVM 15.0.7 for 64-bit Windows. The files are included. The standard library and linker/backend in my environment is MSVC 17.5.1. I have changed the command lines slightly for Windows. The result is a strange failure at Impl.cpp:

Impl.cpp:5:3: error: use of undeclared identifier 'std'
  std::cout << "Hello ";
  ^
1 error generated.

Note, I can compile, link and execute a regular "Hello World" (without C++20 modules) in my LLVM environment just fine, it is only behaving this way when it imports modules and includes system headers in global module fragments of those imported modules.
test2.zip

@EugeneZelenko EugeneZelenko added clang:modules C++20 modules and Clang Header Modules platform:windows and removed new issue labels Feb 28, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 28, 2023

@llvm/issue-subscribers-clang-modules

@EugeneZelenko
Copy link
Contributor

Could you please try 16 or main branch? https://godbolt.org should be helpful.

@csimon-bambecksystems
Copy link
Author

@EugeneZelenko, I will try. Thank you.

@csimon-bambecksystems
Copy link
Author

@EugeneZelenko, I am not succeeding at entering this example into godbolt because it has to import the BMI from module M, but there is no way to save a BMI in godbolt AFAIK. I could paste a screenshot of "fatal error: module M not found" but that would not be useful. There are no LLVM releases past 15.0.7 for Windows. If I cloned the trunk of LLVM from GIT I would have to build it on MSVC, and are the instructions for that even public? I did upload my complete source (same as the example in the docs plus the nine Windows command lines in a powershell-script for compiling/linking the example) so anyone with a Windows PC with LLVM installed could confirm it.

@aaronmondal
Copy link
Contributor

Hmm I'm pretty sure that this should work with Clang 16 or upstream.

It may be an unsatisfactory workaround, but with Docker and WSL things tend to be easier to get running. E.g. the a Gentoo and Arch baseimages have more recent versions available in their package managers.

@csimon-bambecksystems
Copy link
Author

@aaronmondal In fact it does work on my Clang v15.0.7 on 64-bit Windows, as long as I change the backend to a gcc cross-compiler, using Clang to produce MIPS instructions and pass that on to my runs-on-Windows/targets-MIPS version of gcc. My workflow with LLVM is ultimately for the cross-compiler but I am also hoping to test my code on Windows. So, in short, I need Clang to work on both targets but currently it is working on only one of them. As to the version of Clang, the example is from the Clang 15.0.0 docs (see the hyperlink in my first post). It is presented there as the Hello World of modules. Plus, the Clang 15 release notes say "Implemented PR1103R3: Merging Modules," which, if I understand, is the paper that merges modules into the C++20 standard, and includes module partitions. The Clang Status page says the same thing. I am sure Clang would not announce that in the 15 release notes and have 15 on the status page and include a sample project in the 15 docs if it were so seriously broken in 15. TL;DR: Works on other configurations of Clang 15, problem is just in the MSVC backend.

@csimon-bambecksystems
Copy link
Author

Update: I installed llvm 16.0.0 today from the official release. Example still does not work. However, error message is different:

In file included from Impl.cpp:2:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\iostream:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\istream:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\ostream:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\ios:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\xlocnum:16:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\streambuf:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\xiosbase:12:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\system_error:14:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\stdexcept:12:
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\xstring:3133:10: error:
      'std::basic_string' has different definitions in different modules; first difference is defined here found method
      '_Memcpy_val_from' with no body
    void _Memcpy_val_from(const basic_string& _Right) noexcept {
    ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\xstring:3133:10: note:
      but in 'M:impl_part.<global>' found method '_Memcpy_val_from' with body
    void _Memcpy_val_from(const basic_string& _Right) noexcept {
    ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

(Interesting note: I don't know if you can see this in github, but on my terminal in the fixed-width font, the first line of tildes extends from the v in void to the t in noexcept, not including the opening brace of the function body. The second line of tildes includes the opening brace.)

@TheRustifyer
Copy link

Update: I installed llvm 16.0.0 today from the official release. Example still does not work. However, error message is different:

In file included from Impl.cpp:2:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\iostream:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\istream:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\ostream:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\ios:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\xlocnum:16:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\streambuf:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\xiosbase:12:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\system_error:14:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\stdexcept:12:
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\xstring:3133:10: error:
      'std::basic_string' has different definitions in different modules; first difference is defined here found method
      '_Memcpy_val_from' with no body
    void _Memcpy_val_from(const basic_string& _Right) noexcept {
    ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\xstring:3133:10: note:
      but in 'M:impl_part.<global>' found method '_Memcpy_val_from' with body
    void _Memcpy_val_from(const basic_string& _Right) noexcept {
    ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

(Interesting note: I don't know if you can see this in github, but on my terminal in the fixed-width font, the first line of tildes extends from the v in void to the t in noexcept, not including the opening brace of the function body. The second line of tildes includes the opening brace.)

Things usually works better under Windows if you use Mingw and links against stdlibc++.

I typically preferred to link against libc++, but in Windows isn't working with flags like -fimplict-modules, -fimplicit-module-maps

@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.

@csimon-bambecksystems
Copy link
Author

Upgraded to LLVM 16.0.5 today. Confirming that this issue still exists.

06/12/2023 11:21:58 test2> clang++ --version
clang version 16.0.5
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Program Files\LLVM\bin
06/12/2023 11:22:05 test2> clang++ -std=c++20 Impl.cpp -fmodule-file=M='M.pcm' -c -o Impl.obj
In file included from Impl.cpp:2:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\iostream:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\istream:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\ostream:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\ios:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\xlocnum:16:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\streambuf:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\xiosbase:12:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\system_error:14:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\stdexcept:12:
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\xstring:3133:10: error:
      'std::basic_string' has different definitions in different modules; first difference is defined here found method
      '_Memcpy_val_from' with no body
    void _Memcpy_val_from(const basic_string& _Right) noexcept {
    ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.35.32215\include\xstring:3133:10: note:
      but in 'M:impl_part.<global>' found method '_Memcpy_val_from' with body
    void _Memcpy_val_from(const basic_string& _Right) noexcept {
    ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@csimon-bambecksystems
Copy link
Author

The issue still exists in LLVM 17.0.1, and the error message has actually gotten a little more verbose since when I last tried it, in 16.0.5...

C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.37.32822\include\xstring:2993:10: error:
      'std::basic_string' has different definitions in different modules; first difference is defined here found method
      '_Memcpy_val_from' with no body
 2993 |     void _Memcpy_val_from(const basic_string& _Right) noexcept {
      |     ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.37.32822\include\xstring:2993:10: note:
      but in 'M:impl_part.<global>' found method '_Memcpy_val_from' with body
 2993 |     void _Memcpy_val_from(const basic_string& _Right) noexcept {
      |     ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2994 |         _STL_INTERNAL_CHECK(_Can_memcpy_val);
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2995 |         const auto _My_data_mem =
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~
 2996 |             reinterpret_cast<unsigned char*>(_STD addressof(_Mypair._Myval2)) + _Memcpy_val_offset;
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2997 |         const auto _Right_data_mem =
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2998 |             reinterpret_cast<const unsigned char*>(_STD addressof(_Right._Mypair._Myval2)) + _Memcpy_val_offset;
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2999 |         _CSTD memcpy(_My_data_mem, _Right_data_mem, _Memcpy_val_size);
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 3000 |     }
      |     ~

@davidstone
Copy link
Contributor

The issues reported here since the first post look like a duplicate of #60027

@csimon-bambecksystems
Copy link
Author

@davidstone I agree with you. I would be fine with combining the issues.

@EugeneZelenko EugeneZelenko added the duplicate Resolved as duplicate label Oct 9, 2023
@EugeneZelenko EugeneZelenko closed this as not planned Won't fix, can't repro, duplicate, stale Oct 9, 2023
@ChuanqiXu9 ChuanqiXu9 reopened this Oct 11, 2023
@EugeneZelenko EugeneZelenko removed the duplicate Resolved as duplicate label Oct 11, 2023
@csimon-bambecksystems
Copy link
Author

By request... my preprocessor output! All 7 megabytes of it.
Hello Modules.zip

@koplas
Copy link

koplas commented Oct 12, 2023

Does the issue still occurs if you write your impl_part.cppm like:

// impl_part.cppm
module;
#include <iostream>
#include <string>
module M:impl_part;
import :interface_part;

// Comment out the string assignment
// std::string W = "World.";
void World() {
  std::cout << "World." << std::endl;
}

@csimon-bambecksystems
Copy link
Author

@koplas Yes, if I don't instantiate the std::string, then it doesn't complain about the string, now it only complains about std::ostream...

In file included from Impl.cpp:2:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.37.32822\include\iostream:11:
In file included from C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.37.32822\include\istream:11:
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.37.32822\include\ostream:97:37: error:
      'std::basic_ostream::sentry' has different definitions in different modules; first difference is defined here
      found constructor with no body
   97 |         explicit __CLR_OR_THIS_CALL sentry(basic_ostream& _Ostr) : _Sentry_base(_Ostr) {
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.37.32822\include\ostream:97:37: note:
      but in 'M:impl_part.<global>' found constructor with body
   97 |         explicit __CLR_OR_THIS_CALL sentry(basic_ostream& _Ostr) : _Sentry_base(_Ostr) {
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   98 |             if (!_Ostr.good()) {
      |             ~~~~~~~~~~~~~~~~~~~~
   99 |                 _Ok = false;
      |                 ~~~~~~~~~~~~
  100 |                 return;
      |                 ~~~~~~~
  101 |             }
      |             ~
  102 |
  103 |             const auto _Tied = _Ostr.tie();
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  104 |             if (!_Tied || _Tied == &_Ostr) {
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  105 |                 _Ok = true;
      |                 ~~~~~~~~~~~
  106 |                 return;
      |                 ~~~~~~~
  107 |             }
      |             ~
  108 |
  109 |             _Tied->flush();
      |             ~~~~~~~~~~~~~~~
  110 |             _Ok = _Ostr.good(); // store test only after flushing tie
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  111 |         }
      |         ~

@koplas
Copy link

koplas commented Oct 12, 2023

It seems like functions that are not referenced or used are stripped of their body.

For example, the method _Memcpy_val_from is used in impl_part.cppm in this chain:
constexpr basic_string& operator=(const basic_string& _Right) -> _Copy_assign_val_from_small -> _Memcpy_val_from

And std::basic_ostream::sentry is used because std::endl forces a flush() and the flush method references sentry.

As a workaround, you can add a dummy std::string with an assignment and a dummy function that uses std::endl to Impl.cpp to avoid the stripping of the function body.
I don't know how to create a minimal reproducer that triggers this condition and can't explain why similar things don't happen with libc++.

@csimon-bambecksystems
Copy link
Author

You are on to something, but unfortunately I am doing something wrong with that workaround, it isn't working around for me...

@koplas
Copy link

koplas commented Oct 13, 2023

@csimon-bambecksystems Can you try the original reproducer with -fno-delayed-template-parsing? Sorry, I can't get clang to work with Windows. This flag is enabled by default on Windows and has to be disabled to generate the function body.

For more info:

Note this also fails on Linux with -fdelayed-template-parsing, but with redefinition errors.

@csimon-bambecksystems
Copy link
Author

csimon-bambecksystems commented Oct 13, 2023

@koplas omg... that worked!

Note this also fails on Linux with -fdelayed-template-parsing

So that was it, the example doesn't work with delayed template parsing (which means, I guess, clang C++ standard modules does not work yet with delayed templated parsing?). Which means it didn't work for naive Windows users who have delayed template parsing turned on by default.

EDIT: If -fdelayed-template-parsing is just to parse the Windows Active Template Library by making clang as broken as MSVC is, and is not a real C++ feature that developers would want to use, well then! C++ standard modules with delayed template parsing doesn't need to be a goal. Thinking about trying it on Linux made me confused, but now I read up for myself about what it is, and I understand. (TL;DR: Except for this one case only, for the purpose of divining the problem of a Windows user, don't use -fdelayed-template-parsing on Linux.)

@koplas
Copy link

koplas commented Oct 13, 2023

@ChuanqiXu9 Maybe add a note to the docs that delayed template parsing is not supported/broken with C++ modules?

@ChuanqiXu9 ChuanqiXu9 self-assigned this Oct 16, 2023
@ChuanqiXu9
Copy link
Member

@ChuanqiXu9 Maybe add a note to the docs that delayed template parsing is not supported/broken with C++ modules?

Nice catch! Will done!

@ChuanqiXu9 ChuanqiXu9 changed the title C++ modules example fails on LLVM 15.0.7 for 64-bit Windows C++ modules example fails on LLVM 15.0.7 for 64-bit Windows - delayed template parsing is not supported/broken with C++ modules Oct 16, 2023
ChuanqiXu9 added a commit that referenced this issue Oct 16, 2023
…s not working with modules

Catched in #61068.

Add this to the document to avoid further misunderstandings.
@ChuanqiXu9
Copy link
Member

And it is pretty helpful to provide a minimal reproducer to solve the underlying issue.

@koplas
Copy link

koplas commented Oct 16, 2023

@csimon-bambecksystems Can you share the output of -Xclang -ast-dump -fsyntax-only with the failing reproducer? This would simplify the creation of a minimal reproducer. I always get the first error you reported: Impl.cpp:5:3: error: use of undeclared identifier 'std' while compiling with clang 17 on Windows.

@Arthapz
Copy link

Arthapz commented Oct 16, 2023

maybe #64810 and #65027
are linked to this issue ?

@csimon-bambecksystems
Copy link
Author

@koplas Output of: clang++ -std=c++20 -fprebuilt-module-path='.' -fmodule-file=M='M.pcm' Impl.cpp -Xclang -ast-dump -fsyntax-only > Impl-ast-dump.txt
Imp-ast-dump.zip

@koplas
Copy link

koplas commented Oct 16, 2023

Here is a reproducer that also runs on Linux: delayed-template-parsing.zip
Please notify me if there are any issues running this.
Note: This could be reduced further but it should be enough to highlight the issue.

@koplas
Copy link

koplas commented Oct 16, 2023

maybe #64810 and #65027 are linked to this issue ?

Does running with -fno-delayed-template-parsing fixes these issues?

@Arthapz
Copy link

Arthapz commented Oct 17, 2023

maybe #64810 and #65027 are linked to this issue ?

Does running with -fno-delayed-template-parsing fixes these issues?

image
yes ! (i included minimal reproducers in their respective issues)

@koplas
Copy link

koplas commented Oct 17, 2023

maybe #64810 and #65027 are linked to this issue ?

Does running with -fno-delayed-template-parsing fixes these issues?

image yes ! (i included minimal reproducers in their respective issues)

Yes, both are linked to this issue and both report an error with a template function.

@ChuanqiXu9
Copy link
Member

Here is a reproducer that also runs on Linux: delayed-template-parsing.zip Please notify me if there are any issues running this. Note: This could be reduced further but it should be enough to highlight the issue.

Oh, it is good enough that it doesn't contain any third-party headers (including STLs). One suggestion may be to put such small reproducer directly instead of putting a .zip. I admit I got scared when seeing the .zip.

@ChuanqiXu9
Copy link
Member

I sent #69431

ChuanqiXu9 added a commit to ChuanqiXu9/llvm-project that referenced this issue Oct 23, 2023
… default on windows after c++20

There are already 3 issues about the broken state of
-fdelayed-template-parsing and C++20 modules:
- llvm#61068
- llvm#64810
- llvm#65027

The problem is more complex than I thought. I am not sure how to fix it
properly now. Given the complexities and -fdelayed-template-parsing is
actually an extension to support old MS codes, I think it may make sense
to not enable the -fdelayed-template-parsing option by default with
C++20 modules to give more user friendly experience. Users who still want
-fdelayed-template-parsing can specify it explicitly.

Given the discussion in llvm#69551,
we decide to not enable -fdelayed-template-parsing by default on windows
after c++20
ChuanqiXu9 added a commit that referenced this issue Oct 23, 2023
… default on windows with C++20 (#69431)

There are already 3 issues about the broken state of
-fdelayed-template-parsing and C++20 modules:
- #61068
- #64810
- #65027

The problem is more complex than I thought. I am not sure how to fix it
properly now. Given the complexities and -fdelayed-template-parsing is
actually an extension to support old MS codes, I think it may make sense
to not enable the -fdelayed-template-parsing option by default with
C++20 modules to give more user friendly experience. Users who still
want -fdelayed-template-parsing can specify it explicitly.

Also according to https://learn.microsoft.com/en-us/cpp/build/reference/permissive-standards-conformance?view=msvc-170, MSVC actually defaults to -fno-delayed-template-parsing (/Zc:twoPhase-
with MSVC CLI) if using C++20. So we match the behavior with MSVC here to
not enable -fdelayed-template-parsing by default after C++20.
@ChuanqiXu9
Copy link
Member

Originally, I thought #69431 is simply a workaround. But according to the later discussions, I found it may be a proper solution since now we treat -fdelayed-template-parsing as a source of bugs. Then I think we can close the issue now. Feel free to reopen this if any one has different opinions.

ChuanqiXu9 added a commit that referenced this issue Jan 22, 2024
Changed Things:
- Mentioning we need to specify BMIs for indirectly dependent BMIs too.
- Remove the note for `delayed-template-parsing` since
  #61068 got closed.
- Add a note for #78850 since
  we've seen it for a lot of times.
- Add a note for #78173 since
  we've seen it for a lot of times.
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 platform:windows
Projects
None yet
Development

No branches or pull requests

10 participants