[Symbol] GetTypeBitAlign() should return None in case of failure.

[davide]

Summary

And not zero. This is the last API needed to be converted to
an Optional<T>.

[xiaobai]

I appreciate you adding more error handling. Thank you!

[davide]

More than error handling, this allows us to distinguish with a type with zero alignment (if possible at all?) and an error getting the alignment from ASTContext/

[aprantl]

lldb/source/Expression/Materializer.cpp

803

does this also need to be 7ull?

[davide]

lldb/source/Expression/Materializer.cpp

803

I'm going to move this to a function next to make sure we don't have to hardcode this everywhere.

[aprantl]

Check LLVM's MathExtras.h, perhaps we already got one.

lldb/include/lldb/Symbol/ClangASTContext.h

public:
   GetBitSize(lldb::opaque_compiler_type_t type,
              ExecutionContextScope *exe_scope) override;
 
   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;
+  llvm::Optional<size_t>
+  GetTypeBitAlign(lldb::opaque_compiler_type_t type) override;
 
   uint32_t GetNumChildren(lldb::opaque_compiler_type_t type,
                           bool omit_empty_base_classes,
                           const ExecutionContext *exe_ctx) override;
 
   CompilerType GetBuiltinTypeByName(ConstString name) override;
 
   lldb::BasicType

lldb/include/lldb/Symbol/CompilerType.h

public:
   llvm::Optional<uint64_t> GetByteSize(ExecutionContextScope *exe_scope) const;
   /// Return the size of the type in bits.
   llvm::Optional<uint64_t> GetBitSize(ExecutionContextScope *exe_scope) const;
 
   lldb::Encoding GetEncoding(uint64_t &count) const;
 
   lldb::Format GetFormat() const;
 
-  size_t GetTypeBitAlign() const;
+  llvm::Optional<size_t> GetTypeBitAlign() const;
 
   uint32_t GetNumChildren(bool omit_empty_base_classes,
                           const ExecutionContext *exe_ctx) const;
 
   lldb::BasicType GetBasicTypeEnumeration() const;
 
   static lldb::BasicType GetBasicTypeEnumeration(ConstString name);
 

lldb/include/lldb/Symbol/TypeSystem.h

#endif
   virtual bool IsPointerOrReferenceType(lldb::opaque_compiler_type_t type,
                                         CompilerType *pointee_type) = 0;
 
   virtual unsigned GetTypeQualifiers(lldb::opaque_compiler_type_t type) = 0;
 
   virtual bool IsCStringType(lldb::opaque_compiler_type_t type,
                              uint32_t &length) = 0;
 
-  virtual size_t GetTypeBitAlign(lldb::opaque_compiler_type_t type) = 0;
+  virtual llvm::Optional<size_t>
+  GetTypeBitAlign(lldb::opaque_compiler_type_t type) = 0;
 
   virtual CompilerType GetBasicTypeFromAST(lldb::BasicType basic_type) = 0;
 
   virtual CompilerType
   GetBuiltinTypeForEncodingAndBitSize(lldb::Encoding encoding,
                                       size_t bit_size) = 0;
 
   virtual bool IsBeingDefined(lldb::opaque_compiler_type_t type) = 0;

lldb/source/Expression/Materializer.cpp

if (m_is_reference) {
                 ") is larger than the ValueObject's size (%" PRIu64 ")",
                 m_variable_sp->GetName().AsCString(),
                 m_variable_sp->GetType()->GetByteSize().getValueOr(0),
                 data.GetByteSize());
           }
           return;
         }
 
-        size_t bit_align =
+        llvm::Optional<size_t> opt_bit_align =
             m_variable_sp->GetType()->GetLayoutCompilerType().GetTypeBitAlign();
-        size_t byte_align = (bit_align + 7) / 8;
+        if (!opt_bit_align) {
+          err.SetErrorStringWithFormat("can't get the type alignment for %s",
+                                       m_variable_sp->GetName().AsCString());
+          return;
+        }
 
