jarin marked 3 inline comments as done.
jarin added inline comments.
================
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()))));
----------------
clayborg wrote:
> 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?
Removal of typedef for element type is the existing behavior that I have been
trying preserve. Raphael already suggested we should not preserve it.
For example, I get the following output for my program (both without and with
my patch):
```
Type: MCHAR [10]
El type: char
```
Here is the script
```
import lldb, os
debugger = lldb.SBDebugger.Create()
debugger.SetAsync(False)
target = debugger.CreateTargetWithFileAndTargetTriple("df.out", \
"x86_64-pc-linux")
main_bp = target.BreakpointCreateByLocation("df.cc", 11)
process = target.LaunchSimple(None, None, os.getcwd())
frame = process.selected_thread.GetSelectedFrame()
var_a = process.selected_thread.GetSelectedFrame().GetValueForVariablePath("a")
print("Type: " + var_a.GetType().GetName())
print("El type: " + var_a.GetType().GetArrayElementType().GetName())
```
If we remove the canonicalization here, we will indeed get the behavior that
you want:
```
Type: MCHAR [10]
El type: MCHAR
```
================
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 =
----------------
clayborg wrote:
> 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?
This does not work as expected with my patch, let me investigate why.
================
Comment at: lldb/source/Symbol/ClangASTContext.cpp:3946
- CompilerType element_type =
- GetType(array_eletype->getCanonicalTypeUnqualified());
+ CompilerType element_type = GetType(clang::QualType(array_eletype, 0));
----------------
clayborg wrote:
> 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);
> ```
The original code for ClangASTContext::GetArrayElementType did remove the
typedefs. With my change ClangASTContext::GetArrayElementType does not remove
typedefs anymore.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72133/new/
https://reviews.llvm.org/D72133
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
