teemperor added inline comments.
================
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;
+
----------------
shafik wrote:
> teemperor wrote:
> > 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.
> I tried `GetNonReferenceType(type).GetPointeeType()` but I get back an empty
> `CompilerType` it looks like it is doing almost exactly the same thing as the
> previous code:
>
> `(*this)->getAs<ReferenceType>()`
I mean the whole thing could be just `GetNonReferenceType`.
`GetNonReferenceType` gives you the underlying type, so the `GetPointeeType`
after would just fail (-> empty return type) when it gets whatever the
underlying type is:
```
CompilerType int_ref = ...;
int_ref.GetName() // -> "int &"
int_ref.GetNonReferenceType().GetName() // -> "int"
int_ref.GetPointeeType().GetName() // -> "int"
CompilerType int = ...;
int.GetName() // -> "int"
int_ref..GetPointeeType().GetName() // -> "" (Invalid CompilerType as int isn't
a pointer/reference
```
================
Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:6508
+ if (parent_type_class == clang::Type::LValueReference)
+ pointee_clang_type = GetLValueReferenceType(type).GetPointeeType();
+ else
----------------
shafik wrote:
> teemperor wrote:
> > 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.
> Maybe I am confused but I thought given:
>
> ```
> TypedefType 0x7fb11c841460 'std::__compressed_pair_elem<struct
> std::basic_string<char, struct std::char_traits<char>, class
> std::allocator<char> >::__rep, 0, false>::const_reference' sugar
> |-Typedef 0x7fb11c8413f0 'const_reference'
> `-LValueReferenceType 0x7fb11c8413c0 'const struct std::basic_string<char,
> struct std::char_traits<char>, class std::allocator<char> >::__rep &'
> `-QualType 0x7fb11cac3361 'const struct std::basic_string<char, struct
> std::char_traits<char>, class std::allocator<char> >::__rep' const
> `-RecordType 0x7fb11cac3360 'struct std::basic_string<char, struct
> std::char_traits<char>, class std::allocator<char> >::__rep'
> `-CXXRecord 0x7fb11cac32b8 '__rep'
> ```
>
> that `llvm::cast<clang::ReferenceType>(GetQualType(type).getTypePtr())` was
> intended to obtain:
>
> ```
> LValueReferenceType 0x7fb11c829630 'const struct std::basic_string<char,
> struct std::char_traits<char>, class std::allocator<char> >::__rep &'
> `-QualType 0x7fb11cac3361 'const struct std::basic_string<char, struct
> std::char_traits<char>, class std::allocator<char> >::__rep' const
> `-RecordType 0x7fb11cac3360 'struct std::basic_string<char, struct
> std::char_traits<char>, class std::allocator<char> >::__rep'
> `-CXXRecord 0x7fb11cac32b8 '__rep'
> ```
>
> which is what `GetLValueReferenceType(type)` does.
>
>
That is the right intention of the original code, but `GetLValueReferenceType`
is just the inverse of what should happen. It takes a non-reference type and
returns the reference type for it (`int` -> `int &`). I added some dump calls
to the code which might explain what's going on:
```
lang=c++
case clang::Type::LValueReference:
case clang::Type::RValueReference:
if (idx_is_valid) {
CompilerType pointee_clang_type;
if (parent_type_class == clang::Type::LValueReference) {
llvm::errs() << "type:\n";
GetQualType(type).dump();
CompilerType tmp = GetLValueReferenceType(type);
llvm::errs() << "tmp (after GetLValueReferenceType):\n";
tmp.dump();
pointee_clang_type = tmp.GetPointeeType();
llvm::errs() << "pointee\n";
pointee_clang_type.dump();
```
```
lang=python
type:
LValueReferenceType 0x564ba82bde80 'TTT &'
`-TypedefType 0x564ba82bde30 'TTT' sugar
|-Typedef 0x564ba82bddc0 'TTT'
`-BuiltinType 0x564ba82bd5d0 'int'
tmp (after GetLValueReferenceType):
LValueReferenceType 0x564ba82bdeb0 'TTT &'
`-TypedefType 0x564ba82bde30 'TTT' sugar
|-Typedef 0x564ba82bddc0 'TTT'
`-BuiltinType 0x564ba82bd5d0 'int'
pointee
TypedefType 0x564ba82bde30 'TTT' sugar
|-Typedef 0x564ba82bddc0 'TTT'
`-BuiltinType 0x564ba82bd5d0 'int'
```
So you get a reference type, then LValueReferenceType does nothing[1] as it's
already a reference, and `GetPointeeType` returns the underlying type.
[1] It seems the function actually desugars the type, but `GetPointeeType` can
handle sugar so it effectively is a no-op.
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
