Merge pull request #6130 from Bigcheese/objc-lsv-stable

[Clang][driver][cc1] Fix handling of of C++20 modules in ObjectiveC++

Author: Bigcheese

clang/include/clang/Basic/DiagnosticDriverKinds.td

@@ -483,6 +483,9 @@ def err_test_module_file_extension_format : Error<
   "-ftest-module-file-extension argument '%0' is not of the required form "
   "'blockname:major:minor:hashed:user info'">;
 
+def err_modules_no_lsv : Error<"%select{C++20|Modules TS}0 modules require "
+  "the -fmodules-local-submodule-visibility -cc1 option">;
+
 def err_drv_extract_api_wrong_kind : Error<
   "header file '%0' input '%1' does not match the type of prior input "
   "in api extraction; use '-x %2' to override">;

clang/include/clang/Driver/Options.td

@@ -492,6 +492,7 @@ defvar render_script = LangOpts<"RenderScript">;
 defvar hip = LangOpts<"HIP">;
 defvar gnu_mode = LangOpts<"GNUMode">;
 defvar asm_preprocessor = LangOpts<"AsmPreprocessor">;
+defvar objc = LangOpts<"ObjC">;
 
 defvar std = !strconcat("LangStandard::getLangStandardForKind(", lang_std.KeyPath, ")");
 
@@ -1443,8 +1444,9 @@ 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">,
+  // FIXME: improve tblgen to not need strconcat here.
+  LangOpts<"CPlusPlusModules">, Default<!strconcat(cpp20.KeyPath, " && !", objc.KeyPath)>,
+  NegFlag<SetFalse, [CC1Option], "Disable">, PosFlag<SetTrue, [CC1Option], "Enable">,
   BothFlags<[NoXarchOption], " modules for C++">>,
   ShouldParseIf<cplusplus.KeyPath>;
 def fdebug_pass_arguments : Flag<["-"], "fdebug-pass-arguments">, Group<f_Group>;
@@ -5692,12 +5694,15 @@ defm fimplicit_modules_use_lock : BoolOption<"f", "implicit-modules-use-lock",
           "duplicating work in competing clang invocations.">>;
 // FIXME: We only need this in C++ modules / Modules TS if we might textually
 // enter a different module (eg, when building a header unit).
-def fmodules_local_submodule_visibility :
-  Flag<["-"], "fmodules-local-submodule-visibility">,
-  HelpText<"Enforce name visibility rules across submodules of the same "
-           "top-level module.">,
-  MarshallingInfoFlag<LangOpts<"ModulesLocalVisibility">>,
-  ImpliedByAnyOf<[fmodules_ts.KeyPath, fcxx_modules.KeyPath]>;
+defm modules_local_submodule_visibility :
+  BoolFOption<"modules-local-submodule-visibility",
+    LangOpts<"ModulesLocalVisibility">,
+    // FIXME: improve tblgen to not need strconcat here.
+    Default<!strconcat("(", fmodules_ts.KeyPath, "||", fcxx_modules.KeyPath,
+                       ")")>,
+    PosFlag<SetTrue, [], "name visibility rules across submodules of the same "
+                         "top-level module">,
+    NegFlag<SetFalse>>;
 def fmodules_codegen :
   Flag<["-"], "fmodules-codegen">,
   HelpText<"Generate code for uses of this module that assumes an explicit "

clang/lib/Frontend/CompilerInvocation.cpp

@@ -4087,6 +4087,9 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
         (Opts.ObjCRuntime.getKind() == ObjCRuntime::FragileMacOSX);
   }
 
+  if ((Opts.ModulesTS || Opts.CPlusPlusModules) && !Opts.ModulesLocalVisibility)
+    Diags.Report(diag::err_modules_no_lsv) << (Opts.CPlusPlusModules ? 0 : 1);
+
   if (Arg *A = Args.getLastArg(options::OPT_fgnuc_version_EQ)) {
     // Check that the version has 1 to 3 components and the minor and patch
     // versions fit in two decimal digits.

clang/test/Modules/cxx20-disable.cpp

@@ -1,6 +1,6 @@
 // RUN: rm -rf %t && mkdir %t
-// RUN: %clang_cc1 -x objective-c++ -std=c++20                  -I %t %s -verify=enabled
-// RUN: %clang_cc1 -x objective-c++ -std=c++20 -fno-cxx-modules -I %t %s -verify=disabled
+// RUN: %clang_cc1 -x objective-c++ -std=c++20 -fcxx-modules -I %t %s -verify=enabled -fmodules-local-submodule-visibility
+// RUN: %clang_cc1 -x objective-c++ -std=c++20               -I %t %s -verify=disabled
 
 // enabled-no-diagnostics
 

clang/test/Modules/fno-modules-local-submodule-visibility.mm

@@ -0,0 +1,48 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -std=c++20 -fmodules -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t -fsyntax-only -I %t/include -x objective-c++ \
+// RUN:   %t/tu.m -verify
+// RUN: %clang_cc1 -std=c++20 -fmodules -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t -fsyntax-only -I %t/include -x objective-c++ \
+// RUN:   %t/tu2.m -verify -fmodules-local-submodule-visibility
+
+// RUN: not %clang_cc1 -std=c++20 -fmodules -fcxx-modules -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t -fsyntax-only -I %t/include -x objective-c++ \
+// RUN:   %t/driver.mm -fno-modules-local-submodule-visibility 2>&1 \
+// RUN:   | FileCheck %t/driver.mm -check-prefix=CPP20
+// RUN: not %clang_cc1 -std=c++20 -fmodules-ts -fimplicit-module-maps \
+// RUN:   -fmodules-cache-path=%t -fsyntax-only -I %t/include -x objective-c++ \
+// RUN:   %t/driver.mm -fno-modules-local-submodule-visibility 2>&1 \
+// RUN:   | FileCheck %t/driver.mm -check-prefix=TS
+
+//--- include/module.modulemap
+
+module M {
+  module A {
+    header "A.h"
+    export *
+  }
+  module B {
+    header "B.h"
+    export *
+  }
+}
+
+//--- include/A.h
+#define A 1
+
+//--- include/B.h
+inline int B = A;
+
+//--- tu.m
+@import M;
+int i = B;
+// expected-no-diagnostics
+
+//--- tu2.m
+@import M; // expected-error {{could not build module 'M'}}
+
+//--- driver.mm
+// CPP20: error: C++20 modules require the -fmodules-local-submodule-visibility -cc1 option
+// TS: error: Modules TS modules require the -fmodules-local-submodule-visibility -cc1 option

clang/test/Modules/import-syntax.c

@@ -8,7 +8,7 @@
 // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -I%S/Inputs -verify -x objective-c++ -DAT_IMPORT=1 %s
 //
 // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -I%S/Inputs -verify -x c++ -fmodules-ts -DIMPORT=1 %s
-// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -I%S/Inputs -verify -x objective-c++ -fmodules-ts -DIMPORT=1 %s
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -I%S/Inputs -fmodules-local-submodule-visibility -verify -x objective-c++ -fmodules-ts -DIMPORT=1 %s
 //
 // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -I%S/Inputs -verify -x c -DPRAGMA %s
 // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -I%S/Inputs -verify -x objective-c -DPRAGMA %s