[clang] [C++20] [Modules] [Itanium ABI] Generate the vtable in the module unit of dynamic classes (PR #75912)

Chuanqi Xu via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 10 18:55:27 PDT 2024


ChuanqiXu9 wrote:

> The reproducer turned out to be pretty simple:
> 
> ```c++
> export module a;
> module :private;
> static void f() {}
> void g() {
>   f();
> }
> ```
> 
> Compiles without that patch, otherwise produces:
> 
> ```
> error: no matching function for call to 'f'
> ```
> 
> > Oh, sorry, I took another look at the commit and it looks the change makes it not a NFC change is this line: [99873b3#diff-6fe53759e8d797c328c73ada5f3324c6248a8634ef36131c7eb2b9d89192bb64R6514](https://github.com/llvm/llvm-project/commit/99873b35da7ecb905143c8a6b8deca4d4416f1a9#diff-6fe53759e8d797c328c73ada5f3324c6248a8634ef36131c7eb2b9d89192bb64R6514)
> > This shouldn't be in that commit but in this commit. It is not intentional. I guess we can't observe that if we put that in this PR too. And that change looks not bad. So maybe it makes something already bad to show up.
> 
> Even without that change, the other changes are too complex, and there are other suspicious things you wouldn't expect in an NFC commit which doesn't call this out specifically, like the removal of that whole block with the FIXME included.
> 
> I think the appropriate tag for such commits would be NFCI, and should still require PR and review.

Thanks for the reproducer and the information about NFCI. It looks like I took an oversight in the refactoring of `isInAnotherModuleUnit` that I missed the case about private module fragment.

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


More information about the cfe-commits mailing list