[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

I worry that your patch changes the behavior when you add the type_class explicitly in the lookup (i.e. look up "struct Struct" not "Struct".  That should work...

Note, this doesn't currently work in type lookup:

  (lldb) type lookup "struct Foo"
  no type was found matching '"struct Foo"'

which I'll have to fix (grr...), but does work correctly in the SB API's:

  (lldb) script
  Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D.
  >>> module = lldb.target.modules[0]
  >>> print module
  (x86_64) /tmp/structs
  >>> result_list = module.FindTypes("struct Foo")
  >>> print result_list.GetSize()
  1
  >>> print result_list.GetTypeAtIndex(0)
  struct Foo {
      int First;
  }

Other than that this looks correct to me.



================
Comment at: lldb/source/Core/Module.cpp:1012
+    TypeClass type_class = eTypeClassAny;
+    Type::GetTypeScopeAndBasename(type_name, type_scope, type_basename,
+                                  type_class);
----------------
GetTypeScopeAndBasename's behavior is not well documented.  It has a bool return which should mean some kind of failure.  The code you are replacing checks for type_basename.empty() and the type_class not being set, and falls back on the input name if it is.  You unconditionally use type_basename.  

The old code's test doesn't actually accord with the current behavior of GetTypeScopeAndBasename, which will only leave basename empty if the input name was empty, so far as I can see.  So I don't think your version is wrong, but it is right only by accident.  If we are going to rely on this behavior then it's probably a good idea to document it in the definition of GetTypeScopeAndBasename.  Or go back to checking whether type_basename is empty...

Also, by not calling GetTypeScopeAndBasename before you call FindTypes_Impl, your version of the code would pass it "struct Foo" if that's what the user typed, whereas the old code would pass "Foo" (the struct would get stripped by GetTypeScopeAndBasename).  I'm not sure whether that matters, did you try 'type lookup "struct Struct"' or something like that?  It doesn't look like you do that in your test cases.

Also your "exact" would call "struct ::Foo" not exact, whereas the old code would call that exact.


https://reviews.llvm.org/D53662