[Lldb-commits] [PATCH] D88483: Add possibility to get module from SBType

Jim Ingham via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Oct 29 16:40:36 PDT 2020


jingham added a comment.

Sorry to be picky, but you can compress this by putting the IsValid checks into your find_* routines.  And the:

  self.assertEqual(cu_type.GetName(), type2_name)

calls aren't necessary, since you already found the type by matching the name.  So I think you can simplify this by moving all the checks into the helper routines.

Note, you are also assuming that SBTarget::FindFirstType goes through the modules starting with the executable module for the tests at the end of the file.  That is true but is not part of the contract for FindFirstType.  It doesn't seem a good idea to rely on that either.

I pushed this version so we can hopefully either clear up the bot failures or better see why they are happening.  But can you clean up the patch as suggested above for the final version?  Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88483/new/

https://reviews.llvm.org/D88483



More information about the lldb-commits mailing list