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

Fri Apr 7 05:57:47 PDT 2023

J-Camilleri added a reviewer: aaron.ballman.
J-Camilleri added a comment.

Firstly, If there are other reviewers which I should include instead please let me know, the developers that changed around this area did so a long time ago and I do not know who to add.

I have described the problem in the github issue (https://github.com/llvm/llvm-project/issues/59684) and as I said there I am uncertain about my solution.
Given the issue and my limited knowledge of the code base, I am thinking about three possible solutions to this issue.

1. Have the user defined operator present in the `IdResolver` so that it is returned with the implicitly declared declarations.
  - I have seen cases where the declaration is kept in an `FETokenInfo` section but not added to the `IdResolver`, it gets removed at some point and leads to it not being present when `CppLookupName` is called

2. Have the `IdResolver` declarations ordered by scope, going from most nested to least nested.
  - This suggestion stems from how the ommition of `Outer::invokeAssign` is handled.
  - Seems expensive to maintain.

I do not understand the role of the `IdResolver` well so I do not know if the suggestions above misuse it/ change it's intended role.

3. Do not end the search at the `IdResolver` if we are searching for an 'operator='.
  - I went for this solution because it is simplest to implement and has the least changes.
  - This causes the `LookupResult` to temporary contain duplicate declarations, they are cleaned later by a call to `LookupResult::resolveKind` but it still feels like this should not happen. I guess we can skip the loop alltogether to not have duplications but I did not check to see if this is viable.

I guess my main issue with this is understanding the role of `IdResolver`, should the order to implicit declarations change what it returns, should it already contain the user defined operator declaration?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147782