martong added a comment. In D53708#1332922 <https://reviews.llvm.org/D53708#1332922>, @riccibruno wrote:
> In D53708#1332868 <https://reviews.llvm.org/D53708#1332868>, @martong wrote: > > > > This is a perhaps a naive comment, but why is localUncachedLookup used > > > instead of noload_lookup ? There is a fixme dating from 2013 stating > > > that noload_lookup should be used instead. > > > > This is not a naive question. I was wondering myself on the very same thing > > when I started working on the ASTImporter at 2017. > > I think when `noload_lookup` was introduced the author of that function > > tried to replace `localUncachedLookup` in ASTImporter, but that broke some > > tests. The major difference between the two functions is that the latter > > can find declarations even if they are not stored in the `LookupPtr`. This > > is rather unusual from the C/C++ point of view of lookup, and not working > > in all cases (see the introduction of this patch). Ironically, I can't just > > get rid of `localUncachedLookup` because it would break some LLDB test > > cases. > > > Ah I think I understand. For my understanding (and please correct me if I am > wrong here): > > `localUncachedLookup` will first try to do a normal lookup, and if that fails > it will try to look in the declarations in the declaration context. > Now some declarations (including declarations which are not `NamedDecl` and > declarations which matches `shouldBeHidden`) > will not be added to the `LookupPtr` (via > `makeDeclVisibleInContextWithFlags`) and so will not be found though the > normal lookup mechanism. You therefore need to use `localUncachedLookup` to > find these. > > Does this make sense ? Exactly. And this seems to be an important feature in ASTImporter, because this makes it possible that we can chain into the redecl chain a newly imported AST node to existing and structurally equivalent nodes. (The existing nodes are found by the lookup and then we iterate over the lookup results searching for structurally equivalent ones.) Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53708/new/ https://reviews.llvm.org/D53708 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
