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

[Ilya Biryukov via cfe-commits]

================
@@ -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())
----------------
ilya-biryukov wrote:

> // >>> it looks like undocumented pragma?

The pragma is coming from `-frewrite-imports`, which is a way to generate multi-module reproducers based in a single file. It does the "obvious" thing: reads the module map until `#pragma clang module contents`, then builds a unique PCM from the code up until `#pragma clang module end`, which can later be imported by `#pragma clang module import <Name>`, where `<Name>` should match with what's written in `#pragma clang module build <Name>`.

// >>> Is it modulemap file syntax?

Yes!

This testcase should probably be written to use split-file, it's clearly more common these days, so having these pragmas in tests unrelated to those pragmas seems like a bad idea.


re the Clang modules: @ChuanqiXu9's about Clang modules are correct here. It's okay to have multiple declarations in different Clang modules as long as they follow ODR (consist of the same tokens, etc). They will be "merged" together (I believe this is similar to what's happening when we import two C++20 modules and both have the same declaration in the GMF; but I'm on the contrary, not an expert in C++20 modules, so please correct me if I'm wrong).

The Clang modules are not a concatenation of two headers, rather implement "merging" of ASTs from those two headers, as if each class was defined with a unique header guard and consecutive `#include` would only skip over the second one (in fact, we even have optimizations that skip over class tokens if the same class was already imported in some Clang module).

Please let me know if that helps and if there are still unclear things in that test.

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