ChuanqiXu created this revision. ChuanqiXu added reviewers: erichkeane, cor3ntin, royjacobson, clang-language-wg. ChuanqiXu added a project: clang-modules. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.
Close https://github.com/llvm/llvm-project/issues/60890. For the following example: cpp export module a; export template<typename T> struct a { friend void aa(a) requires(true) { } }; cpp export module b; import a; struct b { a<int> m; }; cpp export module c; import a; struct c { void f() const { aa(a<int>()); } }; cpp import a; import b; import c; void d() { aa(a<int>()); } The current clang will reject this incorrectly. The reason is that the require clause will be replaced with the evaluated version (https://github.com/llvm/llvm-project/blob/efae3174f09560353fb0f3d528bcbffe060d5438/clang/lib/Sema/SemaConcept.cpp#L664-L665). In module 'b', the friend function is instantiated but not used so the require clause of the friend function is `(true)`. However, in module 'c', the friend function is used so the require clause is `true`. So deserializer classify these two function to two different functions instead of one. Then here is the bug report. The proposed solution is to provide an original require clause which wouldn't change during the semantic analysis. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D144626 Files: clang/include/clang/AST/Decl.h clang/lib/AST/ASTContext.cpp clang/lib/AST/Decl.cpp clang/lib/Serialization/ASTReaderDecl.cpp clang/lib/Serialization/ASTWriterDecl.cpp clang/test/Modules/pr60890.cppm
Index: clang/test/Modules/pr60890.cppm =================================================================== --- /dev/null +++ clang/test/Modules/pr60890.cppm @@ -0,0 +1,53 @@ +// https://github.com/llvm/llvm-project/issues/60890 +// +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/a.cppm -o %t/a.pcm +// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/b.cppm -fprebuilt-module-path=%t -o %t/b.pcm +// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/c.cppm -fprebuilt-module-path=%t -o %t/c.pcm +// RUN: %clang_cc1 -std=c++20 %t/d.cpp -fprebuilt-module-path=%t -fsyntax-only -verify + +//--- a.cppm +export module a; + +export template<typename T> +struct a { + friend void aa(a x) requires(true) {} + void aaa() requires(true) {} +}; + +//--- b.cppm +export module b; + +import a; + +void b() { + a<int> _; +} + +//--- c.cppm +export module c; + +import a; + +struct c { + void f() const { + a<int> _; + aa(_); + _.aaa(); + } +}; + +//--- d.cpp +// expected-no-diagnostics +import a; +import b; +import c; + +void d() { + a<int> _; + aa(_); + _.aaa(); +} Index: clang/lib/Serialization/ASTWriterDecl.cpp =================================================================== --- clang/lib/Serialization/ASTWriterDecl.cpp +++ clang/lib/Serialization/ASTWriterDecl.cpp @@ -551,6 +551,7 @@ DeclaratorDecl::ExtInfo *Info = D->getExtInfo(); Record.AddQualifierInfo(*Info); Record.AddStmt(Info->TrailingRequiresClause); + Record.AddStmt(Info->OriginalTrailingRequiresClause); } // The location information is deferred until the end of the record. Record.AddTypeRef(D->getTypeSourceInfo() ? D->getTypeSourceInfo()->getType() Index: clang/lib/Serialization/ASTReaderDecl.cpp =================================================================== --- clang/lib/Serialization/ASTReaderDecl.cpp +++ clang/lib/Serialization/ASTReaderDecl.cpp @@ -885,6 +885,7 @@ auto *Info = new (Reader.getContext()) DeclaratorDecl::ExtInfo(); Record.readQualifierInfo(*Info); Info->TrailingRequiresClause = Record.readExpr(); + Info->OriginalTrailingRequiresClause = Record.readExpr(); DD->DeclInfo = Info; } QualType TSIType = Record.readType(); Index: clang/lib/AST/Decl.cpp =================================================================== --- clang/lib/AST/Decl.cpp +++ clang/lib/AST/Decl.cpp @@ -1954,6 +1954,8 @@ DeclInfo = new (getASTContext()) ExtInfo; // Restore savedTInfo into (extended) decl info. getExtInfo()->TInfo = savedTInfo; + // We should only store TrailingRequiresClause once. + getExtInfo()->OriginalTrailingRequiresClause = TrailingRequiresClause; } // Set requires clause info. getExtInfo()->TrailingRequiresClause = TrailingRequiresClause; Index: clang/lib/AST/ASTContext.cpp =================================================================== --- clang/lib/AST/ASTContext.cpp +++ clang/lib/AST/ASTContext.cpp @@ -6683,8 +6683,8 @@ return false; } - if (!isSameConstraintExpr(FuncX->getTrailingRequiresClause(), - FuncY->getTrailingRequiresClause())) + if (!isSameConstraintExpr(FuncX->getOriginalTrailingRequiresClause(), + FuncY->getOriginalTrailingRequiresClause())) return false; auto GetTypeAsWritten = [](const FunctionDecl *FD) { Index: clang/include/clang/AST/Decl.h =================================================================== --- clang/include/clang/AST/Decl.h +++ clang/include/clang/AST/Decl.h @@ -771,6 +771,7 @@ struct ExtInfo : public QualifierInfo { TypeSourceInfo *TInfo; Expr *TrailingRequiresClause = nullptr; + Expr *OriginalTrailingRequiresClause = nullptr; }; llvm::PointerUnion<TypeSourceInfo *, ExtInfo *> DeclInfo; @@ -850,6 +851,18 @@ : nullptr; } + /// \brief The original constraint-expression introduced by the trailing + /// requires-clause. This can be used for ODR-Checking since the trailing + /// requires clause may change during the semantic analysis. + Expr *getOriginalTrailingRequiresClause() { + return hasExtInfo() ? getExtInfo()->OriginalTrailingRequiresClause + : nullptr; + } + const Expr *getOriginalTrailingRequiresClause() const { + return const_cast<DeclaratorDecl *>(this) + ->getOriginalTrailingRequiresClause(); + } + void setTrailingRequiresClause(Expr *TrailingRequiresClause); unsigned getNumTemplateParameterLists() const {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
