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-n...@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 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@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}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits