teemperor added a comment.
You might also be interested in D81471 <https://reviews.llvm.org/D81471> which
is a very similar patch but for DWARF and only focused on static const
integer/enum data members.
One thing I'm curious about is how this is solving (or if it's even supposed to
solve) the issue that the expression parser is by default taking the address of
lvalues (which will bring us back to the issue that LLDB will try to resolve
the symbol)? From what I understand, this patch allows me to to things like
`expr Class::Member + 1` but not `expr Class::Member` (because the expression
evaluator will rewrite that lvalue to `int *$__lldb_expr_result_ptr =
&Class::Member`).
================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:155
+ switch (ClangUtil::GetQualType(member_ct)
+ ->castAs<clang::BuiltinType>()
+ ->getKind()) {
----------------
What about just doing the switch directly on
`member_ct.GetBasicTypeEnumeration()` and check for `eBasicTypeFloat` and so
on. That's less code to do the same thing without all the casting.
================
Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:7321
+ var_decl->setInit(clang::IntegerLiteral::Create(
+ var_decl->getASTContext(), value.extOrTrunc(num_bits),
+ var_decl->getType(), clang::SourceLocation()));
----------------
Passing a too large APSInt into this function will just silently truncate it. I
think this should refuse to add the initializer in this case (and we probably
should at least log that error somewhere).
================
Comment at: lldb/test/Shell/SymbolFile/PDB/Inputs/AstRestoreTest.cpp:27
+ static constexpr float ClassStaticConstexprFloat = 9.f;
+ static constexpr double ClassStaticConstexprDouble = 10.0;
----------------
You might want to also add some static const (not `constexpr`) integer and enum
members to this test.
Also I'm curious what happens if you put a `long double` as a member? From what
I can see the expected behavior is that PDB will not emit that as a constant
(as the current code asserts if it encounters `long double` anywhere).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82160/new/
https://reviews.llvm.org/D82160
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
