shafik added inline comments.
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:803
+ // Make sure we don't accidentally register the same decl twice.
+ if (const Decl *VD = Src.dyn_cast<const Decl *>(); VD && isa<ValueDecl>(VD))
{
+ assert(!P.getGlobal(cast<ValueDecl>(VD)));
----------------
Nitpick, I find using `VD` a bit confusing since we don't know if it is a
`ValueDecl` until after it is initialized.
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1154
+
+ if (!GlobalIndex)
+ return this->bail(VD);
----------------
I wonder if the code in both branches might be better factored out into their
own functions and perhaps move the ` DeclScope<Emitter> LocalScope(this, VD);`
out.
================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:282
+ bool isGlobalDecl(const VarDecl *VD) const {
+ return !VD->hasLocalStorage() || VD->isConstexpr();
+ }
----------------
Why not `hasGlobalStorage()`?
Also what is the logic behind `isConstexpr()` check?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D136815/new/
https://reviews.llvm.org/D136815
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
