[lldb] Unwrap the type when dereferencing the value

[werat]

Summary

The value type can be a typedef of a reference (e.g. typedef int& myint).
In this case GetQualType(type) will return clang::Typedef, which cannot
be casted to clang::ReferenceType.

Fix a regression introduced in https://reviews.llvm.org/D103532.

[shafik]

I was trying to fix this a while back in D108717 but got distracted and did not get back to it.

[teemperor]

Thanks a lot for fixing this, could you point to D103532 as the cause for the regression in the commit message?

This LGTM in general, but I think there could still be some obscure cases where this could fail.

So the switch statement this code is in (and which detects whether its a reference type) is defined as:

clang::QualType parent_qual_type(
    RemoveWrappingTypes(GetCanonicalQualType(type)));
switch (parent_qual_type->getTypeClass()) {

GetCanonicalQualType should strip all the outer and inner type sugar there is. And RemoveWrappingTypes removes *most* of the outer type sugar (which isn't necessary anymore as it should be all gone), but also makes types non-atomic (for simplicity reasons).

Meanwhile in our reference-case code we try to remove the outer type sugar (and atomic) via RemoveWrappingTypes. And if this doesn't end up in a ReferenceType the cast fails.

So IIUC there is still a possibility that we fail to cast here in case GetCanonicalQualType removes more outer sugar than RemoveWrappingTypes, which I think can only happen for type sugar classes not implemented by RemoveWrappingTypes (e.g., a quick grep tells me AttributedType and some template stuff is sugar that we don't handle).

So I would propose we do the following:

1. This patch can land now as-is to fix the regression.
2. It would be nice if we could expand the test with a few more variations of sugar. E.g., typedef to a reference of a typedef type and all that stuff.
3. I think we can simplify RemoveWrappingTypes to remove all type sugar (not just our small list we maintain there). That should eliminate the obscure cases and we no longer need to maintain a list of type sugar there.

lldb/test/API/lang/cpp/dereferencing_references/main.cpp

10

nit: Could you give this a more unique name such as td_to_ref_type?

[werat]

Update commit message, rename test variable

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

case clang::Type::Pointer: {
     }
     break;
   }
 
   case clang::Type::LValueReference:
   case clang::Type::RValueReference:
     if (idx_is_valid) {
       const clang::ReferenceType *reference_type =
-          llvm::cast<clang::ReferenceType>(GetQualType(type).getTypePtr());
+          llvm::cast<clang::ReferenceType>(
+              RemoveWrappingTypes(GetQualType(type)).getTypePtr());
       CompilerType pointee_clang_type =
           GetType(reference_type->getPointeeType());
       if (transparent_pointers && pointee_clang_type.IsAggregateType()) {
         child_is_deref_of_parent = false;
         bool tmp_child_is_deref_of_parent = false;
         return pointee_clang_type.GetChildCompilerTypeAtIndex(
             exe_ctx, idx, transparent_pointers, omit_empty_base_classes,
             ignore_array_bounds, child_name, child_byte_size, child_byte_offset,

lldb/test/API/lang/cpp/dereferencing_references/TestCPPDereferencingReferences.py

def test(self):
         # The result should be `TTT` (and *not* for example the underlying type
         # 'int').
         lref_val = self.expect_var_path("l_ref", type="TTT &")
         self.assertEqual(lref_val.Dereference().GetType().GetName(), "TTT")
 
         # Same as above for rvalue references.
         rref_val = self.expect_var_path("r_ref", type="TTT &&")
         self.assertEqual(rref_val.Dereference().GetType().GetName(), "TTT")
+
+        # Typedef to a reference should dereference to the underlying type.
+        td_val = self.expect_var_path("td", type="td_int_ref")
+        self.assertEqual(td_val.Dereference().GetType().GetName(), "int")

lldb/test/API/lang/cpp/dereferencing_references/main.cpp

 typedef int TTT;
+typedef int &td_int_ref;
 
 int main() {
   int i = 0;
+  // references to typedefs
   TTT &l_ref = i;
   TTT &&r_ref = static_cast<TTT &&>(i);
+  // typedef of a reference
+  td_int_ref td = i;
+
   return l_ref; // break here
 }