teemperor added a comment.
The only remaining issue with the LLDB expression part is that now that
`SetFloatingInitializerForVariable` is based on the D81471
<https://reviews.llvm.org/D81471> version it no longer makes the member
variable a `constexpr`. Int/Enum members with initialisers can just be const,
but for any floating point types this ends ups creating an AST that we usually
can't get from the Clang parser (as floats can't be initialised this way).
Clang will still codegen that for us, but I think making the floating point
VarDecls `constexpr` could save us from bad surprises in the future (e.g.,
someone adds an assert to Clang/Sema that const static data members are only
initialised when they have integer/enum type). I think marking the variable as
constexpr in the two places where you call `SetFloatingInitializerForVariable`
seems like the right place for this (`SetFloatingInitializerForVariable`
probably shouldn't set that flag on its own, that seems like unexpected
behaviour from this function).
Otherwise I think `auto` is used quite frequently and doesn't really fit to
LLVM's "auto only if it makes code more readable" policy. I think all the
integer types for the type widths should just be their own type (`uint32_t`
makes the code easier to understand than auto). And I don't think LLDB usually
does `auto` in LLDB for types/decls as LLDB has its own type/decl abstraction
(`CompilerType` and `CompilerDecl`), so that's also confusing.
Beside that this patch looks good to me. Maybe someone that knows more about
PDB should also take a look about the PDB-specific parts of the patch.
================
Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:7324-7331
+ // If the variable is an enum type, take the underlying integer type as
+ // the type of the integer literal.
+ if (const EnumType *enum_type = llvm::dyn_cast<EnumType>(qt.getTypePtr())) {
+ const EnumDecl *enum_decl = enum_type->getDecl();
+ qt = enum_decl->getIntegerType();
+ }
+ var->setInit(IntegerLiteral::Create(ast, init_value, qt.getUnqualifiedType(),
----------------
aleksandr.urakov wrote:
> I'm not sure, can we initialize a member this way if it has a scoped enum
> type?
(That code is from D81471 so I think I should answer that)
Well, it works and but it relies on CodeGen/Sema not double-checking that we
get the enum type (and not the underlying int type). I can inject a
CXXStaticCastExpr too and will update D81471. Thanks!
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
