This is an archive of the discontinued LLVM Phabricator instance.

Really test type lookup in TestCppTypeLookup.py
ClosedPublic

Authored by friss on May 7 2018, 1:46 PM.

Details

Summary

... and fix one bug found this way. Currently, the test works not because
types are looked up correctly, but because by injecting local variables
we also materialize the types for Clang. If we disable the local variable
injection, then one check fails.

The reason of the failure is that FindTypes is run with max_matches==1
and this value is passed down to the symbol lookup functions. When the
search is performed only on the basename (like it's the case for an
entity defined in the root namespace), then the search will stop after
having found one match on the basename. But that match might be in a
namespace, we were really just looking up the basename in the accelerator
tables.

The solution is to not pass max_matches down, but to search without a
limit and let RemoveMismatchedTypes do its job afterwards. Note the
patch includes 2 hunks with the same change, but only the latter is
tested. I couldn't find a way to create a testcase for the other
branch of the if ('image lookup -t' allows me to get there, but it
only ever returns one type anyway).

Diff Detail

Repository
rL LLVM

Event Timeline

friss created this revision.May 7 2018, 1:46 PM
clayborg accepted this revision.May 7 2018, 1:53 PM

main problem with this approach is if the client tries to lookup "reference_type" as an example... It will end up pulling in and completing every STL type and returning the typedef only for us to weed this out.

Nothing we can do about this at the moment unless we make our SymbolFile::FindType() take some extra parameters where we can specify a decl content in an abstract way (like "at the tranlsation unit level, find me 'reference_unit', or "in class A and in class B find 'reference_type'". For now we could try to specify a parent_decl_context that is the translation unit, but we would need to fix the lookup code to fill in the correct translation unit for each module.

So for now, this works and is our best solution, but there are many problems with out type lookup that do need to be fixed, but those will need to happen later.

This revision is now accepted and ready to land.May 7 2018, 1:53 PM
This revision was automatically updated to reflect the committed changes.