[clang] db3d0e4 - [C++20] [Modules] Don't diagnose on invisible namesapce
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 4 01:05:43 PST 2023
Author: Chuanqi Xu
Date: 2023-12-04T17:05:27+08:00
New Revision: db3d0e4dfa34e59fab90c0726a6722f82db48462
URL: https://github.com/llvm/llvm-project/commit/db3d0e4dfa34e59fab90c0726a6722f82db48462
DIFF: https://github.com/llvm/llvm-project/commit/db3d0e4dfa34e59fab90c0726a6722f82db48462.diff
LOG: [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.
Added:
clang/test/Modules/pr73893.cppm
Modified:
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaLookup.cpp
clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp
clang/test/CXX/module/module.interface/p2.cpp
clang/test/Modules/submodules-merge-defs.cpp
Removed:
################################################################################
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 7385eac48d8c8..8fedf41d8424a 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -12114,6 +12114,24 @@ class NamespaceValidatorCCC final : public CorrectionCandidateCallback {
}
+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);
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 54b48f7ab80eb..996f8b57233ba 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/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
diff --git a/clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp b/clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp
index c1a824bd51493..a27946bd90a46 100644
--- a/clang/test/CXX/basic/basic.lookup/basic.lookup.argdep/p5-ex2.cpp
+++ b/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 at N.cpp:4 {{declaration here is not visible}}
- // expected-error at -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
diff --git a/clang/test/CXX/module/module.interface/p2.cpp b/clang/test/CXX/module/module.interface/p2.cpp
index bf4e3f9650fd4..4f06b9f386869 100644
--- a/clang/test/CXX/module/module.interface/p2.cpp
+++ b/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}}
diff --git a/clang/test/Modules/pr73893.cppm b/clang/test/Modules/pr73893.cppm
new file mode 100644
index 0000000000000..fcf522b55d28e
--- /dev/null
+++ b/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'}}
+}
diff --git a/clang/test/Modules/submodules-merge-defs.cpp b/clang/test/Modules/submodules-merge-defs.cpp
index 777fe6936a43b..0a54699a96723 100644
--- a/clang/test/Modules/submodules-merge-defs.cpp
+++ b/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 at defs.h:49 +{{here}}
// expected-note at defs.h:50 +{{here}}
decltype(G::h) pre_gh = G::h; // expected-error +{{must be declared}} expected-error +{{must be defined}}
// expected-note at defs.h:51 +{{here}}
More information about the cfe-commits
mailing list