================
@@ -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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
