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