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

git-svn-id: https://llvm.org/svn/llvm-project/lldb/trunk@319414 91177308-0d34-0410-b5e6-96231b3b80d8

9 files changed, 36 insertions, 31 deletions

diff --git a/include/lldb/Symbol/ClangASTContext.h b/include/lldb/Symbol/ClangASTContext.h
index e97f3a094..2d2c804eb 100644
--- a/include/lldb/Symbol/ClangASTContext.h
+++ b/include/lldb/Symbol/ClangASTContext.h
@@ -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;
 
diff --git a/include/lldb/Symbol/CompilerType.h b/include/lldb/Symbol/CompilerType.h
index bac6e4492..70d56db7f 100644
--- a/include/lldb/Symbol/CompilerType.h
+++ b/include/lldb/Symbol/CompilerType.h
@@ -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_
diff --git a/include/lldb/Symbol/TypeSystem.h b/include/lldb/Symbol/TypeSystem.h
index 440f3a43e..ff85d8428 100644
--- a/include/lldb/Symbol/TypeSystem.h
+++ b/include/lldb/Symbol/TypeSystem.h
@@ -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
diff --git a/source/API/SBType.cpp b/source/API/SBType.cpp
index b0357da5c..80d48754c 100644
--- a/source/API/SBType.cpp
+++ b/source/API/SBType.cpp
@@ -426,7 +426,7 @@ lldb::SBType SBType::GetTemplateArgumentType(uint32_t idx) {
     case eTemplateArgumentKindIntegral:
       type = m_opaque_sp->GetCompilerType(false)
                  .GetIntegralTemplateArgument(idx)
-                 .second;
+                 ->type;
       break;
     default:
       break;
diff --git a/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp b/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
index 282a8de9c..0cdb0b26c 100644
--- a/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
+++ b/source/Plugins/Language/CPlusPlus/LibCxxBitset.cpp
@@ -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());
 
diff --git a/source/Symbol/ClangASTContext.cpp b/source/Symbol/ClangASTContext.cpp
index 623d6f84c..9d907b161 100644
--- a/source/Symbol/ClangASTContext.cpp
+++ b/source/Symbol/ClangASTContext.cpp
@@ -7650,21 +7650,21 @@ ClangASTContext::GetTypeTemplateArgument(lldb::opaque_compiler_type_t type,
   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) {
diff --git a/source/Symbol/CompilerType.cpp b/source/Symbol/CompilerType.cpp
index 6747c051a..5d845361b 100644
--- a/source/Symbol/CompilerType.cpp
+++ b/source/Symbol/CompilerType.cpp
@@ -703,12 +703,11 @@ CompilerType CompilerType::GetTypeTemplateArgument(size_t idx) const {
   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 {
diff --git a/source/Symbol/TypeSystem.cpp b/source/Symbol/TypeSystem.cpp
index ada9b4e21..b99f21e1e 100644
--- a/source/Symbol/TypeSystem.cpp
+++ b/source/Symbol/TypeSystem.cpp
@@ -111,10 +111,10 @@ CompilerType TypeSystem::GetTypeTemplateArgument(opaque_compiler_type_t type,
   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) {
diff --git a/unittests/Symbol/TestClangASTContext.cpp b/unittests/Symbol/TestClangASTContext.cpp
index 52b8b5392..9fe804708 100644
--- a/unittests/Symbol/TestClangASTContext.cpp
+++ b/unittests/Symbol/TestClangASTContext.cpp
@@ -382,8 +382,8 @@ TEST_F(TestClangASTContext, TemplateArguments) {
   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, TemplateArguments) {
               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);
   }
 }