[Lldb-commits] [PATCH] D83858: [lldb] Desugar template specializations

[Raphael Isemann via Phabricator via lldb-commits]

teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

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!



================
Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:2507
     case clang::Type::TypeOfExpr:
+    case clang::Type::TemplateSpecialization:
       type = type->getLocallyUnqualifiedSingleStepDesugaredType();
----------------
Nit: this is supposed to be in alphabetical order.


================
Comment at: lldb/test/API/lang/cpp/template-specialization/TestTemplateSpecialization.py:1
+"""
+Test evaluating an expression with a cast to a pointer to a template
----------------
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.


================
Comment at: lldb/test/API/lang/cpp/template-specialization/TestTemplateSpecialization.py:25
+
+        v = self.frame().EvaluateExpression("*((TestObj<int>*)&t)")
+        self.assertEquals(2, v.GetNumChildren())
----------------
Maybe add a comment that this turns the RecordType into a TemplateSpecializationType.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83858/new/

https://reviews.llvm.org/D83858