[Lldb-commits] [PATCH] D72133: Data formatters: Look through array element typedefs

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.


================
Comment at: lldb/source/API/SBType.cpp:215-218
+  CompilerType canonical_type =
+      m_opaque_sp->GetCompilerType(true).GetCanonicalType();
+  return LLDB_RECORD_RESULT(
+      SBType(TypeImplSP(new TypeImpl(canonical_type.GetArrayElementType()))));
----------------
Why does getting the canonical type of an array type change the array element type? I would think it wouldn't matter but it obviously must?


================
Comment at: lldb/source/DataFormatters/FormatManager.cpp:245-261
+  uint64_t array_size;
+  if (compiler_type.IsArrayType(nullptr, &array_size, nullptr)) {
+    CompilerType element_type = compiler_type.GetArrayElementType();
+    if (element_type.IsTypedefType()) {
+      // Get the stripped element type and compute the stripped array type
+      // from it.
+      CompilerType deffed_array_type =
----------------
Wouldn't we want to add one for each typedef that has a formatter? Lets say we had:

```
typedef char MCHAR;
typedef MCHAR MMCHAR;

int main() {
  MMCHAR str[5] = "abcd";
  return 0;
}
```

if we had a special formatter for MCHAR that would maybe display MCHAR somehow special, would we be ok with always getting the standard C string formatter in this case? Seems like we should add one for each typedef that has a formatter and possibly also the base type?


================
Comment at: lldb/source/Symbol/ClangASTContext.cpp:3946
 
-    CompilerType element_type =
-        GetType(array_eletype->getCanonicalTypeUnqualified());
+    CompilerType element_type = GetType(clang::QualType(array_eletype, 0));
 
----------------
If I ask an array type for its element type, I wouldn't expect all typedefs to be removed. This seems wrong, or it can become an option to this function as an extra param:

```
CompilerType
ClangASTContext::GetArrayElementType(lldb::opaque_compiler_type_t type, uint64_t *stride, bool get_canonical_element_type);
```


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

https://reviews.llvm.org/D72133