tbaeder marked 4 inline comments as done.
tbaeder added inline comments.

================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:835
     IsTemporary = true;
     Ty = E->getType();
   }
----------------
shafik wrote:
> tbaeder wrote:
> > shafik wrote:
> > > Do we really want to the type of the expression? If we have a `ValueDecl` 
> > > don't we want that type? I think they should be the same, do you have any 
> > > examples where the expression is the type we want? I am looking at the 
> > > AST from ` int x = 1+1L;` 
> > > 
> > > https://godbolt.org/z/q3Tr7Kxoq
> > I don't have a counter example but I didn't write that line either. The 
> > Expr case is for temporary values created for AST expressions, not sure 
> > what other type to use.
> I think it should be an `else if` we either have a `Decl` or an `Expr` right?
Yes, that should work as well.


================
Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:408
     // Compile the initializer in its own scope.
-    {
+    if (Init) {
       ExprScope<Emitter> Scope(this);
----------------
shafik wrote:
> So by moving the check for `Init` forward, we will still allocate but we may 
> not initialize.
Correct.


================
Comment at: clang/lib/AST/Interp/Interp.h:918
     return false;
+  if (!Ptr.isRoot())
+    Ptr.initialize();
----------------
shafik wrote:
> Can you explain what is going on here? Why do we need to initialize if it is 
> not root?
Root means that `Base == 0`, so it cannot have a `InlineDescriptor` before the 
`Base` offset. That is mainly the case for global variables.


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

https://reviews.llvm.org/D135750

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to