[lldb] Desugar template specializations

[jarin]

Summary

Template specializations are not handled in many of the
TypeSystemClang methods. For example, GetNumChildren does not handle
the TemplateSpecialization type class, so template specializations
always look like empty objects.

This patch just desugars template specialization in the existing
RemoveWrappingTypes desugaring helper.

[teemperor]

This could cause that RemoveWrappingTypes goes into an infinite loop under some situations. Usually this function is reserved for types that are always 'sugar', but TemplateSpecializationTypes are not always sugar (e.g., dependent types are not sugar). And for types that are not sugar, getLocallyUnqualifiedSingleStepDesugaredType will return the type that was passed in. So that will make the loop in that function just keep spinning forever.

However I'm not sure though if there is actually a way to get a dependent type into that logic with the normal LLDB APIs. Also Decltype is already suffering from the same problem so I don't think this patch should be blocked over this.

So beside some minor nits in the inline comments this LGTM, thanks!

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

2506

Nit: this is supposed to be in alphabetical order.

lldb/test/API/lang/cpp/template-specialization/TestTemplateSpecialization.py

1 ↗

(On Diff #278097)

I think this test should be just called template-specialization-type as it's mainly testing that we do the right thing this this specific type.

25 ↗

(On Diff #278097)

Maybe add a comment that this turns the RecordType into a TemplateSpecializationType.

[jarin]

Addressed review comments.

[jarin]

In D83858#2152772, @teemperor wrote:

This could cause that RemoveWrappingTypes goes into an infinite loop under some situations. Usually this function is reserved for types that are always 'sugar', but TemplateSpecializationTypes are not always sugar (e.g., dependent types are not sugar). And for types that are not sugar, getLocallyUnqualifiedSingleStepDesugaredType will return the type that was passed in. So that will make the loop in that function just keep spinning forever.

That sounds some what worrisome indeed. I changed the code to special case the TemplateSpecialization case to return if the type cannot be desugared. There are two other options:

1. Special case TemplateSpecialization to always return type->getLocallyUnqualifiedSingleStepDesugaredType().
2. No special case, but always early return if the type was unchanged. I am imagining something like

case clang::Type::TemplateSpecialization:
case clang::Type::TypeOf:
case clang::Type::TypeOfExpr: {
  auto original = type;
  type = type->getLocallyUnqualifiedSingleStepDesugaredType();
  if (type == original)
    return type;
}

However I'm not sure though if there is actually a way to get a dependent type into that logic with the normal LLDB APIs. Also Decltype is already suffering from the same problem so I don't think this patch should be blocked over this.

So beside some minor nits in the inline comments this LGTM, thanks!

[teemperor]

I think your comment got broken by Phabricator. I actually think the old code was fine. It's a generic bug that also applies to Decltype (which is already implemented), so someone just has to fix the whole function in a follow up patch (your current code just fixes the TemplateSpecializationType but not Decltype). I can do that, but finding a good testing strategy for this is a bit of a pain (it's a lot of boilerplate to make a dependent type by hand and not sure if we have a good way to reach this from a normal LLDB API).

Just revert the RemoveWrappingTypes to the old implementation and then this is ready to go IMHO.

[jarin]

Undo the infinite loop detection.

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

while (true) {
     // sufficient for the purposes we care about.
     case clang::Type::Atomic:
       type = cast<clang::AtomicType>(type)->getValueType();
       break;
     case clang::Type::Auto:
     case clang::Type::Decltype:
     case clang::Type::Elaborated:
     case clang::Type::Paren:
+    case clang::Type::TemplateSpecialization:
     case clang::Type::Typedef:
     case clang::Type::TypeOf:
     case clang::Type::TypeOfExpr:
       type = type->getLocallyUnqualifiedSingleStepDesugaredType();
       break;
     default:
       return type;
     }
   }
 }
 
 clang::DeclContext *

lldb/test/API/lang/cpp/template-specialization-type/Makefile

+CXX_SOURCES := main.cpp
+
+include Makefile.rules

lldb/test/API/lang/cpp/template-specialization-type/TestTemplateSpecializationType.py

+"""
+Test value with a template specialization type.
+"""
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TemplateSpecializationTypeTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def test_template_specialization_cast_children(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, '// break here',
+                lldb.SBFileSpec("main.cpp", False))
+
+        v = self.frame().EvaluateExpression("t")
+        self.assertEquals(2, v.GetNumChildren())
+        self.assertEquals("42", v.GetChildAtIndex(0).GetValue())
+        self.assertEquals("21", v.GetChildAtIndex(1).GetValue())
+
+        # Test a value of the TemplateSpecialization type. We turn
+        # RecordType into TemplateSpecializationType by casting and
+        # dereferencing a pointer to a record.
+        v = self.frame().EvaluateExpression("*((TestObj<int>*)&t)")
+        self.assertEquals(2, v.GetNumChildren())
+        self.assertEquals("42", v.GetChildAtIndex(0).GetValue())
+        self.assertEquals("21", v.GetChildAtIndex(1).GetValue())

lldb/test/API/lang/cpp/template-specialization-type/main.cpp

+template <typename T> struct TestObj {
+  int f;
+  T g;
+};
+
+int main() {
+  TestObj<int> t{42, 21};
+  return t.f + t.g; // break here
+}