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

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik 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);
----------------
tbaeder wrote:
> 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.
Also I believe this can also be an `IndirectFieldDecl` (anonymous union members) or a `ValueDecl` for static data members. 

Maybe outline what is left to fill in a comment?


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:673
+      if (Optional<PrimType> T = classify(Init->getType())) {
+        if (!this->emitDupPtr(Initializer))
+          return false;
----------------
This section of code looks duplicated w/ the above, can it be factored out or will they diverge as you fill in more details?


================
Comment at: clang/test/AST/Interp/records.cpp:7
+
+struct Ints {
+  int a = 20;
----------------
How about also have a field that is a struct and initializing that.

Also using initializer lists in in class member initializers and also designated initializers as well. 

I am not sure if unions works yet but anon union members as well.


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

https://reviews.llvm.org/D134057