ChuanqiXu created this revision.
ChuanqiXu added reviewers: aaron.ballman, erichkeane, ilya-biryukov, iains.
ChuanqiXu added a project: clang.
Herald added a project: All.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.
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.
Another alternative may let the `Decl::getContext()` return non transparent
context but I feel it changes too drastically.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D131651
Files:
clang/include/clang/AST/DeclBase.h
clang/lib/AST/DeclBase.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/Modules/pr56826.cppm
clang/unittests/AST/DeclTest.cpp
Index: clang/unittests/AST/DeclTest.cpp
===================================================================
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -260,3 +260,22 @@
EXPECT_EQ(b->getLinkageInternal(), ModuleLinkage);
EXPECT_EQ(g->getLinkageInternal(), ModuleLinkage);
}
+
+TEST(Decl, GetNonTransparentDeclContext) {
+ llvm::Annotations Code(R"(
+ export module m3;
+ export template <class> struct X {
+ template <class Self> friend void f(Self &&self) {
+ (Self&)self;
+ }
+ };)");
+
+ auto AST =
+ tooling::buildASTFromCodeWithArgs(Code.code(), /*Args=*/{"-std=c++20"});
+ ASTContext &Ctx = AST->getASTContext();
+
+ auto *f = selectFirst<FunctionDecl>(
+ "f", match(functionDecl(hasName("f")).bind("f"), Ctx));
+
+ EXPECT_TRUE(f->getNonTransparentDeclContext()->isFileContext());
+}
Index: clang/test/Modules/pr56826.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/pr56826.cppm
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+export module m3;
+export template <class> struct X {
+ template <class Self> friend void f(Self &&self) {
+ (Self&)self; // expected-warning {{expression result unused}}
+ }
+};
+void g() { f(X<void>{}); } // expected-note {{in instantiation of function template specialization}}
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6147,6 +6147,8 @@
// Move to the outer template scope.
if (FunctionDecl *FD = dyn_cast<FunctionDecl>(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;
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -168,7 +168,7 @@
// 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;
Index: clang/lib/AST/DeclBase.cpp
===================================================================
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1032,6 +1032,11 @@
return Ty->getAs<FunctionType>();
}
+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 <class T> static Decl *getNonClosureContext(T *D) {
Index: clang/include/clang/AST/DeclBase.h
===================================================================
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -447,6 +447,14 @@
return const_cast<Decl*>(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<Decl *>(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 @@
/// 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
