[clang] [C++20] [Modules] Don't set modules owner ship information for builtin declarations (PR #102115)

[Chuanqi Xu via cfe-commits]

================
@@ -2376,6 +2376,12 @@ NamedDecl *Sema::LazilyCreateBuiltin(IdentifierInfo *II, unsigned ID,
   FunctionDecl *New = CreateBuiltin(II, R, ID, Loc);
   RegisterLocallyScopedExternCDecl(New, S);
 
+  // Builtin functions shouldn't be owned by any module.
+  if (New->hasOwningModule()) {
+    New->setLocalOwningModule(nullptr);
----------------
ChuanqiXu9 wrote:

> The bug report is unclear to me.
> 
> Are you saying this is a bug, or are you just saying the AST representation is not clean from a user understanding point of view?

This is a reduced root cause of another problem I saw. http://eel.is/c++draft/basic.def.odr#15.3 says the declarations attached to named modules are not allowed to be duplicated in different TUs. But currently the builtin declarations violate this. And I believe this is only an example and we may meet other problems.

> What about builtin TypedefDecls?

This is somewhat coincidence. We would create the builtin TypedefDecls in `Sema::Initialize()`. But we haven't assign a module for the TU that time. So it works more or less surprisingly. The difference between the builtin typedef declaration and the current builtin function decl is that, the builtin typedef declaration  are created eagerly and the builtin function decls is created lazily.

> Can this situation currently arise for anything else?

IIUC, the above builtin TypedefDecl is an example. 

> It's also strange that they would belong to the TU, and the TU would belong to the module, but the builtin itself would not.

Yes, from the high level, the abstract is more or less not so straight forward. Maybe we want to save some typing works initially so we tight the process of assigning modules with the process of assigning decl contexts. But I admit, the mechanism works pretty well except the few exceptions we raise above.

> If this is a cleanliness issue, can we instead create a builtin module, add all those builtin declarations to a TU belonging to that module, and then implicitly import it in every module?
> Is that idea workable?

First this may not be a cleanliness issue. And I feel the idea is more or less not good or match the design. From the perspective of the specification, there are only two kinds of module, named module and the unnamed, aka, the global module. And **all** declarations should live in modules, either named module and the global module. So technically the compiler should assign all the declaration not in the named module into the global module. But due to historical reasons, we didn't implement the model directly but treat the declarations not in a `clang::Module` as if they are in the global module.

So I feel the idea to introduce implicit module may make the model more complex and not match the design.

BTW, another idea may be, attach the builtin function decl to the global module. But now, the implementation may only create the global module (in implementation, it is actually the global module fragment in the specification) conditionally, in another word, we will only create the global module fragment after we see `module;`. So the idea may be more complex.

So I feel the current one may be the most simple fix.

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