[PATCH] D147782: [clang] Fix 'operator=' lookup in nested class

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 7 15:58:24 PDT 2023


rsmith added a comment.

What *should* happen here is that the lookup for `operator=` in `Inner::invokeAssign` should walk up the enclosing `Scope`s and `DeclContext`s, traversing both the results from the `IdResolver` and the results from `DeclContext` lookups. First, we should look in the scope corresponding to `invokeAssign`, where we find no results in the `IdResolver` (and there's no corresponding `DeclContext` so there's nowhere else to look). Then we should look in the scope corresponding to `Inner`, where again there's nothing in the `IdResolver` at that scope, but lookup in the `DeclContext` should find the member `operator=`. And so it shouldn't matter that the `Outer` scope has a lookup result in the `IdContext`, because that scope is never the one that we're doing a lookup within.

It sounds like the implicit lazy declaration of the `operator=` might be adding the name to the wrong scope, or perhaps the set of nested `Scope`s at the point where we parse `Inner::invokeAssign` is misconfigured. In `Sema::DeclareImplicitCopyAssignment`, we call `getScopeForContext(ClassDecl)` to find the right scope, and inject the name into that scope. I suspect that we could simply delete that entirely, given that any lookup for a class member name will also look into the `DeclContext` and doesn't need to find results in the `Scope` (in the `IdResolver`), but it still seems valuable to figure out why this isn't working. `getScopeForContext` finds the *innermost* scope corresponding to the given class, so if we had multiple enclosing scopes for the same class, we might get into some trouble, and it seems possible that that happens when reentering scopes when doing delayed parsing of class members. The scopes for delayed parsing are created by `Parser::ReenterClassScopeRAII`, and it looks like it's doing the right thing, but that's where I'd start looking.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147782/new/

https://reviews.llvm.org/D147782



More information about the cfe-commits mailing list