From 0bc993edf4ec57993c8fc11ea511b3d62ffd3c93 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Thu, 11 Aug 2022 14:08:14 +0800 Subject: [PATCH] [AST] [Modules] Introduce Decl::getNonTransparentDeclContext to handle exported friends Closing https://github.com/llvm/llvm-project/issues/56826. The root cause for pr56826 is: when we collect the template args for the friend, we need to judge if the friend lives in file context. However, if the friend lives in ExportDecl lexically, the judgement here is invalid. The solution is easy. We should judge the non transparent context and the ExportDecl is transparent context. So the solution should be good. A main concern may be the patch doesn't handle all the places of the same defect. I think it might not be bad since the patch itself should be innocent. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D131651 --- clang/include/clang/AST/DeclBase.h | 10 +++++++++- clang/lib/AST/DeclBase.cpp | 5 +++++ clang/lib/Sema/SemaTemplateInstantiate.cpp | 2 +- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 2 ++ clang/test/Modules/pr56826.cppm | 9 +++++++++ clang/unittests/AST/DeclTest.cpp | 19 +++++++++++++++++++ 6 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 clang/test/Modules/pr56826.cppm diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index d1193161fd75..1c4e726817bb 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -447,6 +447,14 @@ public: return const_cast(this)->getDeclContext(); } + /// Return the non transparent context. + /// See the comment of `DeclContext::isTransparentContext()` for the + /// definition of transparent context. + DeclContext *getNonTransparentDeclContext(); + const DeclContext *getNonTransparentDeclContext() const { + return const_cast(this)->getNonTransparentDeclContext(); + } + /// Find the innermost non-closure ancestor of this declaration, /// walking up through blocks, lambdas, etc. If that ancestor is /// not a code context (!isFunctionOrMethod()), returns null. @@ -2009,7 +2017,7 @@ public: /// Here, E is a transparent context, so its enumerator (Val1) will /// appear (semantically) that it is in the same context of E. /// Examples of transparent contexts include: enumerations (except for - /// C++0x scoped enums), and C++ linkage specifications. + /// C++0x scoped enums), C++ linkage specifications and export declaration. bool isTransparentContext() const; /// Determines whether this context or some of its ancestors is a diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index d12330de1500..f22bf6b0937e 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1032,6 +1032,11 @@ const FunctionType *Decl::getFunctionType(bool BlocksToo) const { return Ty->getAs(); } +DeclContext *Decl::getNonTransparentDeclContext() { + assert(getDeclContext()); + return getDeclContext()->getNonTransparentContext(); +} + /// Starting at a given context (a Decl or DeclContext), look for a /// code context that is not a closure (a lambda, block, etc.). template static Decl *getNonClosureContext(T *D) { diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index c4e5fb50f8cc..1dd4baf90182 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -168,7 +168,7 @@ MultiLevelTemplateArgumentList Sema::getTemplateInstantiationArgs( // instead of its semantic parent, unless of course the pattern we're // instantiating actually comes from the file's context! if (Function->getFriendObjectKind() && - Function->getDeclContext()->isFileContext() && + Function->getNonTransparentDeclContext()->isFileContext() && (!Pattern || !Pattern->getLexicalDeclContext()->isFileContext())) { Ctx = Function->getLexicalDeclContext(); RelativeToPrimary = false; diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index d303e9f1724b..140844ca4e81 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -6147,6 +6147,8 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D, // Move to the outer template scope. if (FunctionDecl *FD = dyn_cast(DC)) { + // FIXME: We should use `getNonTransparentDeclContext()` here instead + // of `getDeclContext()` once we find the invalid test case. if (FD->getFriendObjectKind() && FD->getDeclContext()->isFileContext()){ DC = FD->getLexicalDeclContext(); continue; diff --git a/clang/test/Modules/pr56826.cppm b/clang/test/Modules/pr56826.cppm new file mode 100644 index 000000000000..e6eb60bd2d0f --- /dev/null +++ b/clang/test/Modules/pr56826.cppm @@ -0,0 +1,9 @@ +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s + +export module m3; +export template struct X { + template friend void f(Self &&self) { + (Self&)self; // expected-warning {{expression result unused}} + } +}; +void g() { f(X{}); } // expected-note {{in instantiation of function template specialization}} diff --git a/clang/unittests/AST/DeclTest.cpp b/clang/unittests/AST/DeclTest.cpp index 71835a0508fd..f651f36f1344 100644 --- a/clang/unittests/AST/DeclTest.cpp +++ b/clang/unittests/AST/DeclTest.cpp @@ -260,3 +260,22 @@ TEST(Decl, ModuleAndInternalLinkage) { EXPECT_EQ(b->getLinkageInternal(), ModuleLinkage); EXPECT_EQ(g->getLinkageInternal(), ModuleLinkage); } + +TEST(Decl, GetNonTransparentDeclContext) { + llvm::Annotations Code(R"( + export module m3; + export template struct X { + template friend void f(Self &&self) { + (Self&)self; + } + };)"); + + auto AST = + tooling::buildASTFromCodeWithArgs(Code.code(), /*Args=*/{"-std=c++20"}); + ASTContext &Ctx = AST->getASTContext(); + + auto *f = selectFirst( + "f", match(functionDecl(hasName("f")).bind("f"), Ctx)); + + EXPECT_TRUE(f->getNonTransparentDeclContext()->isFileContext()); +}