- 
                Notifications
    
You must be signed in to change notification settings  - Fork 15.1k
 
[lldb][TypeSystem] Better support for _BitInt types #165689
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| 
          
 @llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesDepends on: This patch ensures we make use of the  For DWARF from older versions of Clang that didn't emit a  Before: After: Full diff: https://github.com/llvm/llvm-project/pull/165689.diff 5 Files Affected: 
 diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 82e9d867c3ac0..56cd592418328 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -450,6 +450,10 @@ ParsedDWARFTypeAttributes::ParsedDWARFTypeAttributes(const DWARFDIE &die) {
       byte_size = form_value.Unsigned();
       break;
 
+    case DW_AT_bit_size:
+      data_bit_size = form_value.Unsigned();
+      break;
+
     case DW_AT_alignment:
       alignment = form_value.Unsigned();
       break;
@@ -810,13 +814,17 @@ DWARFASTParserClang::ParseTypeModifier(const SymbolContext &sc,
     // there...
     [[fallthrough]];
 
-  case DW_TAG_base_type:
+  case DW_TAG_base_type: {
     resolve_state = Type::ResolveState::Full;
+    // If a builtin type's size isn't a multiple of a byte, DWARF producers may
+    // add a precise bit-size to the type. Use the most precise bit-size
+    // possible.
+    uint64_t bit_size = attrs.data_bit_size ? *attrs.data_bit_size
+                                            : attrs.byte_size.value_or(0) * 8;
     clang_type = m_ast.GetBuiltinTypeForDWARFEncodingAndBitSize(
-        attrs.name.GetStringRef(), attrs.encoding,
-        attrs.byte_size.value_or(0) * 8);
+        attrs.name.GetStringRef(), attrs.encoding, bit_size);
     break;
-
+  }
   case DW_TAG_pointer_type:
     encoding_data_type = Type::eEncodingIsPointerUID;
     break;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index da58f4c146226..f5f707129d67d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -574,6 +574,7 @@ struct ParsedDWARFTypeAttributes {
   lldb_private::plugin::dwarf::DWARFFormValue type;
   lldb::LanguageType class_language = lldb::eLanguageTypeUnknown;
   std::optional<uint64_t> byte_size;
+  std::optional<uint64_t> data_bit_size;
   std::optional<uint64_t> alignment;
   size_t calling_convention = llvm::dwarf::DW_CC_normal;
   uint32_t bit_stride = 0;
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 82dfe7e540717..c8057edaa4aeb 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -1000,6 +1000,8 @@ CompilerType TypeSystemClang::GetBuiltinTypeForDWARFEncodingAndBitSize(
 
   case DW_ATE_signed:
     if (!type_name.empty()) {
+      if (type_name.starts_with("_BitInt"))
+        return GetType(ast.getBitIntType(/*Unsigned=*/false, bit_size));
       if (type_name == "wchar_t" &&
           QualTypeMatchesBitSize(bit_size, ast, ast.WCharTy) &&
           (getTargetInfo() &&
@@ -1056,6 +1058,8 @@ CompilerType TypeSystemClang::GetBuiltinTypeForDWARFEncodingAndBitSize(
 
   case DW_ATE_unsigned:
     if (!type_name.empty()) {
+      if (type_name.starts_with("unsigned _BitInt"))
+        return GetType(ast.getBitIntType(/*Unsigned=*/true, bit_size));
       if (type_name == "wchar_t") {
         if (QualTypeMatchesBitSize(bit_size, ast, ast.WCharTy)) {
           if (!(getTargetInfo() &&
@@ -3888,6 +3892,13 @@ TypeSystemClang::GetTypeInfo(lldb::opaque_compiler_type_t type,
                            ->getModifiedType()
                            .getAsOpaquePtr(),
                        pointee_or_element_clang_type);
+  case clang::Type::BitInt: {
+    uint32_t type_flags = eTypeIsScalar | eTypeIsInteger | eTypeHasValue;
+    if (qual_type->isSignedIntegerType())
+      type_flags |= eTypeIsSigned;
+
+    return type_flags;
+  }
   case clang::Type::Builtin: {
     const clang::BuiltinType *builtin_type =
         llvm::cast<clang::BuiltinType>(qual_type->getCanonicalTypeInternal());
diff --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
index 1981e912fa4fa..b2fb0814b70f9 100644
--- a/lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -52,6 +52,12 @@ class TestTypeSystemClang : public testing::Test {
     return ClangUtil::GetQualType(
         m_ast->GetBuiltinTypeByName(ConstString(name)));
   }
+
+  CompilerType GetBuiltinTypeForDWARFEncodingAndBitSize(
+      llvm::StringRef type_name, uint32_t encoding, uint32_t bit_size) const {
+    return m_ast->GetBuiltinTypeForDWARFEncodingAndBitSize(type_name, encoding,
+                                                           bit_size);
+  }
 };
 
 TEST_F(TestTypeSystemClang, TestGetBasicTypeFromEnum) {
@@ -238,6 +244,75 @@ TEST_F(TestTypeSystemClang, TestBuiltinTypeForEncodingAndBitSize) {
   VerifyEncodingAndBitSize(*m_ast, eEncodingIEEE754, 64);
 }
 
+TEST_F(TestTypeSystemClang, TestGetBuiltinTypeForDWARFEncodingAndBitSize) {
+  EXPECT_FALSE(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                   "_BitIn", llvm::dwarf::DW_ATE_signed, 2)
+                   .IsValid());
+  EXPECT_FALSE(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                   "BitInt", llvm::dwarf::DW_ATE_signed, 2)
+                   .IsValid());
+  EXPECT_FALSE(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                   "_BitInt(2)", llvm::dwarf::DW_ATE_signed_char, 2)
+                   .IsValid());
+  EXPECT_FALSE(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                   "_BitInt", llvm::dwarf::DW_ATE_signed_char, 2)
+                   .IsValid());
+  EXPECT_FALSE(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                   "_BitInt(2)", llvm::dwarf::DW_ATE_unsigned, 2)
+                   .IsValid());
+  EXPECT_FALSE(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                   "_BitInt", llvm::dwarf::DW_ATE_unsigned, 2)
+                   .IsValid());
+
+  EXPECT_EQ(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                "_BitInt(2)", llvm::dwarf::DW_ATE_signed, 2)
+                .GetTypeName(),
+            "_BitInt(2)");
+  EXPECT_EQ(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                "_BitInt", llvm::dwarf::DW_ATE_signed, 2)
+                .GetTypeName(),
+            "_BitInt(2)");
+
+  EXPECT_FALSE(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                   "unsigned _BitIn", llvm::dwarf::DW_ATE_unsigned, 2)
+                   .IsValid());
+  EXPECT_FALSE(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                   "unsigned BitInt", llvm::dwarf::DW_ATE_unsigned, 2)
+                   .IsValid());
+  EXPECT_FALSE(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                   "unsigned _BitInt(2)", llvm::dwarf::DW_ATE_unsigned_char, 2)
+                   .IsValid());
+  EXPECT_FALSE(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                   "unsigned _BitInt", llvm::dwarf::DW_ATE_unsigned_char, 2)
+                   .IsValid());
+  EXPECT_FALSE(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                   "unsigned _BitInt(2)", llvm::dwarf::DW_ATE_signed, 2)
+                   .IsValid());
+  EXPECT_FALSE(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                   "unsigned _BitInt", llvm::dwarf::DW_ATE_signed, 2)
+                   .IsValid());
+
+  EXPECT_EQ(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                "unsigned _BitInt(2)", llvm::dwarf::DW_ATE_unsigned, 2)
+                .GetTypeName(),
+            "unsigned _BitInt(2)");
+  EXPECT_EQ(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                "unsigned _BitInt", llvm::dwarf::DW_ATE_unsigned, 2)
+                .GetTypeName(),
+            "unsigned _BitInt(2)");
+}
+
+TEST_F(TestTypeSystemClang, TestBitIntTypeInfo) {
+  EXPECT_EQ(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                "_BitInt", llvm::dwarf::DW_ATE_signed, 2)
+                .GetTypeInfo(),
+            eTypeIsSigned | eTypeIsScalar | eTypeHasValue | eTypeIsInteger);
+  EXPECT_EQ(GetBuiltinTypeForDWARFEncodingAndBitSize(
+                "unsigned _BitInt", llvm::dwarf::DW_ATE_unsigned, 2)
+                .GetTypeInfo(),
+            eTypeIsScalar | eTypeHasValue | eTypeIsInteger);
+}
+
 TEST_F(TestTypeSystemClang, TestBuiltinTypeForEmptyTriple) {
   // Test that we can access type-info of builtin Clang AST
   // types without crashing even when the target triple is
diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
index 0cae01de2902a..e68ab87a87316 100644
--- a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
@@ -1617,3 +1617,310 @@ TEST_F(DWARFASTParserClangTests, TestObjectPointer_IndexEncoding) {
     EXPECT_EQ(param_die, ast_parser.GetObjectParameter(sub2, context_die));
   }
 }
