[lldb] Add MockTypeSystem and some basic unit test for CompilerType

[teemperor]

Summary

There are a lot of classes using TypeSystem in some way (mostly indirectly
through CompilerType) and we can't really unittest this code at the moment without
having some kind of mock TypeSystem.

This adds a MockTypeSystem to the TestingSupport that provides a stub implementation
for all the deleted functions. Also adds a very basic CompilerTest that just uses the MockTypeSystem
as a simple TypeSystem placeholder.

[labath]

I think this should go into TestingSupport/Symbol (new folder) similar to how NativeProcessUtils.h is in TestingSupport/Host (to avoid the basic testing support functionality depending on the whole world).

It's not obvious how you intend to use this class, but instead of defining all methods as llvm_unreachable you could consider mocking them via gmock. This way you wouldn't need to create a separate mock class for each use case, and could instead "program" the mocks via something like EXPECT_CALL(my_mock, GetNumVirtualBaseClasses(42)).Return(47);.

[teemperor]

I tried moving this to gmock, but after an hour of not being able to even declare a simple virtual method due to issues like this I don't think this approach is really practical. gmock is a nightmare to debug with all these completely undocumented macros. If gmock was actually documented and would work, then I think we should use it. But at the current state I can't even get it to compile code it seems without having to diagnostic-disable hacks, so that's a no-go

Also we anyway also want to model things like fake type hierarchies that one can traverse via CompilerType/TypeSystem (e.g. when doing completions like in the Variable completion code), so simple mocking is anyway not really the right fit.

Changes:

• Renamed MockTypeSystem -> FakeTypeSystem to reflect that this is not just mocking.
• Moved TestingSupport library to its own subdirectory and library that links against Symbol.
• Replaced llvm_unreachable in the unimplemented functions with macro to make migration easier if someone has a better approach to handling them.

[labath]

In D70775#1761597, @teemperor wrote:

I tried moving this to gmock, but after an hour of not being able to even declare a simple virtual method due to issues like this I don't think this approach is really practical. gmock is a nightmare to debug with all these completely undocumented macros. If gmock was actually documented and would work, then I think we should use it. But at the current state I can't even get it to compile code it seems without having to diagnostic-disable hacks, so that's a no-go

That's easy to work around by just not using any "override" keywords in the class which defines the mock methods. This is unfortunate, but I don't think it's a showstopper. If anything, it's a reason for updating to a newer gmock version, since that's solved there.

I'm not sure what documentation you were looking at, but overall, I've found googlemock to be documented pretty well (definitely better than llvm or lldb).

