[PATCH] D53708: [ASTImporter] Add importer specific lookup
Bruno Ricci via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 17 06:40:08 PST 2018
riccibruno added a comment.
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 ?
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D53708/new/
https://reviews.llvm.org/D53708
More information about the cfe-commits
mailing list