teemperor requested changes to this revision.
teemperor added a comment.
This revision now requires changes to proceed.
Thanks for the patch!
So IIUC correctly this fixes a crash when calling `Dereference` on an SBValue
that is of type `SomeTypedef` with `typedef int& SomeTypedef`? If yes, then I
think the test here could just be another type+assert in
`TestCPPDereferencingReferences.py`. The test here is a bit expensive and seems
to depend on a specific libc++ version (-> it will also always pass on Linux),
even though this is just a TypeSystemClang patch.
================
Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:6505
if (idx_is_valid) {
- const clang::ReferenceType *reference_type =
- llvm::cast<clang::ReferenceType>(GetQualType(type).getTypePtr());
- CompilerType pointee_clang_type =
- GetType(reference_type->getPointeeType());
+ CompilerType pointee_clang_type;
+
----------------
I really like the idea of reusing the TypeSystemClang functions here (we should
do this everywhere I think). FWIW, I think this can all just be `CompilerType
pointee_clang_type = GetNonReferenceType(type);` From the code below
`GetPointeeType` would also work. We already call the variable here `pointee`
type so I don't think calling references pointers here will hurt too much, so I
think both is fine.
================
Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:6508
+ if (parent_type_class == clang::Type::LValueReference)
+ pointee_clang_type = GetLValueReferenceType(type).GetPointeeType();
+ else
----------------
I think the logic here is reversed? `type` is a reference type (potentially
behind a typedef). `GetLValueReferenceType` on `type` returns the reference
type *to* that type. Not the underlying reference. The fallback for this is
just that it returns the current type IIRC if you already have a reference,
that's why this works. So, `GetLValueReferenceType` is just a no-op and
`GetPointeeType` is doing the actual dereferencing. I think just the
`GetPointeeType` is needed.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108717/new/
https://reviews.llvm.org/D108717
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
