Change TypeSystem::GetBitSize() to return an optional result.

[aprantl]

Summary

This patch changes the behavior when printing C++ function references: where we previously would get a <could not determine size>, there is now a <no summary available>. It's not clear to me whether this is a bug or an omission, but it's one step further than LLDB previously got.

[labath]

The new "error" message definitely makes more sense. The rest of the patch seems fine too.

[stella.stamenova]

TestFunctionRefs is failing on the Windows bot: http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/677

include/lldb/Symbol/ClangASTContext.h

public:
   // Create related types using the current type's AST
   //----------------------------------------------------------------------
   CompilerType GetBasicTypeFromAST(lldb::BasicType basic_type) override;
 
   //----------------------------------------------------------------------
   // Exploring the type
   //----------------------------------------------------------------------
 
-  uint64_t GetByteSize(lldb::opaque_compiler_type_t type,
+  llvm::Optional<uint64_t> GetByteSize(lldb::opaque_compiler_type_t type,
                        ExecutionContextScope *exe_scope) {
-    return (GetBitSize(type, exe_scope) + 7) / 8;
+    if (llvm::Optional<uint64_t> bit_size = GetBitSize(type, exe_scope))
+      return (*bit_size + 7) / 8;
+    return llvm::None;
   }
 
-  uint64_t GetBitSize(lldb::opaque_compiler_type_t type,
-                      ExecutionContextScope *exe_scope) override;
+  llvm::Optional<uint64_t>
+  GetBitSize(lldb::opaque_compiler_type_t type,
 
   lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type,
                              uint64_t &count) override;
 
   lldb::Format GetFormat(lldb::opaque_compiler_type_t type) override;
 
   size_t GetTypeBitAlign(lldb::opaque_compiler_type_t type) override;
 

include/lldb/Symbol/TypeSystem.h

public:
   virtual CompilerType CreateTypedef(lldb::opaque_compiler_type_t type,
                                      const char *name,
                                      const CompilerDeclContext &decl_ctx);
 
   //----------------------------------------------------------------------
   // Exploring the type
   //----------------------------------------------------------------------
 
-  virtual uint64_t GetBitSize(lldb::opaque_compiler_type_t type,
-                              ExecutionContextScope *exe_scope) = 0;
+  virtual llvm::Optional<uint64_t>
+  GetBitSize(lldb::opaque_compiler_type_t type,
 
   virtual lldb::Encoding GetEncoding(lldb::opaque_compiler_type_t type,
                                      uint64_t &count) = 0;
 
   virtual lldb::Format GetFormat(lldb::opaque_compiler_type_t type) = 0;
 
   virtual uint32_t GetNumChildren(lldb::opaque_compiler_type_t type,
                                   bool omit_empty_base_classes,

lit/SymbolFile/NativePDB/function-types-builtins.cpp

 auto rad = &unaryret<int(*const)[5], 28>;
 // CHECK: (int (*const (*)())[5]) rad = {{.*}}
 
 
 
 // Function references, we only need a couple of these since most of the
 // interesting cases are already tested.
 auto &ref = unary<bool>;
-// CHECK: (void (&)(bool)) ref = {{.*}} (&::ref = <Unable to determine byte size.>)
+// CHECK: (void (&)(bool)) ref = {{.*}} (&::ref = <no summary available>)
 auto &ref2 = unary<volatile int*>;
-// CHECK: (void (&)(volatile int *)) ref2 = {{.*}} (&::ref2 = <Unable to determine byte size.>)
+// CHECK: (void (&)(volatile int *)) ref2 = {{.*}} (&::ref2 = <no summary available>)
 auto &ref3 = varargs;
-// CHECK: (int (&)(int, int, ...)) ref3 = {{.*}} (&::ref3 = <Unable to determine byte size.>)
+// CHECK: (int (&)(int, int, ...)) ref3 = {{.*}} (&::ref3 = <no summary available>)
 
 // Multiple arguments, as before, just something to make sure it works.
 auto binp = &binary<int*, const int*>;
 // CHECK: (void (*)(int *, const int *)) binp = {{.*}}
 auto &binr = binary<int*, const int*>;
-// CHECK: (void (&)(int *, const int *)) binr = {{.*}} (&::binr = <Unable to determine byte size.>)
+// CHECK: (void (&)(int *, const int *)) binr = {{.*}} (&::binr = <no summary available>)
 
 // And finally, a function with no arguments.
 auto null = &nullary;
 // CHECK: (void (*)()) null = {{.*}}
 
 // FIXME: These currently don't work because clang-cl emits incorrect debug info
 // for std::nullptr_t.  We should fix these in clang-cl.
 auto rae = &unaryret<decltype(nullptr), 29>;
 // CHECK: (nullptr_t (*)()) rae = {{.*}}
 auto aae = &unary<decltype(nullptr)>;
 // CHECK: (void (*)(nullptr_t)) aae = {{.*}}
 
 int main(int argc, char **argv) {
   return 0;
 }

packages/Python/lldbsuite/test/lang/c/struct_types/main.c

struct point_tag {
         int x;
         int y;
         char padding[0];
     }; //% self.expect("frame variable pt.padding[0]", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["pt.padding[0] = "])
        //% self.expect("frame variable pt.padding[1]", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["pt.padding[1] = "])
        //% self.expect("expression -- (pt.padding[0])", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["(char)", " = "])
        //% self.expect("image lookup -t point_tag", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ['padding[]']) # Once rdar://problem/12566646 is fixed, this should display correctly
 
+    struct {} empty;
+    //% self.expect("frame variable empty", substrs = ["empty = {}"])
+    //% self.expect("expression -- sizeof(empty)", substrs = ["= 0"])
+
     struct rect_tag {
         struct point_tag bottom_left;
         struct point_tag top_right;
     };
     struct point_tag pt = { 2, 3, {} };
     struct rect_tag rect = {{1, 2, {}}, {3, 4, {}}};
     struct things_to_sum tts = { 2, 3, 4 };
 
     int sum = sum_things(tts); //% self.expect("expression -- &pt == (struct point_tag*)0", substrs = ['false'])
                                //% self.expect("expression -- sum_things(tts)", substrs = ['9'])
     return 0; 
 }

packages/Python/lldbsuite/test/lang/cpp/function_refs/Makefile

+LEVEL = ../../../make
+CXX_SOURCES := main.cpp
+include $(LEVEL)/Makefile.rules

packages/Python/lldbsuite/test/lang/cpp/function_refs/TestFunctionRefs.py

+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals())

packages/Python/lldbsuite/test/lang/cpp/function_refs/main.cpp

+// This is plagiarized from lit/SymbolFile/NativePDB/function-types-builtin.cpp.
+void nullary() {}
+
+template<typename Arg>
+void unary(Arg) { }
+
+template<typename A1, typename A2>
+void binary(A1, A2) { }
+
+int varargs(int, int, ...) { return 0; }
+
+auto &ref = unary<bool>;
+auto &ref2 = unary<volatile int*>;
+auto &ref3 = varargs;
+auto binp = &binary<int*, const int*>;
+auto &binr = binary<int*, const int*>;
+auto null = &nullary;
+int main(int argc, char **argv) {
+//% self.expect("target var ref", substrs=["(void (&)(bool))", "ref = 0x",
+//%             "&::ref = <no summary available>"])
+//% self.expect("target var ref2",
+//%              substrs=["(void (&)(volatile int *))", "ref2 = 0x"])
+//% self.expect("target var ref3",
+//%              substrs=["(int (&)(int, int, ...))", "ref3 = 0x"])
+//% self.expect("target var binp",
+//%              substrs=["(void (*)(int *, const int *))", "binp = 0x"])
+//% self.expect("target var binr",
+//%              substrs=["(void (&)(int *, const int *))", "binr = 0x"])
+//% self.expect("target var null",
+//%              substrs=["(void (*)())", "null = 0x"])
+  return 0;
+}

source/Core/Value.cpp

bool Value::ValueOf(ExecutionContext *exe_ctx) {
   case eContextTypeVariable: // Variable *
     ResolveValue(exe_ctx);
     return true;
   }
   return false;
 }
 
 uint64_t Value::GetValueByteSize(Status *error_ptr, ExecutionContext *exe_ctx) {
-  uint64_t byte_size = 0;
-
   switch (m_context_type) {
   case eContextTypeRegisterInfo: // RegisterInfo *
-    if (GetRegisterInfo())
-      byte_size = GetRegisterInfo()->byte_size;
+    if (GetRegisterInfo()) {
+      if (error_ptr)
+        error_ptr->Clear();
+      return GetRegisterInfo()->byte_size;
+    }
     break;
 
   case eContextTypeInvalid:
   case eContextTypeLLDBType: // Type *
   case eContextTypeVariable: // Variable *
   {
-    const CompilerType &ast_type = GetCompilerType();
-    if (ast_type.IsValid())
-      if (llvm::Optional<uint64_t> size = ast_type.GetByteSize(
-              exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr))
-        byte_size = *size;
-  } break;
-  }
-
-  if (error_ptr) {
-    if (byte_size == 0) {
-      if (error_ptr->Success())
-        error_ptr->SetErrorString("Unable to determine byte size.");
-    } else {
+    auto *scope = exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr;
+    if (llvm::Optional<uint64_t> size = GetCompilerType().GetByteSize(scope)) {
+      if (error_ptr)
         error_ptr->Clear();
+      return *size;
     }
+    break;
   }
-  return byte_size;
+  }
+  if (error_ptr && error_ptr->Success())
+    error_ptr->SetErrorString("Unable to determine byte size.");
+  return 0;
 }
 
 const CompilerType &Value::GetCompilerType() {
   if (!m_compiler_type.IsValid()) {
     switch (m_context_type) {
     case eContextTypeInvalid:
       break;
 
Status Value::GetValueAsData(ExecutionContext *exe_ctx, DataExtractor &data,
 
   // If we got here, we need to read the value from memory
   size_t byte_size = GetValueByteSize(&error, exe_ctx);
 
   // Bail if we encountered any errors getting the byte size
   if (error.Fail())
     return error;
 
+  // No memory to read for zero-sized types.
+  if (byte_size == 0)
+    return error;
+
   // Make sure we have enough room within "data", and if we don't make
   // something large enough that does
   if (!data.ValidOffsetForDataOfSize(data_offset, byte_size)) {
     auto data_sp =
         std::make_shared<DataBufferHeap>(data_offset + byte_size, '\0');
     data.SetData(data_sp);
   }
 

source/Symbol/ClangASTContext.cpp

 
 CompilerType ClangASTContext::GetBasicTypeFromAST(lldb::BasicType basic_type) {
   return ClangASTContext::GetBasicType(getASTContext(), basic_type);
 }
 //----------------------------------------------------------------------
 // Exploring the type
 //----------------------------------------------------------------------
 
-uint64_t ClangASTContext::GetBitSize(lldb::opaque_compiler_type_t type,
-                                     ExecutionContextScope *exe_scope) {
+Optional<uint64_t>
+ClangASTContext::GetBitSize(lldb::opaque_compiler_type_t type,
   if (GetCompleteType(type)) {
     clang::QualType qual_type(GetCanonicalQualType(type));
     const clang::Type::TypeClass type_class = qual_type->getTypeClass();
     switch (type_class) {
     case clang::Type::Record:
       if (GetCompleteType(type))
         return getASTContext()->getTypeSize(qual_type);
       else
-        return 0;
+        return None;
       break;
 
     case clang::Type::ObjCInterface:
     case clang::Type::ObjCObject: {
       ExecutionContext exe_ctx(exe_scope);
       Process *process = exe_ctx.GetProcessPtr();
       if (process) {
         ObjCLanguageRuntime *objc_runtime = process->GetObjCLanguageRuntime();
default:
           return getASTContext()->getTypeSize(
               qual_type->getArrayElementTypeNoTypeQual()
                   ->getCanonicalTypeUnqualified());
       }
       if (qual_type->isObjCObjectOrInterfaceType())
         return bit_size +
                getASTContext()->getTypeSize(
                    getASTContext()->ObjCBuiltinClassTy);
+      // Function types actually have a size of 0, that's not an error.
+      if (qual_type->isFunctionProtoType())
+        return bit_size;
+      if (bit_size)
         return bit_size;
     }
   }
-  return 0;
+  return None;
 }
 
 size_t ClangASTContext::GetTypeBitAlign(lldb::opaque_compiler_type_t type) {
   if (GetCompleteType(type))
     return getASTContext()->getTypeAlign(GetQualType(type));
   return 0;
 }