Author: Chuanqi Xu Date: 2025-08-20T11:47:00+08:00 New Revision: 2db239acd1e4cb61e8c5c55dcc4b08c7d64919b6
URL: https://github.com/llvm/llvm-project/commit/2db239acd1e4cb61e8c5c55dcc4b08c7d64919b6 DIFF: https://github.com/llvm/llvm-project/commit/2db239acd1e4cb61e8c5c55dcc4b08c7d64919b6.diff LOG: [C++20] [Modules] Improve the import-and-include pattern The import and include problem is a long-standing issue with the use of C++20 modules. This patch tried to improve this by skipping parsing class and functions if their declaration is already defined in modules. The scale of the patch itself is small as the patch reuses previous optimization. Maybe we can skip parsing other declarations in the future. But the patch itself should be good. Added: clang/test/Modules/skip-body-2.cppm Modified: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaDecl.cpp clang/lib/Sema/SemaTemplate.cpp clang/test/CXX/drs/cwg279.cpp clang/test/Modules/redundant-template-default-arg2.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index da9070842694e..2b835bea1c6ea 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -15328,6 +15328,16 @@ class Sema final : public SemaBase { NamedDecl *Hidden; return hasVisibleDefinition(const_cast<NamedDecl *>(D), &Hidden); } + /// Determine if \p D has a definition which allows we redefine it in current + /// TU. \p Suggested is the definition that should be made visible to expose + /// the definition. + bool isRedefinitionAllowedFor(NamedDecl *D, NamedDecl **Suggested, + bool &Visible); + bool isRedefinitionAllowedFor(const NamedDecl *D, bool &Visible) { + NamedDecl *Hidden; + return isRedefinitionAllowedFor(const_cast<NamedDecl *>(D), &Hidden, + Visible); + } /// Determine if \p D has a reachable definition. If not, suggest a /// declaration that should be made reachable to expose the definition. diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 8ddbaf34a7f47..d419f6643cfd3 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -15837,17 +15837,18 @@ Sema::CheckForFunctionRedefinition(FunctionDecl *FD, if (TypoCorrectedFunctionDefinitions.count(Definition)) return; - // If we don't have a visible definition of the function, and it's inline or - // a template, skip the new definition. - if (SkipBody && !hasVisibleDefinition(Definition) && + bool DefinitionVisible = false; + if (SkipBody && isRedefinitionAllowedFor(Definition, DefinitionVisible) && (Definition->getFormalLinkage() == Linkage::Internal || Definition->isInlined() || Definition->getDescribedFunctionTemplate() || Definition->getNumTemplateParameterLists())) { SkipBody->ShouldSkip = true; SkipBody->Previous = const_cast<FunctionDecl*>(Definition); - if (auto *TD = Definition->getDescribedFunctionTemplate()) - makeMergedDefinitionVisible(TD); - makeMergedDefinitionVisible(const_cast<FunctionDecl*>(Definition)); + if (!DefinitionVisible) { + if (auto *TD = Definition->getDescribedFunctionTemplate()) + makeMergedDefinitionVisible(TD); + makeMergedDefinitionVisible(const_cast<FunctionDecl *>(Definition)); + } return; } @@ -18196,8 +18197,10 @@ Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc, // However, ensure the decl passes the structural compatibility // check in C11 6.2.7/1 (or 6.1.2.6/1 in C89). NamedDecl *Hidden = nullptr; - if (SkipBody && (!hasVisibleDefinition(Def, &Hidden) || - getLangOpts().C23)) { + bool HiddenDefVisible = false; + if (SkipBody && + (isRedefinitionAllowedFor(Def, &Hidden, HiddenDefVisible) || + getLangOpts().C23)) { // There is a definition of this tag, but it is not visible. // We explicitly make use of C++'s one definition rule here, // and assume that this definition is identical to the hidden @@ -18213,13 +18216,15 @@ Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, SourceLocation KWLoc, ProcessDeclAttributeList(S, SkipBody->New, Attrs); return Def; - } else { - SkipBody->ShouldSkip = true; - SkipBody->Previous = Def; - makeMergedDefinitionVisible(Hidden); - // Carry on and handle it like a normal definition. We'll - // skip starting the definition later. } + + SkipBody->ShouldSkip = true; + SkipBody->Previous = Def; + if (!HiddenDefVisible && Hidden) + makeMergedDefinitionVisible(Hidden); + // Carry on and handle it like a normal definition. We'll + // skip starting the definition later. + } else if (!IsExplicitSpecializationAfterInstantiation) { // A redeclaration in function prototype scope in C isn't // visible elsewhere, so merely issue a warning. @@ -20842,3 +20847,13 @@ bool Sema::shouldIgnoreInHostDeviceCheck(FunctionDecl *Callee) { return LangOpts.CUDA && !LangOpts.CUDAIsDevice && CUDA().IdentifyTarget(Callee) == CUDAFunctionTarget::Global; } + +bool Sema::isRedefinitionAllowedFor(NamedDecl *D, NamedDecl **Suggested, + bool &Visible) { + Visible = hasVisibleDefinition(D, Suggested); + // The redefinition of D in the **current** TU is allowed if D is invisible or + // D is defined in the global module of other module units. We didn't check if + // it is in global module as, we'll check the redefinition in named module + // later with better diagnostic message. + return D->isInAnotherModuleUnit() || !Visible; +} diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 72d98ca391f4b..0edc850ae1b13 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -2078,14 +2078,19 @@ DeclResult Sema::CheckClassTemplate( // If we have a prior definition that is not visible, treat this as // simply making that previous definition visible. NamedDecl *Hidden = nullptr; - if (SkipBody && !hasVisibleDefinition(Def, &Hidden)) { + bool HiddenDefVisible = false; + if (SkipBody && + isRedefinitionAllowedFor(Def, &Hidden, HiddenDefVisible)) { SkipBody->ShouldSkip = true; SkipBody->Previous = Def; - auto *Tmpl = cast<CXXRecordDecl>(Hidden)->getDescribedClassTemplate(); - assert(Tmpl && "original definition of a class template is not a " - "class template?"); - makeMergedDefinitionVisible(Hidden); - makeMergedDefinitionVisible(Tmpl); + if (!HiddenDefVisible && Hidden) { + auto *Tmpl = + cast<CXXRecordDecl>(Hidden)->getDescribedClassTemplate(); + assert(Tmpl && "original definition of a class template is not a " + "class template?"); + makeMergedDefinitionVisible(Hidden); + makeMergedDefinitionVisible(Tmpl); + } } else { Diag(NameLoc, diag::err_redefinition) << Name; Diag(Def->getLocation(), diag::note_previous_definition); @@ -8956,10 +8961,13 @@ DeclResult Sema::ActOnClassTemplateSpecialization( if (TUK == TagUseKind::Definition) { RecordDecl *Def = Specialization->getDefinition(); NamedDecl *Hidden = nullptr; - if (Def && SkipBody && !hasVisibleDefinition(Def, &Hidden)) { + bool HiddenDefVisible = false; + if (Def && SkipBody && + isRedefinitionAllowedFor(Def, &Hidden, HiddenDefVisible)) { SkipBody->ShouldSkip = true; SkipBody->Previous = Def; - makeMergedDefinitionVisible(Hidden); + if (!HiddenDefVisible && Hidden) + makeMergedDefinitionVisible(Hidden); } else if (Def) { SourceRange Range(TemplateNameLoc, RAngleLoc); Diag(TemplateNameLoc, diag::err_redefinition) << Specialization << Range; diff --git a/clang/test/CXX/drs/cwg279.cpp b/clang/test/CXX/drs/cwg279.cpp index 3c63486cc0dd5..d10ceb2f05d30 100644 --- a/clang/test/CXX/drs/cwg279.cpp +++ b/clang/test/CXX/drs/cwg279.cpp @@ -46,8 +46,8 @@ extern S2 *q2; // FIXME: This is well-formed, because [basic.def.odr]/15 is satisfied. struct S3 {}; -// since-cxx20-error@-1 {{redefinition of 'S3'}} -// since-cxx20-note@cwg279_A.cppm:23 {{previous definition is here}} +// since-cxx20-error@-1 {{declaration of 'S3' in the global module follows declaration in module cwg279_A}} +// since-cxx20-note@cwg279_A.cppm:23 {{previous declaration is here}} extern S3 *q3; // since-cxx20-error@-1 {{declaration of 'q3' in the global module follows declaration in module cwg279_A}} // since-cxx20-note@cwg279_A.cppm:24 {{previous declaration is here}} diff --git a/clang/test/Modules/redundant-template-default-arg2.cpp b/clang/test/Modules/redundant-template-default-arg2.cpp index ae1f0c7e69cc0..6e22d823a18dc 100644 --- a/clang/test/Modules/redundant-template-default-arg2.cpp +++ b/clang/test/Modules/redundant-template-default-arg2.cpp @@ -33,8 +33,8 @@ int v2; // expected-error {{declaration of 'v2' in the global module follows dec // expected-n...@foo.cppm:6 {{previous declaration is here}} template <typename T> -class my_array {}; // expected-error {{redefinition of 'my_array'}} - // expected-n...@foo.cppm:9 {{previous definition is here}} +class my_array {}; // expected-error {{declaration of 'my_array' in the global module follows declaration in module foo}} + // expected-n...@foo.cppm:9 {{previous declaration is here}} template <template <typename> typename C = my_array> int v3; // expected-error {{declaration of 'v3' in the global module follows declaration in module foo}} diff --git a/clang/test/Modules/skip-body-2.cppm b/clang/test/Modules/skip-body-2.cppm new file mode 100644 index 0000000000000..781f4fefb72b1 --- /dev/null +++ b/clang/test/Modules/skip-body-2.cppm @@ -0,0 +1,58 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-reduced-module-interface -o %t/a.pcm +// RUN: %clang_cc1 -std=c++20 %t/a.cpp -fmodule-file=a=%t/a.pcm -ast-dump | FileCheck %s + +// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm +// RUN: %clang_cc1 -std=c++20 %t/a.cpp -fmodule-file=a=%t/a.pcm -ast-dump | FileCheck %s + +//--- a.h +namespace a { +class A { +public: + int aaaa; + + int get() { + return aaaa; + } +}; + + +template <class T> +class B { +public: + B(T t): t(t) {} + T t; +}; + +using BI = B<int>; + +inline int get(A a, BI b) { + return a.get() + b.t; +} + +} + +//--- a.cppm +export module a; + +export extern "C++" { +#include "a.h" +} + +//--- a.cpp +import a; +#include "a.h" + +int test() { + a::A aa; + a::BI bb(43); + return get(aa, bb); +} + +// CHECK-NOT: DefinitionData +// CHECK: FunctionDecl {{.*}} get 'int (A, BI)' {{.*}} +// CHECK-NOT: CompoundStmt +// CHECK: FunctionDecl {{.*}} test {{.*}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
