hokein added a comment. In D153248#4431780 <https://reviews.llvm.org/D153248#4431780>, @nridge wrote:
> As this patch involves copying a bit of code from > CXXRecordDecl::lookupDepenentName() to HeuristicResolver, I wanted to mention > a couple of alternatives I considered: > > 1. Copy things in the other direction, i.e. implement some of the necessary > parts of resolveTypeToRecordDecl() in the helpers of lookupDependentName() > instead. I decided against this because HeuristicResolver is an area of the > code that's actively being improved; instead of having to make multiple > updates to lookupDependentName() over time, it seems cleaner to just break > HeuristicResolver's dependency on that function. > 2. Wait to make this change until after HeuristicResolver has been upstreamed > <https://github.com/clangd/clangd/discussions/1662> and unified with > lookupDependentName(). I do plan to work on this upstreaming, but as the > process will involve several steps (e.g. writing new tests, getting approval > from relevant upstream maintainers), I would prefer not to block this change > on the resolution of that process. Thanks for all thoughtful considerations. It is a bit sad that we duplicate the implementation in the `HeuristicResolver`, but given that - this part of code is not large - we need it in the Resolver anyway when upstreaming to clang I think it is probably OK to do that. The other question might worth thinking, are these cases critical to fix now? I'm not sure `this->find()` is a common case (`find();` already works today). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153248/new/ https://reviews.llvm.org/D153248 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
