@llvm/issue-subscribers-clang-modules

Given the experience in clang, we should emit warnings for one version (clang17) and emit the hard error in the next version to not break too many things.

Note to self: when this ships, CMake needs to enable the transitive logic done for MSVC in Clang as well.

hi folks I came across this while making a demo for hidden partition use (somehow i missed the discussion that lead to this change - and I cannot see a paper or change to the std. that allows it)....

After this fix the following code ...

// RUN: rm -rf %t
// RUN: mkdir %t
// RUN: split-file %s %t

// Build the public interface for M
// RUN: %clang_cc1 -std=c++20 %t/M-Part1.cpp -emit-module-interface -o %t/M-Part1.pcm
// RUN: %clang_cc1 -std=c++20 %t/M.cpp -emit-module-interface -o %t/M.pcm -fmodule-file=M:Part1=%t/M-Part1.pcm
// RUN: %clang_cc1 -std=c++20 %t/M.pcm -emit-obj -o %t/M.o

// RUN: %clang_cc1 -std=c++20 %t/M-Part1-impl.cpp -emit-obj -o %t/M-Part1.o -fmodule-file=M:Part1=%t/M-Part1.pcm
// RUN: %clang_cc1 -std=c++20 %t/M-impl.cpp -emit-obj -o %t/M-impl.o -fmodule-file=M=%t/M.pcm

// Build a program using M
// RUN: %clang_cc1 -std=c++20 %t/Use-M.cpp -emit-obj -o %t/Use.o -fmodule-file=M=%t/M.pcm
// RUN: %clang -std=c++20 %t/Use.o %t/M.o %t/M-impl.o %t/M-Part1.o -o %t/Use

// Test that it works.
// RUN: %t/Use

// M and M:Part1 contain the public interfaces for M.
//--- M.cpp
// 
export module M;

export import :Part1;

export int in_module (int);

//--- M-Part1.cpp
//
export module M:Part1;

export int in_part1 (int);
int in_part1_mod (int);

//--- M-Part1-impl.cpp
//
module M:Part1;

int in_part1 (int x) { return 40 + x; }
int in_part1_mod (int x) { return 40 + x; }

//--- M-impl.cpp
// 
module M;

int in_module (int x) { return in_part1 (x) + in_part1_mod (x); }

//--- Use-M.cpp
//

import M;

int main ()
{
  int a = in_module (2) + in_part1 (2);
  return a == 168 ? 0 : 1;
}

produces:

...partition-warning.cpp.tmp/M-impl.cpp:2:1: warning: it is deprecated to read module 'M:Part1' implicitly; it is going to be removed in clang 18; consider to specify the dependencies explicitly [-Wread-modules-implicitly]
    2 | module M;
      | ^
1 warning generated.
....partition-warning.cpp.tmp/Use-M.cpp:3:1: warning: it is deprecated to read module 'M:Part1' implicitly; it is going to be removed in clang 18; consider to specify the dependencies explicitly [-Wread-modules-implicitly]
    3 | import M;
      | ^
1 warning generated.

I do not believe that this is correct per the standard;
https://eel.is/c++draft/module#unit-3

The clarifying note is clear, that the user of a module should not need to know about its partitions; so it is not correct to ask the user to do this...

import M;
import :Part1; // because this is ill-formed 

So .. either we need to explain how we're going to fix the underlying problem or we need to remove this before 17 branches

.... does it also fire for export import B; ?

I think there are in different levels. https://eel.is/c++draft/module#unit-3 talks about the source level and the issue is about the tools (build systems) level. I think the build systems knows every partitions naturally.

hmm .. I do not follow that logic (yet)
... surely the user must be able to write this code:

import M;

int main ()
{
  int a = in_module (2) + in_part1 (2);
  return a == 168 ? 0 : 1;
}

without knowing that M has some partitions?

Yes. But the build systems knows about the partitions, then the build systems can add these flags. They are on the different abstract levels.

Hmm, OK so now the meaning of the diagnostic becomes clear - perhaps we might add a hint about how to fix the issue - if it was not obvious to me - then probably someone else will also have troubles.

as an aside:

I must confess IMO it would be very unfortunate to make simple use of modules (like my example above) conditional on having a build system (while it is completely accepted that is required for larger projects, of course), that seems likely to be a barrier to initial adoption.

So, IMO, we need to make this work for the simple kinds of example being discussed in https://discourse.llvm.org/t/rfc-modules-build-daemon-build-system-agnostic-support-for-explicitly-built-modules/71524/47. to allow simple projects to work with just the compiler.

(let's continue the discussion there and not side-track this bug)

In my opinion, such simple use cases can be solved by -fprebuilt-module-path=. And this is what I saw for existing users.