-        if (!byte_align)
-          byte_align = 1;
+        size_t byte_align = (*opt_bit_align + 7) / 8;
 
         Status alloc_error;
         const bool zero_memory = false;
 
         m_temporary_allocation = map.Malloc(
             data.GetByteSize(), byte_align,
             lldb::ePermissionsReadable | lldb::ePermissionsWritable,
             IRMemoryMap::eAllocationPolicyMirror, zero_memory, alloc_error);
if (!m_is_program_reference) {
 
       ExecutionContextScope *exe_scope = map.GetBestExecutionContextScope();
 
       llvm::Optional<uint64_t> byte_size = m_type.GetByteSize(exe_scope);
       if (!byte_size) {
         err.SetErrorString("can't get size of type");
         return;
       }
-      size_t bit_align = m_type.GetTypeBitAlign();
-      size_t byte_align = (bit_align + 7) / 8;
 
-      if (!byte_align)
-        byte_align = 1;
+      llvm::Optional<size_t> opt_bit_align =
+          m_variable_sp->GetType()->GetLayoutCompilerType().GetTypeBitAlign();
+      if (!opt_bit_align) {
+        err.SetErrorStringWithFormat("can't get the type alignment for %s",
+                                     m_variable_sp->GetName().AsCString());
+        return;
+      }
+
+      size_t byte_align = (*opt_bit_align + 7) / 8;
 
       Status alloc_error;
       const bool zero_memory = true;
 
       m_temporary_allocation = map.Malloc(
           *byte_size, byte_align,
           lldb::ePermissionsReadable | lldb::ePermissionsWritable,
           IRMemoryMap::eAllocationPolicyMirror, zero_memory, alloc_error);

lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp

if (name.startswith("$")) {
       value_type = PointerType::get(global_variable->getType(), 0);
     } else {
       value_type = global_variable->getType();
     }
 
     llvm::Optional<uint64_t> value_size = compiler_type.GetByteSize(nullptr);
     if (!value_size)
       return false;
-    lldb::offset_t value_alignment =
-        (compiler_type.GetTypeBitAlign() + 7ull) / 8ull;
+    llvm::Optional<size_t> opt_alignment = compiler_type.GetTypeBitAlign();
+    if (!opt_alignment)
+      return false;
+    lldb::offset_t value_alignment = (*opt_alignment + 7ull) / 8ull;
 
     LLDB_LOG(log,
              "Type of \"{0}\" is [clang \"{1}\", llvm \"{2}\"] [size {3}, "
              "align {4}]",
              name,
              lldb_private::ClangUtil::GetQualType(compiler_type).getAsString(),
              PrintType(value_type), *value_size, value_alignment);
 

lldb/source/Symbol/ClangASTContext.cpp

default:
         return bit_size;
       if (bit_size)
         return bit_size;
     }
   }
   return None;
 }
 
-size_t ClangASTContext::GetTypeBitAlign(lldb::opaque_compiler_type_t type) {
+llvm::Optional<size_t>
+ClangASTContext::GetTypeBitAlign(lldb::opaque_compiler_type_t type) {
   if (GetCompleteType(type))
     return getASTContext()->getTypeAlign(GetQualType(type));
-  return 0;
+  return {};
 }
 
 lldb::Encoding ClangASTContext::GetEncoding(lldb::opaque_compiler_type_t type,
                                             uint64_t &count) {
   if (!type)
     return lldb::eEncodingInvalid;
 
   count = 1;

lldb/source/Symbol/CompilerType.cpp

 
 llvm::Optional<uint64_t>
 CompilerType::GetByteSize(ExecutionContextScope *exe_scope) const {
   if (llvm::Optional<uint64_t> bit_size = GetBitSize(exe_scope))
     return (*bit_size + 7) / 8;
   return {};
 }
 
-size_t CompilerType::GetTypeBitAlign() const {
+llvm::Optional<size_t> CompilerType::GetTypeBitAlign() const {
   if (IsValid())
     return m_type_system->GetTypeBitAlign(m_type);
-  return 0;
+  return {};
 }
 
 lldb::Encoding CompilerType::GetEncoding(uint64_t &count) const {
   if (!IsValid())
     return lldb::eEncodingInvalid;
 
   return m_type_system->GetEncoding(m_type, count);
 }