[PATCH] D134057: [clang][Interp] Start implementing record types

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:316
+  // Base above gives us a pointer on the stack.
+  const auto *FD = dyn_cast<FieldDecl>(Member);
+  assert(FD);
----------------
erichkeane wrote:
> I THINK Member is a ValueDecl because it could be a member function, right?  So forcing it to be a FieldDecl here is likely not valid.  Perhaps in the 'non-FieldDecl' case we could jsut return false for now?
> 
> ALSO, don't do a dyn_cast followed by an assert, `cast` will do the assert for you.
Right, I was just trying to limit the code to the subset I've implemented for now. I can try to make it more defensive.


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:659
+
+    // FIXME
+    const Stmt *Body = Ctor->getBody();
----------------
erichkeane wrote:
> Eh?
As far as I understand, actually visiting the body of the constructor will require a bit more setup work of `this` pointers and such. So for now, not implemented and a FIXME.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134057/new/

https://reviews.llvm.org/D134057