So, I don't think either of these things are a reason to *not use* it. However, they are not reasons for *using* it either. Gmock is good for some cases, and not so good for others. Right now it's not clear to me what kind of uses you have in mind, but in the simple tests you have in this patch, it is true that gmock would not provide any value. We can revisit that decision later, or add a new gmock-based type system class (there's nothing stopping us from having more than one).

Also we anyway also want to model things like fake type hierarchies that one can traverse via CompilerType/TypeSystem (e.g. when doing completions like in the Variable completion code), so simple mocking is anyway not really the right fit.

Yes, that is something where gmock would not be particularly useful..

Changes:

• Renamed MockTypeSystem -> FakeTypeSystem to reflect that this is not just mocking.
• Moved TestingSupport library to its own subdirectory and library that links against Symbol.

Thanks.

• Replaced llvm_unreachable in the unimplemented functions with macro to make migration easier if someone has a better approach to handling them.

I don't think this brings any value (the "verboseness" of llvm_unreachable("unimplemented") was not my main issue with this approach). Let's just revert that.

lldb/unittests/TestingSupport/Symbol/FakeTypeSystem.h

22

I don't think this is worth a macro. If you really want to save a couple of characters you can just declare a unimplemented static member function...

lldb/unittests/Symbol/CMakeLists.txt

 add_lldb_unittest(SymbolTests
+  CompilerTypeTest.cpp
   LocateSymbolFileTest.cpp
   PostfixExpressionTest.cpp
   TestClangASTContext.cpp
   TestDWARFCallFrameInfo.cpp
   TestType.cpp
   TestLineEntry.cpp
 
   LINK_LIBS
     lldbHost
     lldbSymbol
     lldbUtilityHelpers
+    lldbSymbolHelpers
     lldbPluginObjectFileELF
     lldbPluginObjectFileMachO
     lldbPluginSymbolFileDWARF
     lldbPluginSymbolFileSymtab
     LLVMTestingSupport
   )
 
 set(test_inputs
   inlined-functions.yaml
   )
 add_unittest_inputs(SymbolTests "${test_inputs}")

lldb/unittests/Symbol/CompilerTypeTest.cpp

+//===-- CompilerTypeTest.cpp -------------------------==---------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Symbol/CompilerType.h"
+#include "TestingSupport/Symbol/FakeTypeSystem.h"
+
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+namespace {
+struct TestTypeSystem : public lldb_private::FakeTypeSystem {
+  void *GetOpaqueValidType() { return &valid_type_storage; }
+
+private:
+  int valid_type_storage;
+};
+} // namespace
+
+TEST(CompilerType, DefaultConstructor) {
+  CompilerType type;
+  EXPECT_EQ(nullptr, type.GetTypeSystem());
+  EXPECT_EQ(nullptr, type.GetOpaqueQualType());
+}
+
+TEST(CompilerType, Constructor) {
+  TestTypeSystem ts;
+  CompilerType type(&ts, ts.GetOpaqueValidType());
+  EXPECT_EQ(&ts, type.GetTypeSystem());
+  EXPECT_EQ(ts.GetOpaqueValidType(), type.GetOpaqueQualType());
+}
+
+TEST(CompilerType, Clear) {
+  TestTypeSystem ts;
+  CompilerType type(&ts, ts.GetOpaqueValidType());
+
+  type.Clear();
+  EXPECT_EQ(nullptr, type.GetTypeSystem());
+  EXPECT_EQ(nullptr, type.GetOpaqueQualType());
+}

lldb/unittests/TestingSupport/CMakeLists.txt

 set_property(DIRECTORY PROPERTY EXCLUDE_FROM_ALL ON)
 add_lldb_library(lldbUtilityHelpers
   MockTildeExpressionResolver.cpp
   TestUtilities.cpp
 
   LINK_LIBS
     lldbUtility
 
   LINK_COMPONENTS
     Support
     ObjectYAML
   )
 include_directories(${LLVM_MAIN_SRC_DIR}/utils/unittest/googletest/include)
+
+add_subdirectory(Symbol)

lldb/unittests/TestingSupport/Symbol/CMakeLists.txt

+set_property(DIRECTORY PROPERTY EXCLUDE_FROM_ALL ON)
+add_lldb_library(lldbSymbolHelpers
+  FakeTypeSystem.cpp
+
+  LINK_LIBS
+    lldbSymbol
+
+  LINK_COMPONENTS
+    Support
+  )
+include_directories(${LLVM_MAIN_SRC_DIR}/utils/unittest/googletest/include)

lldb/unittests/TestingSupport/Symbol/FakeTypeSystem.h

+//===-- FakeTypeSystem.h -----------------------------==---------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeSystem.h"
+
+namespace lldb_private {
+/// A TypeSystem implementation that throws an error if any of its methods are
+/// called. Should be used as a base class for new TypeSystem subclasses in
+/// tests.
+struct FakeTypeSystem : public TypeSystem {
+  // llvm casting support
+  static char ID;
+  bool isA(const void *ClassID) const override { return ClassID == &ID; }
+  static bool classof(const TypeSystem *ts) { return ts->isA(&ID); }
+
+#define UNIMPLEMENTED_METHOD llvm_unreachable("not implemented")
+
+  ConstString GetPluginName() override { UNIMPLEMENTED_METHOD; }
+
+  uint32_t GetPluginVersion() override { UNIMPLEMENTED_METHOD; }
+
+  ConstString DeclGetName(void *opaque_decl) override { UNIMPLEMENTED_METHOD; }
+
+  CompilerType GetTypeForDecl(void *opaque_decl) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool DeclContextIsStructUnionOrClass(void *opaque_decl_ctx) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  ConstString DeclContextGetName(void *opaque_decl_ctx) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  ConstString DeclContextGetScopeQualifiedName(void *opaque_decl_ctx) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool
+  DeclContextIsClassMethod(void *opaque_decl_ctx,
+                           lldb::LanguageType *language_ptr,
+                           bool *is_instance_method_ptr,
+                           ConstString *language_object_name_ptr) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool DeclContextIsContainedInLookup(void *opaque_decl_ctx,
+                                      void *other_opaque_decl_ctx) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsArrayType(lldb::opaque_compiler_type_t type,
+                   CompilerType *element_type, uint64_t *size,
+                   bool *is_incomplete) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsAggregateType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsCharType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsCompleteType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsDefined(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsFloatingPointType(lldb::opaque_compiler_type_t type, uint32_t &count,
+                           bool &is_complex) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsFunctionType(lldb::opaque_compiler_type_t type,
+                      bool *is_variadic_ptr) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  size_t
+  GetNumberOfFunctionArguments(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetFunctionArgumentAtIndex(lldb::opaque_compiler_type_t type,
+                                          const size_t index) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsFunctionPointerType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsBlockPointerType(lldb::opaque_compiler_type_t type,
+                          CompilerType *function_pointer_type_ptr) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsIntegerType(lldb::opaque_compiler_type_t type,
+                     bool &is_signed) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsPossibleDynamicType(lldb::opaque_compiler_type_t type,
+                             CompilerType *target_type, bool check_cplusplus,
+                             bool check_objc) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsPointerType(lldb::opaque_compiler_type_t type,
+                     CompilerType *pointee_type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsScalarType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsVoidType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool CanPassInRegisters(const CompilerType &type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool SupportsLanguage(lldb::LanguageType language) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool GetCompleteType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  uint32_t GetPointerByteSize() override { UNIMPLEMENTED_METHOD; }
+
+  ConstString GetTypeName(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  uint32_t
+  GetTypeInfo(lldb::opaque_compiler_type_t type,
+              CompilerType *pointee_or_element_compiler_type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  lldb::LanguageType
+  GetMinimumLanguage(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  lldb::TypeClass GetTypeClass(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetArrayElementType(lldb::opaque_compiler_type_t type,
+                                   uint64_t *stride) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetCanonicalType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  int GetFunctionArgumentCount(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetFunctionArgumentTypeAtIndex(lldb::opaque_compiler_type_t type,
+                                              size_t idx) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType
+  GetFunctionReturnType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  size_t GetNumMemberFunctions(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  TypeMemberFunctionImpl
+  GetMemberFunctionAtIndex(lldb::opaque_compiler_type_t type,
+                           size_t idx) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetPointeeType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetPointerType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  const llvm::fltSemantics &GetFloatTypeSemantics(size_t byte_size) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  llvm::Optional<uint64_t>
+  GetBitSize(lldb::opaque_compiler_type_t type,
+             ExecutionContextScope *exe_scope) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type,
+                             uint64_t &count) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  lldb::Format GetFormat(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  uint32_t GetNumChildren(lldb::opaque_compiler_type_t type,
+                          bool omit_empty_base_classes,
+                          const ExecutionContext *exe_ctx) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  lldb::BasicType
+  GetBasicTypeEnumeration(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  uint32_t GetNumFields(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetFieldAtIndex(lldb::opaque_compiler_type_t type, size_t idx,
+                               std::string &name, uint64_t *bit_offset_ptr,
+                               uint32_t *bitfield_bit_size_ptr,
+                               bool *is_bitfield_ptr) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  uint32_t GetNumDirectBaseClasses(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  uint32_t
+  GetNumVirtualBaseClasses(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetDirectBaseClassAtIndex(lldb::opaque_compiler_type_t type,
+                                         size_t idx,
+                                         uint32_t *bit_offset_ptr) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetVirtualBaseClassAtIndex(lldb::opaque_compiler_type_t type,
+                                          size_t idx,
+                                          uint32_t *bit_offset_ptr) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetChildCompilerTypeAtIndex(
+      lldb::opaque_compiler_type_t type, ExecutionContext *exe_ctx, size_t idx,
+      bool transparent_pointers, bool omit_empty_base_classes,
+      bool ignore_array_bounds, std::string &child_name,
+      uint32_t &child_byte_size, int32_t &child_byte_offset,
+      uint32_t &child_bitfield_bit_size, uint32_t &child_bitfield_bit_offset,
+      bool &child_is_base_class, bool &child_is_deref_of_parent,
+      ValueObject *valobj, uint64_t &language_flags) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  uint32_t GetIndexOfChildWithName(lldb::opaque_compiler_type_t type,
+                                   const char *name,
+                                   bool omit_empty_base_classes) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  size_t
+  GetIndexOfChildMemberWithName(lldb::opaque_compiler_type_t type,
+                                const char *name, bool omit_empty_base_classes,
+                                std::vector<uint32_t> &child_indexes) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+#ifndef NDEBUG
+  /// Convenience LLVM-style dump method for use in the debugger only.
+  LLVM_DUMP_METHOD void dump(lldb::opaque_compiler_type_t type) const override {
+    UNIMPLEMENTED_METHOD;
+  }
+#endif
+
+  void DumpValue(lldb::opaque_compiler_type_t type, ExecutionContext *exe_ctx,
+                 Stream *s, lldb::Format format, const DataExtractor &data,
+                 lldb::offset_t data_offset, size_t data_byte_size,
+                 uint32_t bitfield_bit_size, uint32_t bitfield_bit_offset,
+                 bool show_types, bool show_summary, bool verbose,
+                 uint32_t depth) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool DumpTypeValue(lldb::opaque_compiler_type_t type, Stream *s,
+                     lldb::Format format, const DataExtractor &data,
+                     lldb::offset_t data_offset, size_t data_byte_size,
+                     uint32_t bitfield_bit_size, uint32_t bitfield_bit_offset,
+                     ExecutionContextScope *exe_scope) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  void DumpTypeDescription(lldb::opaque_compiler_type_t type) override {}
+
+  void DumpTypeDescription(lldb::opaque_compiler_type_t type,
+                           Stream *s) override {}
+
+  bool IsRuntimeGeneratedType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  void DumpSummary(lldb::opaque_compiler_type_t type, ExecutionContext *exe_ctx,
+                   Stream *s, const DataExtractor &data,
+                   lldb::offset_t data_offset, size_t data_byte_size) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsPointerOrReferenceType(lldb::opaque_compiler_type_t type,
+                                CompilerType *pointee_type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  unsigned GetTypeQualifiers(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsCStringType(lldb::opaque_compiler_type_t type,
+                     uint32_t &length) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  llvm::Optional<size_t>
+  GetTypeBitAlign(lldb::opaque_compiler_type_t type,
+                  ExecutionContextScope *exe_scope) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetBasicTypeFromAST(lldb::BasicType basic_type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetBuiltinTypeForEncodingAndBitSize(lldb::Encoding encoding,
+                                                   size_t bit_size) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsBeingDefined(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+  bool IsConst(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  uint32_t IsHomogeneousAggregate(lldb::opaque_compiler_type_t type,
+                                  CompilerType *base_type_ptr) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsPolymorphicClass(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsTypedefType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetTypedefedType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsVectorType(lldb::opaque_compiler_type_t type,
+                    CompilerType *element_type, uint64_t *size) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType
+  GetFullyUnqualifiedType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  CompilerType GetNonReferenceType(lldb::opaque_compiler_type_t type) override {
+    UNIMPLEMENTED_METHOD;
+  }
+
+  bool IsReferenceType(lldb::opaque_compiler_type_t type,
+                       CompilerType *pointee_type, bool *is_rvalue) override {
+    UNIMPLEMENTED_METHOD;
+  }
+};
+
+#undef UNIMPLEMENTED_METHOD
+
+} // namespace lldb_private

lldb/unittests/TestingSupport/Symbol/FakeTypeSystem.cpp

+//===-- FakeTypeSystem.cpp ---------------------------==---------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "FakeTypeSystem.h"
+
+char lldb_private::FakeTypeSystem::ID;