[C++20] [Modules] Don't diagnose on invisible namesapce

Close https://github.com/llvm/llvm-project/issues/73893

As the issue shows, generally, the diagnose information for
invisible namespace is confusing more than helpful. Also this patch
implements the same solution as suggested in the issue: don't diagnose
on invisible namespace.

clang/lib/Sema/SemaDeclCXX.cpp

@@ -12114,6 +12114,24 @@ public:
 
 }
 
+static void DiagnoseInvisibleNamespace(const TypoCorrection &Corrected,
+                                       Sema &S) {
+  auto *ND = cast<NamespaceDecl>(Corrected.getFoundDecl());
+  Module *M = ND->getOwningModule();
+  assert(M && "hidden namespace definition not in a module?");
+
+  if (M->isExplicitGlobalModule())
+    S.Diag(Corrected.getCorrectionRange().getBegin(),
+           diag::err_module_unimported_use_header)
+        << (int)Sema::MissingImportKind::Declaration << Corrected.getFoundDecl()
+        << /*Header Name*/ false;
+  else
+    S.Diag(Corrected.getCorrectionRange().getBegin(),
+           diag::err_module_unimported_use)
+        << (int)Sema::MissingImportKind::Declaration << Corrected.getFoundDecl()
+        << M->getTopLevelModuleName();
+}
+
 static bool TryNamespaceTypoCorrection(Sema &S, LookupResult &R, Scope *Sc,
                                        CXXScopeSpec &SS,
                                        SourceLocation IdentLoc,
@@ -12123,7 +12141,16 @@ static bool TryNamespaceTypoCorrection(Sema &S, LookupResult &R, Scope *Sc,
   if (TypoCorrection Corrected =
           S.CorrectTypo(R.getLookupNameInfo(), R.getLookupKind(), Sc, &SS, CCC,
                         Sema::CTK_ErrorRecovery)) {
-    if (DeclContext *DC = S.computeDeclContext(SS, false)) {
+    // Generally we find it is confusing more than helpful to diagnose the
+    // invisible namespace.
+    // See https://github.com/llvm/llvm-project/issues/73893.
+    //
+    // However, we should diagnose when the users are trying to using an
+    // invisible namespace. So we handle the case specially here.
+    if (isa_and_nonnull<NamespaceDecl>(Corrected.getFoundDecl()) &&
+        Corrected.requiresImport()) {
+      DiagnoseInvisibleNamespace(Corrected, S);
+    } else if (DeclContext *DC = S.computeDeclContext(SS, false)) {
       std::string CorrectedStr(Corrected.getAsString(S.getLangOpts()));
       bool DroppedSpecifier = Corrected.WillReplaceSpecifier() &&
                               Ident->getName().equals(CorrectedStr);

clang/lib/Sema/SemaLookup.cpp

@@ -5712,6 +5712,11 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, const NamedDecl *Decl,
                                  MissingImportKind MIK, bool Recover) {
   assert(!Modules.empty());
 
+  // See https://github.com/llvm/llvm-project/issues/73893. It is generally
+  // confusing than helpful to show the namespace is not visible.
+  if (isa<NamespaceDecl>(Decl))
+    return;
+
   auto NotePrevious = [&] {
     // FIXME: Suppress the note backtrace even under
     // -fdiagnostics-show-note-include-stack. We don't care how this

clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp

@@ -50,9 +50,7 @@ void test() {
   auto x = make();
 
   // error: R and R::f are not visible here
-  R::f(x); // expected-error {{declaration of 'R' must be imported from module 'N' before it is required}}
-  // expected-note@N.cpp:4 {{declaration here is not visible}}
-  // expected-error@-2 {{no type named 'f' in namespace 'R'}}
+  R::f(x); // expected-error {{no type named 'f' in namespace 'R'}}
 
   f(x); // Found by [basic.lookup.argdep] / p4.3
 

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

@@ -78,7 +78,6 @@ void use() {
   A::h();                   // expected-error {{declaration of 'h' must be imported from module 'p2' before it is required}}
                             // expected-note@* {{declaration here is not visible}}
   using namespace A::inner; // expected-error {{declaration of 'inner' must be imported from module 'p2' before it is required}}
-                            // expected-note@* {{declaration here is not visible}}
 
   // namespace B and B::inner are explicitly exported
   using namespace B;
@@ -90,7 +89,6 @@ void use() {
 
   // namespace C is not exported
   using namespace C; // expected-error {{declaration of 'C' must be imported from module 'p2' before it is required}}
-                     // expected-note@* {{declaration here is not visible}}
 
   // namespace D is exported, but D::f is not
   D::f(); // expected-error {{declaration of 'f' must be imported from module 'p2' before it is required}}

clang/test/Modules/pr73893.cppm

@@ -0,0 +1,22 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/foo.cppm -I%t -emit-module-interface -o %t/foo.pcm
+// RUN: %clang_cc1 -std=c++20 %t/use.cc -fmodule-file=foo=%t/foo.pcm -fsyntax-only -verify
+
+//--- foo.h
+namespace foo {
+
+}
+
+//--- foo.cppm
+module;
+#include "foo.h"
+export module foo;
+
+//--- use.cc
+import foo;
+void use() {
+    foo::bar(); // expected-error {{no member named 'bar' in namespace 'foo'}}
+}

clang/test/Modules/submodules-merge-defs.cpp

@@ -49,7 +49,6 @@ int pre_fg = F<int>().g<int>(); // expected-error +{{must be declared}}
 
 G::A pre_ga // expected-error +{{must be declared}}
   = G::a; // expected-error +{{must be declared}}
-// expected-note@defs.h:49 +{{here}}
 // expected-note@defs.h:50 +{{here}}
 decltype(G::h) pre_gh = G::h; // expected-error +{{must be declared}} expected-error +{{must be defined}}
 // expected-note@defs.h:51 +{{here}}