[clang] [C++20][Modules] Load function body from the module that gives canonical decl (PR #111992)

Dmitry Polukhin via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 11 10:06:23 PDT 2024


================
@@ -10057,15 +10057,18 @@ void ASTReader::finishPendingActions() {
       // For a function defined inline within a class template, force the
       // canonical definition to be the one inside the canonical definition of
       // the template. This ensures that we instantiate from a correct view
-      // of the template.
+      // of the template. This behaviour seems to be important only for inline
+      // friend functions. For normal member functions, it might results in
+      // selecting canonical decl from module A but body from module B.
       //
       // Sadly we can't do this more generally: we can't be sure that all
       // copies of an arbitrary class definition will have the same members
       // defined (eg, some member functions may not be instantiated, and some
       // special members may or may not have been implicitly defined).
-      if (auto *RD = dyn_cast<CXXRecordDecl>(FD->getLexicalParent()))
-        if (RD->isDependentContext() && !RD->isThisDeclarationADefinition())
-          continue;
+      if (FD->getFriendObjectKind())
----------------
dmpolukhin wrote:

Yeah, I'm not sure that there is no sequence of deserialization that might also lead to similar issue. The whole logic about which lazy desrialization and which decl will be canonical and finally definition is very fragile I think :(

Initially I was thinking about removing the whole if because it can transform canonical decl with body may not be a definition after deserialization. But removing this logic completely breaks this [test case](https://github.com/llvm/llvm-project/blob/main/clang/test/Modules/friend-definition-2.cpp). But it seems that it was added only for friend decls and nothing else.

https://github.com/llvm/llvm-project/pull/111992


More information about the cfe-commits mailing list