mpark wrote: > > > Why is the namespace decl special? Can we make this optimization more > > > general? > > > > > > I did try to generalize it for all redeclarable decls, but it failed a > > bunch of unit tests. My understanding is that namespaces are indeed special > > in this aspect, according to [this comment in > > `getAcceptableDeclSlow`](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaLookup.cpp#L2129-L2134): > > > > > > ``` > > > if (auto *ND = dyn_cast<NamespaceDecl>(D)) { > > > // Namespaces are a bit of a special case: we expect there to be a lot > > of > > > // redeclarations of some namespaces, all declarations of a namespace > > are > > > // essentially interchangeable, all declarations are found by name > > lookup > > > // if any is, and namespaces are never looked up during template > > > // instantiation. So we benefit from caching the check in this case, and > > > // it is correct to do so. > > > ... > > > } > > > ... > > > ``` > > > > I think the only point in the comment are all declarations of a namespace are > interchangable. While for class we may have: > > > > ``` > > class Foo; > > ``` > > > > and > > > > ``` > > class Foo { > > ... > > }; > > ``` > > > > they are not interchangable. > > > > But the idea of the patch still seems valuable and doable for at least > classes. Maybe it is not a super stragihtfoward extension. But I believe the > problems are solvable and it is worthy to look into the problems. > > > > I am not asking to address this in the PR. But let's add a FIXME to TODO for > that.
Right, I was giving this more thought earlier today and I do think there's likely to be an improvement to be had there. Probably along the lines of: keeping one class decl, and replacing it with the definition if one exists, etc rather than unconditionally taking one decl like we can with namespaces. As much as I'd like to dig further, I can't afford to do this right now so I'm happy to leave a note for the opportunity. https://github.com/llvm/llvm-project/pull/171769 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
