[C++20] [Modules] Implement AllAdditionalTUReachable

[ChuanqiXu]

Summary

[module.reach]p2 gives vendors freedom to decide whether or not additional TU is reachable:

All translation units that are necessarily reachable are reachable. Additional translation units on which the point within the program has an interface dependency may be considered reachable, but it is unspecified which are and under what circumstances.

And @rsmith suggested:

The default should probably be that we enforce reachability as strictly as we can, for portability and to help users enforce an "import what you use" hygiene rule, but in any case, we should give users the choice.

[ChuanqiXu]

Rebasing.

[ChuanqiXu]

The direction is approved in https://reviews.llvm.org/D113545. @MaskRay might you help to review the style?

[ChuanqiXu]

@iains @rsmith @MaskRay gentle ping~

[iains]

Hi @ChuanqiXu,
I have no comment on the technical content of the patch (it looks reasonable to me).

However, I wonder if we should be supplying this option at all because in:
https://eel.is/c++draft/module#reach-2
note2 says "[Note 2: It is advisable to avoid depending on the reachability of any additional translation units in programs intending to be portable. — end note]"

So by providing the option, we are effectively giving users an easy mechanism to make non-portable programs (and I'm fairly sure that would mean that they would not have the same characteristics between clang and GCC with this);
So I'd like to know what (preferably real-world) code motivates the addition of this option.

[ChuanqiXu]

In D127187#3650824, @iains wrote:

Hi @ChuanqiXu,
I have no comment on the technical content of the patch (it looks reasonable to me).

However, I wonder if we should be supplying this option at all because in:
https://eel.is/c++draft/module#reach-2
note2 says "[Note 2: It is advisable to avoid depending on the reachability of any additional translation units in programs intending to be portable. — end note]"

So by providing the option, we are effectively giving users an easy mechanism to make non-portable programs (and I'm fairly sure that would mean that they would not have the same characteristics between clang and GCC with this);
So I'd like to know what (preferably real-world) code motivates the addition of this option.

Oh, I see. In fact, I didn't meet any motivated case before. The motivation of the revision is that we feel like it might be better to add an option in D113545. But now I feel like the option is not good indeed. And this one is relatively easy to implement. So we could re-land this if we find it is useful someday.

clang/include/clang/Basic/LangOptions.def

 LANGOPT(POSIXThreads      , 1, 0, "POSIX thread support")
 LANGOPT(Blocks            , 1, 0, "blocks extension to C")
 BENIGN_LANGOPT(EmitAllDecls      , 1, 0, "emitting all declarations")
 LANGOPT(MathErrno         , 1, 1, "errno in math functions")
 BENIGN_LANGOPT(HeinousExtensions , 1, 0, "extensions that we really don't like and may be ripped out at any time")
 LANGOPT(Modules           , 1, 0, "modules semantics")
 COMPATIBLE_LANGOPT(ModulesTS  , 1, 0, "C++ Modules TS syntax")
 COMPATIBLE_LANGOPT(CPlusPlusModules, 1, 0, "C++ modules syntax")
+COMPATIBLE_LANGOPT(AllAdditionalTUReachable, 1, 0, "C++ modules syntax")
 BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 3, CMK_None,
                     "compiling a module interface")
 BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
 BENIGN_LANGOPT(BuildingPCHWithObjectFile, 1, 0, "building a pch which has a corresponding object file")
 BENIGN_LANGOPT(CacheGeneratedPCH, 1, 0, "cache generated PCH files in memory")
 BENIGN_LANGOPT(PCHInstantiateTemplates, 1, 0, "instantiate templates while building a PCH")
 COMPATIBLE_LANGOPT(ModulesDeclUse    , 1, 0, "require declaration of module uses")
 BENIGN_LANGOPT(ModulesSearchAll  , 1, 1, "searching even non-imported modules to find unresolved references")

clang/include/clang/Driver/Options.td

 defm async_exceptions: BoolFOption<"async-exceptions",
   LangOpts<"EHAsynch">, DefaultFalse,
   PosFlag<SetTrue, [CC1Option], "Enable EH Asynchronous exceptions">, NegFlag<SetFalse>>;
 defm cxx_modules : BoolFOption<"cxx-modules",
   LangOpts<"CPlusPlusModules">, Default<cpp20.KeyPath>,
   NegFlag<SetFalse, [CC1Option], "Disable">, PosFlag<SetTrue, [], "Enable">,
   BothFlags<[NoXarchOption], " modules for C++">>,
   ShouldParseIf<cplusplus.KeyPath>;
+defm all_additional_TU_reachable : BoolFOption<"all-additional-TU-reachable",
+  LangOpts<"AllAdditionalTUReachable">, DefaultFalse,
+  NegFlag<SetFalse, [CC1Option], "Disable">, PosFlag<SetTrue, [CC1Option], "Enable">,
+  BothFlags<[NoXarchOption], " treat all additional variable as reachable">>;
 def fdebug_pass_arguments : Flag<["-"], "fdebug-pass-arguments">, Group<f_Group>;
 def fdebug_pass_structure : Flag<["-"], "fdebug-pass-structure">, Group<f_Group>;
 def fdepfile_entry : Joined<["-"], "fdepfile-entry=">,
     Group<f_clang_Group>, Flags<[CC1Option]>;
 def fdiagnostics_fixit_info : Flag<["-"], "fdiagnostics-fixit-info">, Group<f_clang_Group>;
 def fno_diagnostics_fixit_info : Flag<["-"], "fno-diagnostics-fixit-info">, Group<f_Group>,
   Flags<[CC1Option]>, HelpText<"Do not include fixit information in diagnostics">,
   MarshallingInfoNegativeFlag<DiagnosticOpts<"ShowFixits">>;

clang/lib/Driver/ToolChains/Clang.cpp

static void RenderModulesOptions(Compilation &C, const Driver &D,
   }
 
   // -fmodule-maps enables implicit reading of module map files. By default,
   // this is enabled if we are using Clang's flavor of precompiled modules.
   if (Args.hasFlag(options::OPT_fimplicit_module_maps,
                    options::OPT_fno_implicit_module_maps, HaveClangModules))
     CmdArgs.push_back("-fimplicit-module-maps");
 
+  // -fall-additional-TU-reachable enables to treat all translation unit as
+  // reachable.
+  if (Args.hasFlag(options::OPT_fall_additional_TU_reachable,
+                   options::OPT_fno_all_additional_TU_reachable, false))
+    CmdArgs.push_back("-fall-additional-TU-reachable");
+
   // -fmodules-decluse checks that modules used are declared so (off by default)
   Args.addOptInFlag(CmdArgs, options::OPT_fmodules_decluse,
                     options::OPT_fno_modules_decluse);
 
   // -fmodules-strict-decluse is like -fmodule-decluse, but also checks that
   // all #included headers are part of modules.
   if (Args.hasFlag(options::OPT_fmodules_strict_decluse,
                    options::OPT_fno_modules_strict_decluse, false))

clang/lib/Sema/SemaLookup.cpp

if (DeclModule->getTopLevelModule()->isModuleInterfaceUnit())
     return true;
 
   // [module.reach]/p2
   //   Additional translation units on
   //   which the point within the program has an interface dependency may be
   //   considered reachable, but it is unspecified which are and under what
   //   circumstances.
   //
-  // The decision here is to treat all additional tranditional units as
-  // unreachable.
-  return false;
+  // The default value for AllAdditionalTUReachable is false. It implies all
+  // additional translation unit is not considered as reachable for portability.
+  return SemaRef.getLangOpts().AllAdditionalTUReachable;
 }
 
 bool Sema::isAcceptableSlow(const NamedDecl *D, Sema::AcceptableKind Kind) {
   return LookupResult::isAcceptable(*this, const_cast<NamedDecl *>(D), Kind);
 }
 
 bool Sema::shouldLinkPossiblyHiddenDecl(LookupResult &R, const NamedDecl *New) {
   // FIXME: If there are both visible and hidden declarations, we need to take

clang/test/CXX/module/module.reach/ex1.cpp

 // From [module.reach]p4, example 1
 //
 // RUN: rm -fr %t
 // RUN: mkdir -p %t
 // RUN: split-file %s %t
 //
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/M-A.cppm -o %t/M-A.pcm
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/M-B-impl.cppm -o %t/M-B.pcm
 // RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/M-C.cppm -fsyntax-only -verify
 //
 // RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/M.cppm -o %t/M.pcm
 // RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/X.cppm -fsyntax-only -verify
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/X-relax.cppm -fsyntax-only -verify -fall-additional-TU-reachable
 //
 //--- M-A.cppm
 export module M:A;
 export struct B;
 
 //--- M-B-impl.cppm
 module M:B;
 struct B {

 
 //--- X.cppm
 export module X;
 import M;
 B b3; // expected-error {{definition of 'B' must be imported from module 'M:B' before it is required}} expected-error {{}}
       // expected-note@* {{definition here is not reachable}} expected-note@* {{}}
 // FIXME: We should emit an error for unreachable definition of B.
 void g() { f(); }
+
+//--- X-relax.cppm
+// Tests that it wouldn't complain if we treat all additional TU as reachable TU.
+// expected-no-diagnostics
+export module X;
+import M;
+B b3;
+void g() { f(); }
 No newline at end of file

clang/test/CXX/module/module.reach/p2.cpp

 // RUN: rm -rf %t
 // RUN: mkdir -p %t
 // RUN: split-file %s %t
 // RUN: %clang_cc1 -std=c++20 %t/impl.cppm -emit-module-interface -o %t/M-impl.pcm
 // RUN: %clang_cc1 -std=c++20 %t/M.cppm -emit-module-interface -fprebuilt-module-path=%t -o %t/M.pcm
+// RUN: %clang_cc1 -std=c++20 %t/Use.cpp -fprebuilt-module-path=%t -verify -fsyntax-only -fall-additional-TU-reachable
 // RUN: %clang_cc1 -std=c++20 %t/UseStrict.cpp -fprebuilt-module-path=%t -verify -fsyntax-only
 
 //--- impl.cppm
 module M:impl;
 class A {};
 
 //--- M.cppm
 export module M;
 import :impl;
 export A f();
 
+//--- Use.cpp
+// expected-no-diagnostics
+import M;
+void test() {
+  auto a = f();
+}
+
 //--- UseStrict.cpp
 import M;
 void test() {
   auto a = f(); // expected-error {{definition of 'A' must be imported from module 'M:impl' before it is required}} expected-error{{}}
                 // expected-note@* {{definition here is not reachable}} expected-note@* {{}}
 }