[clang] [RFC][C++20][Modules] Fix crash when function and lambda inside loaded from different modules (PR #104512)
Dmitry Polukhin via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 20 04:52:15 PDT 2024
dmpolukhin wrote:
> Here is another example that merging lambdas are problematic: #102721. Although I think we need to solve the problems separately. They are different problems.
Thank you for the reference, we need this fix too.
> But the current patch still smells bad. Let's try to find a cleaner way to proceed. IIUC, the key of the current problem is that, the CXXRecordDecl of the lambda are assigned to the wrong FunctionDecl as the DeclContext? Or the LambdaExpr appears in the wrong FunctionDecl?
Yeah, I agree. Problem with current code is not wrong assignment (everything match respectively) but which decl becomes canonical. Function decl from module A became canonical but for lambda canonical decl is chosen from module B. It happens because of the order of loading template specialization. Lookup by name makes canonical decl for function `tryTo` from module `thrift_cpp2_base.pcm` (last module loaded). `RedeclarableTemplateDecl::loadLazySpecializationsImpl` gets the most recent decl from module `thrift_cpp2_base.pcm` but it loads specialization in direct order so specialization from the first module `folly-conv.pcm` are loaded before specialization from `thrift_cpp2_base.pcm` and thus via dependencies lambda from the wrong module became canonical. In the last version I changed order of specialization loading to match order of lookup by name. I'm not sure that it will solve all cases when canonical decl for lambda can be chosen from another module but it seems that it reduces such probability substantially because now it match the most frequent case lookup by name. I'm still testing my changes on bigger codebase but so far it seems that it doesn't regress anything and solves my test case.
It doesn't completely solve the issue because it might be other code paths that result making canonical decl in different order. Alternative solution is abandon lazy loading for function and load its internal lambda right after making any function canonical. I think it will negatively affect performance because more things will be loaded that can be avoided with current approach. WDYT?
https://github.com/llvm/llvm-project/pull/104512
More information about the cfe-commits
mailing list