[C++20] [Modules] We may meet problems if we import first and include second

[ChuanqiXu9]

Currently, the following can be accepted:

#include <iostream>
import foo; // assume module 'foo' contain the declarations from `<iostream>`

int main(int argc, char *argv[])
{
    std::cout << "Test\n";
    return 0;
}

but it will get rejected if we reverse the order of import and include here:

import foo; // assume module 'foo' contain the declarations from `<iostream>`
#include <iostream>

int main(int argc, char *argv[])
{
    std::cout << "Test\n";
    return 0;
}

Both the above examples should be accepted.

This is a limitation in the implementation. In the first example, the compiler will see and parse <iostream> first then the compiler will see the import. So the ODR Checking and declarations merging will happen in the deserializer. In the second example, the compiler will see the import first and the include second. So the ODR Checking and declarations merging will happen in the semantic analyzer.

So there is divergence in the implementation path. It might be understandable that why the orders matter here in the case.
(Note that "understandable" is different from "makes sense"). Given the current scale of clang, it will be pretty hard to merge the two paths together quickly and innocently. Maybe we could only fix the problems reported one by one. So the current issue report is for summarizing such issues. And it may open for a pretty long time.

Note that MSVC's documents says to include headers before import directly. (https://learn.microsoft.com/en-us/cpp/cpp/import-export-module?view=msvc-170#import)

[llvmbot]

@llvm/issue-subscribers-clang-modules

[rnikander]

In issue #61329 (closed in favor of this one), @ChuanqiXu9, you wrote:

A header shouldn't contain an import. This should be a bad manner.

I think I'm forced to put imports in headers, if I want to mix C++ modules with old style .h/.cc file pairs. And I need that mix, if I'm transitioning existing code to C++ modules.

In other words, if I write a class in the old style (.h, .cc), and it's declaration needs something from a module, then I need to import that module in the header file. (?)

[davidstone]

My view is that anything that can be in a module should be. This means that headers are mostly just for macro definitions, and sometimes those macros will need to use something that was defined in a module. I don't see anything wrong with that. To give a really basic example, I have a macro that depends on std::is_constant_evaluated and std::unreachable. It is surely better for me to import std; than to #include <some headers>, so that all of the users of my macro get as much of the benefits of modules as possible.

[ChuanqiXu9]

A trick for transitioning existing code to C++ modules may be: https://github.com/alibaba/async_simple/blob/CXX20Modules/third_party_module/asio/asio.cppm. I mean you can have a module wrapper to wrap the current headers. Then you can import the module in the .cc files. Then I suggest to replace/refactor it step by step.

[rnikander]

Is the same issue? It's not import-include order but I thought it might the same underlying issue.

CMakeLists.txt

add_executable(mod_bug2)
target_sources(mod_bug2
  PUBLIC  main.cc
  PUBLIC  FILE_SET cxx_modules TYPE CXX_MODULES FILES
  Mod1.cc Mod2.cc
)
target_link_libraries(mod_bug2 PRIVATE nlohmann_json::nlohmann_json)

Mod1.cc

module;
#include <ranges>
export module Mod1;

Mod2.cc

module;
#include <nlohmann/json.hpp>  // this ends up including <ranges>. I'm not sure what it does to make things screwy.
export module Mod2;
import Mod1;

Errors below. Something about how the json headers include of <ranges> causes it to be different?

In module 'Mod1' imported from /Users/rob/Workspace/MonoRepo/Learning/ModBug2/Mod2.cc:8:
/Users/rob/Dev/llvm-project/build/bin/../include/c++/v1/__ranges/zip_view.h:494:40: error: 'std::ranges::views::__zip::__fn::operator()' from module 'Mod1.<global>' is not present in definition of 'std::ranges::views::__zip::__fn' provided earlier
  _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Ranges&&... __rs) const
                                       ^
/Users/rob/Dev/llvm-project/build/bin/../include/c++/v1/__ranges/zip_view.h:491:40: note: declaration of 'operator()' does not match
  _LIBCPP_HIDE_FROM_ABI constexpr auto operator()() const noexcept { return empty_view<tuple<>>{}; }
                                       ^
/Users/rob/Dev/llvm-project/build/bin/../include/c++/v1/__ranges/zip_view.h:494:40: note: declaration of 'operator()' does not match
  _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Ranges&&... __rs) const

[ChuanqiXu9]

This is not the same issue. It'd better to file another issue. And if possible, it will be best and pretty helpful if you can reduce it. Currently, you can reduce it by :

1. Replace Mod1.cc and Mod2.cc with the preprocessed version.
2. Remove things that won't affect the error message.
3. Repeat 2 until we can't make it any more.

Generally it may take 1 hour. (If you don't have time to reduce it, I can only make it myself. But it may take longer time to fix the bug.) Thanks~

[rnikander]

Okay, I can't now, but I will try in coming days.

[rnikander]

I got to it faster than I thought :) #61642