[Lldb-commits] [lldb] r319414 - Fix assertion in ClangASTContext

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Thu Nov 30 02:16:54 PST 2017


Author: labath
Date: Thu Nov 30 02:16:54 2017
New Revision: 319414

URL: http://llvm.org/viewvc/llvm-project?rev=319414&view=rev
Log:
Fix assertion in ClangASTContext

Summary:
llvm::APSInt(0) asserts because it creates an int with bit-width 0 and
not (as I thought) a value 0.

Theoretically it should be sufficient to change this to APSInt(1), as
the intention there was that the value of the first argument should be
ignored if the type is invalid, but that would look dodgy.

Instead, I use llvm::Optional to denote an invalid value and use a
special struct instead of a std::pair, to reduce typing and increase
clarity.

Reviewers: clayborg

Subscribers: lldb-commits

Differential Revision: https://reviews.llvm.org/D40615

Modified:
    lldb/trunk/include/lldb/Symbol/ClangASTContext.h
    lldb/trunk/include/lldb/Symbol/CompilerType.h
    lldb/trunk/include/lldb/Symbol/TypeSystem.h
    lldb/trunk/source/API/SBType.cpp
    lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
    lldb/trunk/source/Symbol/ClangASTContext.cpp
    lldb/trunk/source/Symbol/CompilerType.cpp
    lldb/trunk/source/Symbol/TypeSystem.cpp
    lldb/trunk/unittests/Symbol/TestClangASTContext.cpp

Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=319414&r1=319413&r2=319414&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Thu Nov 30 02:16:54 2017
@@ -787,7 +787,7 @@ public:
                           size_t idx) override;
   CompilerType GetTypeTemplateArgument(lldb::opaque_compiler_type_t type,
                                        size_t idx) override;
-  std::pair<llvm::APSInt, CompilerType>
+  llvm::Optional<CompilerType::IntegralTemplateArgument>
   GetIntegralTemplateArgument(lldb::opaque_compiler_type_t type,
                               size_t idx) override;
 

Modified: lldb/trunk/include/lldb/Symbol/CompilerType.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/CompilerType.h?rev=319414&r1=319413&r2=319414&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Symbol/CompilerType.h (original)
+++ lldb/trunk/include/lldb/Symbol/CompilerType.h Thu Nov 30 02:16:54 2017
@@ -20,6 +20,7 @@
 // Project includes
 #include "lldb/Core/ClangForward.h"
 #include "lldb/lldb-private.h"
+#include "llvm/ADT/APSInt.h"
 
 namespace lldb_private {
 
@@ -290,6 +291,8 @@ public:
   // Exploring the type
   //----------------------------------------------------------------------
 
+  struct IntegralTemplateArgument;
+
   uint64_t GetByteSize(ExecutionContextScope *exe_scope) const;
 
   uint64_t GetBitSize(ExecutionContextScope *exe_scope) const;
@@ -368,9 +371,9 @@ public:
   lldb::TemplateArgumentKind GetTemplateArgumentKind(size_t idx) const;
   CompilerType GetTypeTemplateArgument(size_t idx) const;
 
-  // Returns the value of the template argument and its type. In case the
-  // argument is not found, returns an invalid CompilerType.
-  std::pair<llvm::APSInt, CompilerType> GetIntegralTemplateArgument(size_t idx) const;
+  // Returns the value of the template argument and its type.
+  llvm::Optional<IntegralTemplateArgument>
+  GetIntegralTemplateArgument(size_t idx) const;
 
   CompilerType GetTypeForFormatters() const;
 
@@ -433,6 +436,11 @@ private:
 bool operator==(const CompilerType &lhs, const CompilerType &rhs);
 bool operator!=(const CompilerType &lhs, const CompilerType &rhs);
 
+struct CompilerType::IntegralTemplateArgument {
+  llvm::APSInt value;
+  CompilerType type;
+};
+
 } // namespace lldb_private
 
 #endif // liblldb_CompilerType_h_