+
+TEST_F(DWARFASTParserClangTests, TestTypeBitSize) {
+  // Tests that we correctly parse DW_AT_bit_size of a DW_AT_base_type.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_AARCH64
+DWARF:
+  debug_str:
+    - _BitInt(2)
+  debug_abbrev:
+    - ID:              0
+      Table:
+        - Code:            0x1
+          Tag:             DW_TAG_compile_unit
+          Children:        DW_CHILDREN_yes
+          Attributes:
+            - Attribute:       DW_AT_language
+              Form:            DW_FORM_data2
+        - Code:            0x2
+          Tag:             DW_TAG_base_type
+          Children:        DW_CHILDREN_no
+          Attributes:
+            - Attribute: DW_AT_name
+              Form:      DW_FORM_strp
+            - Attribute: DW_AT_encoding
+              Form:      DW_FORM_data1
+            - Attribute: DW_AT_byte_size
+              Form:      DW_FORM_data1
+            - Attribute: DW_AT_bit_size
+              Form:      DW_FORM_data1
+
+  debug_info:
+     - Version:  5
+       UnitType: DW_UT_compile
+       AddrSize: 8
+       Entries:
+
+# DW_TAG_compile_unit
+#   DW_AT_language [DW_FORM_data2]    (DW_LANG_C_plus_plus)
+
+        - AbbrCode: 0x1
+          Values:
+            - Value: 0x04
+
+#   DW_TAG_base_type
+#     DW_AT_name [DW_FORM_strp] ('_BitInt(2)')
+
+        - AbbrCode: 0x2
+          Values:
+            - Value: 0x0
+            - Value: 0x3e
+            - Value: 0x01
+            - Value: 0x02
+...
+)";
+
+  YAMLModuleTester t(yamldata);
+
+  DWARFUnit *unit = t.GetDwarfUnit();
+  ASSERT_NE(unit, nullptr);
+  const DWARFDebugInfoEntry *cu_entry = unit->DIE().GetDIE();
+  ASSERT_EQ(cu_entry->Tag(), DW_TAG_compile_unit);
+  ASSERT_EQ(unit->GetDWARFLanguageType(), DW_LANG_C_plus_plus);
+  DWARFDIE cu_die(unit, cu_entry);
+
+  auto holder = std::make_unique<clang_utils::TypeSystemClangHolder>("ast");
+  auto &ast_ctx = *holder->GetAST();
+  DWARFASTParserClangStub ast_parser(ast_ctx);
+
+  auto type_die = cu_die.GetFirstChild();
+  ASSERT_TRUE(type_die.IsValid());
+  ASSERT_EQ(type_die.Tag(), DW_TAG_base_type);
+
+  ParsedDWARFTypeAttributes attrs(type_die);
+  EXPECT_EQ(attrs.byte_size.value_or(0), 1U);
+  EXPECT_EQ(attrs.data_bit_size.value_or(0), 2U);
+
+  SymbolContext sc;
+  auto type_sp =
+      ast_parser.ParseTypeFromDWARF(sc, type_die, /*type_is_new_ptr=*/nullptr);
+  ASSERT_NE(type_sp, nullptr);
+
+  EXPECT_EQ(llvm::expectedToOptional(type_sp->GetByteSize(nullptr)).value_or(0),
+            1U);
+}
+
+TEST_F(DWARFASTParserClangTests, TestBitIntParsing) {
+  // Tests that we correctly parse the DW_AT_base_type for a _BitInt.
+  // Older versions of Clang only emit the `_BitInt` string into the
+  // DW_AT_name (not including the bitsize). Make sure we understand
+  // those too.
+
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_AARCH64
+DWARF:
+  debug_str:
+    - _BitInt(2)
+    - _BitInt
+    - unsigned _BitInt(2)
+    - unsigned _BitInt
+  debug_abbrev:
+    - ID:              0
+      Table:
+        - Code:            0x1
+          Tag:             DW_TAG_compile_unit
+          Children:        DW_CHILDREN_yes
+          Attributes:
+            - Attribute:       DW_AT_language
+              Form:            DW_FORM_data2
+        - Code:            0x2
+          Tag:             DW_TAG_base_type
+          Children:        DW_CHILDREN_no
+          Attributes:
+            - Attribute: DW_AT_name
+              Form:      DW_FORM_strp
+            - Attribute: DW_AT_encoding
+              Form:      DW_FORM_data1
+            - Attribute: DW_AT_byte_size
+              Form:      DW_FORM_data1
+            - Attribute: DW_AT_bit_size
+              Form:      DW_FORM_data1
+        - Code:            0x3
+          Tag:             DW_TAG_base_type
+          Children:        DW_CHILDREN_no
+          Attributes:
+            - Attribute: DW_AT_name
+              Form:      DW_FORM_strp
+            - Attribute: DW_AT_encoding
+              Form:      DW_FORM_data1
+            - Attribute: DW_AT_byte_size
+              Form:      DW_FORM_data1
+
+  debug_info:
+     - Version:  5
+       UnitType: DW_UT_compile
+       AddrSize: 8
+       Entries:
+
+# DW_TAG_compile_unit
+#   DW_AT_language [DW_FORM_data2]    (DW_LANG_C_plus_plus)
+
+        - AbbrCode: 0x1
+          Values:
+            - Value: 0x04
+
+#   DW_TAG_base_type
+#     DW_AT_name [DW_FORM_strp] ('_BitInt(2)')
+
+        - AbbrCode: 0x2
+          Values:
+            - Value: 0x0
+            - Value: 0x05
+            - Value: 0x01
+            - Value: 0x02
+
+#   DW_TAG_base_type
+#     DW_AT_name [DW_FORM_strp] ('_BitInt')
+
+        - AbbrCode: 0x2
+          Values:
+            - Value: 0x0b
+            - Value: 0x05
+            - Value: 0x08
+            - Value: 0x34
+
+#   DW_TAG_base_type
+#     DW_AT_name [DW_FORM_strp] ('unsigned _BitInt(2)')
+
+        - AbbrCode: 0x2
+          Values:
+            - Value: 0x13
+            - Value: 0x07
+            - Value: 0x01
+            - Value: 0x02
+
+#   DW_TAG_base_type
+#     DW_AT_name [DW_FORM_strp] ('unsigned _BitInt')
+
+        - AbbrCode: 0x2
+          Values:
+            - Value: 0x27
+            - Value: 0x07
+            - Value: 0x08
+            - Value: 0x34
+
+#   DW_TAG_base_type
+#     DW_AT_name [DW_FORM_strp] ('_BitInt')
+
+        - AbbrCode: 0x3
+          Values:
+            - Value: 0x0b
+            - Value: 0x05
+            - Value: 0x08
+...
+
+)";
+
+  YAMLModuleTester t(yamldata);
+
+  DWARFUnit *unit = t.GetDwarfUnit();
+  ASSERT_NE(unit, nullptr);
+  const DWARFDebugInfoEntry *cu_entry = unit->DIE().GetDIE();
+  ASSERT_EQ(cu_entry->Tag(), DW_TAG_compile_unit);
+  ASSERT_EQ(unit->GetDWARFLanguageType(), DW_LANG_C_plus_plus);
+  DWARFDIE cu_die(unit, cu_entry);
+
+  auto holder = std::make_unique<clang_utils::TypeSystemClangHolder>("ast");
+  auto &ast_ctx = *holder->GetAST();
+  DWARFASTParserClangStub ast_parser(ast_ctx);
+
+  auto type_die = cu_die.GetFirstChild();
+  ASSERT_TRUE(type_die.IsValid());
+
+  {
+    SymbolContext sc;
+    auto type_sp = ast_parser.ParseTypeFromDWARF(sc, type_die,
+                                                 /*type_is_new_ptr=*/nullptr);
+    ASSERT_NE(type_sp, nullptr);
+
+    EXPECT_EQ(
+        llvm::expectedToOptional(type_sp->GetByteSize(nullptr)).value_or(0),
+        1U);
+    uint64_t count;
+    EXPECT_EQ(type_sp->GetEncoding(count), lldb::eEncodingSint);
+    EXPECT_EQ(type_sp->GetName(), "_BitInt(2)");
+    EXPECT_EQ(type_sp->GetForwardCompilerType().GetTypeName(), "_BitInt(2)");
+  }
+
+  {
+    type_die = type_die.GetSibling();
+    SymbolContext sc;
+    auto type_sp = ast_parser.ParseTypeFromDWARF(sc, type_die,
+                                                 /*type_is_new_ptr=*/nullptr);
+    ASSERT_NE(type_sp, nullptr);
+
+    EXPECT_EQ(
+        llvm::expectedToOptional(type_sp->GetByteSize(nullptr)).value_or(0),
+        8U);
+    uint64_t count;
+    EXPECT_EQ(type_sp->GetEncoding(count), lldb::eEncodingSint);
+    EXPECT_EQ(type_sp->GetName(), "_BitInt");
+    EXPECT_EQ(type_sp->GetForwardCompilerType().GetTypeName(), "_BitInt(52)");
+  }
+
+  {
+    type_die = type_die.GetSibling();
+    SymbolContext sc;
+    auto type_sp = ast_parser.ParseTypeFromDWARF(sc, type_die,
+                                                 /*type_is_new_ptr=*/nullptr);
+    ASSERT_NE(type_sp, nullptr);
+
+    EXPECT_EQ(
+        llvm::expectedToOptional(type_sp->GetByteSize(nullptr)).value_or(0),
+        1U);
+    uint64_t count;
+    EXPECT_EQ(type_sp->GetEncoding(count), lldb::eEncodingUint);
+    EXPECT_EQ(type_sp->GetName(), "unsigned _BitInt(2)");
+    EXPECT_EQ(type_sp->GetForwardCompilerType().GetTypeName(),
+              "unsigned _BitInt(2)");
+  }
+
+  {
+    type_die = type_die.GetSibling();
+    SymbolContext sc;
+    auto type_sp = ast_parser.ParseTypeFromDWARF(sc, type_die,
+                                                 /*type_is_new_ptr=*/nullptr);
+    ASSERT_NE(type_sp, nullptr);
+
+    EXPECT_EQ(
+        llvm::expectedToOptional(type_sp->GetByteSize(nullptr)).value_or(0),
+        8U);
+    uint64_t count;
+    EXPECT_EQ(type_sp->GetEncoding(count), lldb::eEncodingUint);
+    EXPECT_EQ(type_sp->GetName(), "unsigned _BitInt");
+    EXPECT_EQ(type_sp->GetForwardCompilerType().GetTypeName(),
+              "unsigned _BitInt(52)");
+  }
+
+  {
+    type_die = type_die.GetSibling();
+    SymbolContext sc;
+    auto type_sp = ast_parser.ParseTypeFromDWARF(sc, type_die,
+                                                 /*type_is_new_ptr=*/nullptr);
+    ASSERT_NE(type_sp, nullptr);
+
+    EXPECT_EQ(
+        llvm::expectedToOptional(type_sp->GetByteSize(nullptr)).value_or(0),
+        8U);
+    uint64_t count;
+    EXPECT_EQ(type_sp->GetEncoding(count), lldb::eEncodingSint);
+    EXPECT_EQ(type_sp->GetName(), "_BitInt");
+
+    // Older versions of Clang didn't emit a DW_AT_bit_size for _BitInt. In
+    // those cases we would format the CompilerType name using the byte-size.
+    EXPECT_EQ(type_sp->GetForwardCompilerType().GetTypeName(), "_BitInt(64)");
+  }
+}
 | 
    
