[PATCH] D134057: [clang][Interp] Start implementing record types
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 26 14:21:47 PDT 2022
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM aside from a minor nit, we can handle the other cases in follow-ups.
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:642
+
+ for (const auto *Init : Ctor->inits()) {
+ const FieldDecl *Member = Init->getMember();
----------------
tbaeder wrote:
> aaron.ballman wrote:
> > Do we have to do anything special if the ctor is an inheriting ctor?
> I have a feeling we do, but I'll tackle inheritance and virtual functions later.
Okay, that seems reasonable to me.
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:687
+ return true;
+ } else if (const CallExpr *CE = dyn_cast<CallExpr>(Initializer)) {
+ const Decl *Callee = CE->getCalleeDecl();
----------------
tbaeder wrote:
> aaron.ballman wrote:
> > `CXXMemberCallExpr` as well?
> Yup, that's unhandled here right now it seems, I can add it to this patch or add it in a later one.
I'm fine doing it in a follow-up.
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:689
+ const Decl *Callee = CE->getCalleeDecl();
+ const Function *Func = P.getFunction(dyn_cast<FunctionDecl>(Callee));
+
----------------
tbaeder wrote:
> aaron.ballman wrote:
> > What if this comes back as `nullptr` (does `getFunction()` handle that gracefully)?
> It does not :) I was relying on a crash somewhere when I have a test case for it.
I'd rather use an explicit `assert` for that instead of relying on a crash; it's easier to spot the `assert` in that case.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134057/new/
https://reviews.llvm.org/D134057
More information about the cfe-commits
mailing list