Modified: lldb/trunk/include/lldb/Symbol/TypeSystem.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/TypeSystem.h?rev=319414&r1=319413&r2=319414&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Symbol/TypeSystem.h (original)
+++ lldb/trunk/include/lldb/Symbol/TypeSystem.h Thu Nov 30 02:16:54 2017
@@ -355,9 +355,8 @@ public:
   GetTemplateArgumentKind(lldb::opaque_compiler_type_t type, size_t idx);
   virtual CompilerType GetTypeTemplateArgument(lldb::opaque_compiler_type_t type,
                                            size_t idx);
-  virtual std::pair<llvm::APSInt, CompilerType>
-  GetIntegralTemplateArgument(lldb::opaque_compiler_type_t type,
-                              size_t idx);
+  virtual llvm::Optional<CompilerType::IntegralTemplateArgument>
+  GetIntegralTemplateArgument(lldb::opaque_compiler_type_t type, size_t idx);
 
   //----------------------------------------------------------------------
   // Dumping types

Modified: lldb/trunk/source/API/SBType.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBType.cpp?rev=319414&r1=319413&r2=319414&view=diff
==============================================================================
--- lldb/trunk/source/API/SBType.cpp (original)
+++ lldb/trunk/source/API/SBType.cpp Thu Nov 30 02:16:54 2017
@@ -426,7 +426,7 @@ lldb::SBType SBType::GetTemplateArgument
     case eTemplateArgumentKindIntegral:
       type = m_opaque_sp->GetCompilerType(false)
                  .GetIntegralTemplateArgument(idx)
-                 .second;
+                 ->type;
       break;
     default:
       break;

Modified: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp?rev=319414&r1=319413&r2=319414&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp (original)
+++ lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp Thu Nov 30 02:16:54 2017
@@ -59,10 +59,8 @@ bool BitsetFrontEnd::Update() {
   size_t capping_size = target_sp->GetMaximumNumberOfChildrenToDisplay();
 
   size_t size = 0;
-  auto value_and_type =
-      m_backend.GetCompilerType().GetIntegralTemplateArgument(0);
-  if (value_and_type.second)
-    size = value_and_type.first.getLimitedValue(capping_size);
+  if (auto arg = m_backend.GetCompilerType().GetIntegralTemplateArgument(0))
+    size = arg->value.getLimitedValue(capping_size);
 
   m_elements.assign(size, ValueObjectSP());
 

Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=319414&r1=319413&r2=319414&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Thu Nov 30 02:16:54 2017
@@ -7650,21 +7650,21 @@ ClangASTContext::GetTypeTemplateArgument
   return CompilerType(getASTContext(), template_arg.getAsType());
 }
 
-std::pair<llvm::APSInt, CompilerType>
+llvm::Optional<CompilerType::IntegralTemplateArgument>
 ClangASTContext::GetIntegralTemplateArgument(lldb::opaque_compiler_type_t type,
                                              size_t idx) {
   const clang::ClassTemplateSpecializationDecl *template_decl =
       GetAsTemplateSpecialization(type);
   if (! template_decl || idx >= template_decl->getTemplateArgs().size())
-    return {llvm::APSInt(0), CompilerType()};
+    return llvm::None;
 
   const clang::TemplateArgument &template_arg =
       template_decl->getTemplateArgs()[idx];
   if (template_arg.getKind() != clang::TemplateArgument::Integral)
-    return {llvm::APSInt(0), CompilerType()};
+    return llvm::None;
 
-  return {template_arg.getAsIntegral(),
-          CompilerType(getASTContext(), template_arg.getIntegralType())};
+  return {{template_arg.getAsIntegral(),
+           CompilerType(getASTContext(), template_arg.getIntegralType())}};
 }
 
 CompilerType ClangASTContext::GetTypeForFormatters(void *type) {

Modified: lldb/trunk/source/Symbol/CompilerType.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/CompilerType.cpp?rev=319414&r1=319413&r2=319414&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/CompilerType.cpp (original)
+++ lldb/trunk/source/Symbol/CompilerType.cpp Thu Nov 30 02:16:54 2017
@@ -703,12 +703,11 @@ CompilerType CompilerType::GetTypeTempla
   return CompilerType();
 }
 