81e1e29    to
    9c68ad5      
    Compare
  
    | // If a builtin type's size isn't a multiple of a byte, DWARF producers may | ||
| // add a precise bit-size to the type. Use the most precise bit-size | ||
| // possible. | ||
| uint64_t bit_size = attrs.data_bit_size ? *attrs.data_bit_size | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: const
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every time I try to make a local variable const I get push-back saying that's not LLVM-style. Though I couldn't find any such statements in the style guide. I think the reasoning for avoiding const is that it makes the reader question, "why isn't that other variable const". By that logic, if we wanted const local variables, we'd have to do them in bulk for the entire function. I'm not sure where I stand on that, I think I prefer making new variables const when possible 🤷♂️
But I agree with you that these particular cases have a small enough scope that const should be uncontroversial.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's incredibly strange... I've never encountered that feedback myself. I'll suggest const anywhere and everywhere something does not need to change.
| .getAsOpaquePtr(), | ||
| pointee_or_element_clang_type); | ||
| case clang::Type::BitInt: { | ||
| uint32_t type_flags = eTypeIsScalar | eTypeIsInteger | eTypeHasValue; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: const
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't make this one const cause it's modified conditionally few lines down
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable.
This patch ensures we make use of the `DW_AT_bit_size` on `DW_TAG_base_type`s (which since llvm#164372 can exist on `_BitInt`s) and adjusts `TypeSystemClang` to recognize `_BitInt`. For DWARF from older versions of Clang that didn't emit a `DW_AT_bit_size`, we would create `_BitInt`s using the byte-size. Not sure we can do much better than that. But the situation beforehand wasn't much better. Before: ``` (lldb) v (char) a = '\x01' (unsigned char) b = '\x01' (long) c = 2 (unsigned long) d = 2 ``` After: ``` (lldb) v (_BitInt(2)) a = 1 (unsigned _BitInt(2)) b = 1 (_BitInt(52)) c = 2 (unsigned _BitInt(52)) d = 2 ```
c68dbc4    to
    5e85563      
    Compare
  
    | 
           LLVM Buildbot has detected a new failure on builder  Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/34289 Here is the relevant piece of the build log for the reference | 
    
| 
           Huh looks like I merged this (possibly using auto merge?) before Adrian/Alex could approve. @adrian-prantl @bulbazord let me know if you have any other concerns and i'll apply them retroactively  | 
    
* main: [SPIRV] Fix vector bitcast check in LegalizePointerCast (llvm#164997) [lldb][docs] Add troubleshooting section to scripting introduction [Sema] Fix parameter index checks on explicit object member functions (llvm#165586) To fix polymorphic pointer assignment in FORALL when LHS is unlimited polymorphic and RHS is intrinsic type target (llvm#164999) [CostModel][AArch64] Model cost of extract.last.active intrinsic (clastb) (llvm#165739) [MemProf] Select largest of matching contexts from profile (llvm#165338) [lldb][TypeSystem] Better support for _BitInt types (llvm#165689) [NVPTX] Move TMA G2S lowering to Tablegen (llvm#165710) [MLIR][NVVM] Extend NVVM mma ops to support fp64 (llvm#165380) [UTC] Support to test annotated IR (llvm#165419)
Depends on: * llvm#165686 This patch ensures we make use of the `DW_AT_bit_size` on `DW_TAG_base_type`s (which since llvm#164372 can exist on `_BitInt`s) and adjusts `TypeSystemClang` to recognize `_BitInt`. For DWARF from older versions of Clang that didn't emit a `DW_AT_bit_size`, we would create `_BitInt`s using the byte-size. Not sure we can do much better than that. But the situation beforehand wasn't much better. Before: ``` (lldb) v (char) a = '\x01' (unsigned char) b = '\x01' (long) c = 2 (unsigned long) d = 2 ``` After: ``` (lldb) v (_BitInt(2)) a = 1 (unsigned _BitInt(2)) b = 1 (_BitInt(52)) c = 2 (unsigned _BitInt(52)) d = 2 ``` Fixes llvm#110273
Depends on:
This patch ensures we make use of the
DW_AT_bit_sizeonDW_TAG_base_types (which since #164372 can exist on_BitInts) and adjustsTypeSystemClangto recognize_BitInt.For DWARF from older versions of Clang that didn't emit a
DW_AT_bit_size, we would create_BitInts using the byte-size. Not sure we can do much better than that. But the situation beforehand wasn't much better.Before:
After:
Fixes #110273