-std::pair<llvm::APSInt, CompilerType>
-CompilerType::GetIntegralTemplateArgument(size_t idx) const
-{
+llvm::Optional<CompilerType::IntegralTemplateArgument>
+CompilerType::GetIntegralTemplateArgument(size_t idx) const {
   if (IsValid())
     return m_type_system->GetIntegralTemplateArgument(m_type, idx);
-  return {llvm::APSInt(0), CompilerType()};
+  return llvm::None;
 }
 
 CompilerType CompilerType::GetTypeForFormatters() const {

Modified: lldb/trunk/source/Symbol/TypeSystem.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/TypeSystem.cpp?rev=319414&r1=319413&r2=319414&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/TypeSystem.cpp (original)
+++ lldb/trunk/source/Symbol/TypeSystem.cpp Thu Nov 30 02:16:54 2017
@@ -111,10 +111,10 @@ CompilerType TypeSystem::GetTypeTemplate
   return CompilerType();
 }
 
-std::pair<llvm::APSInt, CompilerType>
+llvm::Optional<CompilerType::IntegralTemplateArgument>
 TypeSystem::GetIntegralTemplateArgument(opaque_compiler_type_t type,
-                            size_t idx) {
-  return {llvm::APSInt(0), CompilerType()};
+                                        size_t idx) {
+  return llvm::None;
 }
 
 LazyBool TypeSystem::ShouldPrintAsOneLiner(void *type, ValueObject *valobj) {

Modified: lldb/trunk/unittests/Symbol/TestClangASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Symbol/TestClangASTContext.cpp?rev=319414&r1=319413&r2=319414&view=diff
==============================================================================
--- lldb/trunk/unittests/Symbol/TestClangASTContext.cpp (original)
+++ lldb/trunk/unittests/Symbol/TestClangASTContext.cpp Thu Nov 30 02:16:54 2017
@@ -382,8 +382,8 @@ TEST_F(TestClangASTContext, TemplateArgu
   infos.names.push_back("T");
   infos.args.push_back(TemplateArgument(m_ast->getASTContext()->IntTy));
   infos.names.push_back("I");
-  infos.args.push_back(TemplateArgument(*m_ast->getASTContext(),
-                                        llvm::APSInt(47),
+  llvm::APSInt arg(llvm::APInt(8, 47));
+  infos.args.push_back(TemplateArgument(*m_ast->getASTContext(), arg,
                                         m_ast->getASTContext()->IntTy));
 
   // template<typename T, int I> struct foo;
@@ -419,15 +419,16 @@ TEST_F(TestClangASTContext, TemplateArgu
               eTemplateArgumentKindType);
     EXPECT_EQ(m_ast->GetTypeTemplateArgument(t.GetOpaqueQualType(), 0),
               int_type);
-    auto p = m_ast->GetIntegralTemplateArgument(t.GetOpaqueQualType(), 0);
-    EXPECT_EQ(p.second, CompilerType());
+    EXPECT_EQ(llvm::None,
+              m_ast->GetIntegralTemplateArgument(t.GetOpaqueQualType(), 0));
 
     EXPECT_EQ(m_ast->GetTemplateArgumentKind(t.GetOpaqueQualType(), 1),
               eTemplateArgumentKindIntegral);
     EXPECT_EQ(m_ast->GetTypeTemplateArgument(t.GetOpaqueQualType(), 1),
               CompilerType());
-    p = m_ast->GetIntegralTemplateArgument(t.GetOpaqueQualType(), 1);
-    EXPECT_EQ(p.first, llvm::APSInt(47));
-    EXPECT_EQ(p.second, int_type);
+    auto result = m_ast->GetIntegralTemplateArgument(t.GetOpaqueQualType(), 1);
+    ASSERT_NE(llvm::None, result);
+    EXPECT_EQ(arg, result->value);
+    EXPECT_EQ(int_type, result->type);
   }
 }




More information about the lldb-commits mailing list