This revision was automatically updated to reflect the committed changes.
Closed by commit rG865094746e2a: [clang][Interp] Track initialization state of
local variables (authored by tbaeder).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135750/new/
aaron.ballman accepted this revision.
aaron.ballman added a comment.
LGTM!
Comment at: clang/lib/AST/Interp/Pointer.h:63-64
private:
static constexpr unsigned PastEndMark = (unsigned)-1;
static constexpr unsigned RootPtrMark = (unsigned)-1;
aaron.ballm
tbaeder updated this revision to Diff 489693.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135750/new/
https://reviews.llvm.org/D135750
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/Context.cpp
clang/lib/AST/Interp/
shafik accepted this revision.
shafik added a comment.
This revision is now accepted and ready to land.
LGTM
@aaron.ballman are you happy with this?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135750/new/
https://reviews.llvm.org/D135750
_
tbaeder updated this revision to Diff 488996.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135750/new/
https://reviews.llvm.org/D135750
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/Context.cpp
clang/lib/AST/Interp/
tbaeder added a comment.
Ping
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
tbaeder added a comment.
Ping
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
tbaeder updated this revision to Diff 485271.
tbaeder added a comment.
Add missing inline descriptor handling to local variables created in the
EvalEmitter.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135750/new/
https://reviews.llvm.org/D135750
Files:
clang/lib/AST/Interp/ByteCod
tbaeder added inline comments.
Comment at: clang/lib/AST/Interp/Interp.h:918
return false;
+ if (!Ptr.isRoot())
+Ptr.initialize();
shafik wrote:
> tbaeder wrote:
> > shafik wrote:
> > > Can you explain what is going on here? Why do we need to initialize
shafik added inline comments.
Comment at: clang/lib/AST/Interp/Interp.h:918
return false;
+ if (!Ptr.isRoot())
+Ptr.initialize();
tbaeder wrote:
> shafik wrote:
> > Can you explain what is going on here? Why do we need to initialize if it
> > is not ro
tbaeder updated this revision to Diff 484499.
tbaeder marked 2 inline comments as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135750/new/
https://reviews.llvm.org/D135750
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/
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
shafik added a comment.
This looks good to me but I see at least one concern that Aaron had that he did
not get back on, so I will wait for him to approve.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:835
IsTemporary = true;
Ty = E->getType();
}
--
tbaeder updated this revision to Diff 484212.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135750/new/
https://reviews.llvm.org/D135750
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/Context.cpp
clang/lib/AST/Interp/
tbaeder added inline comments.
Comment at: clang/lib/AST/Interp/InterpBlock.h:97
void invokeCtor() {
-std::memset(data(), 0, getSize());
+std::memset(rawData(), 0, Desc->getAllocSize());
if (Desc->CtorFn)
aaron.ballman wrote:
> tbaeder wrote:
> >
aaron.ballman added inline comments.
Comment at: clang/lib/AST/Interp/InterpBlock.h:97
void invokeCtor() {
-std::memset(data(), 0, getSize());
+std::memset(rawData(), 0, Desc->getAllocSize());
if (Desc->CtorFn)
tbaeder wrote:
> aaron.ballman wrote
tbaeder marked 9 inline comments as done.
tbaeder added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:835
IsTemporary = true;
Ty = E->getType();
}
shafik wrote:
> Do we really want to the type of the expression? If we have a `
aaron.ballman added inline comments.
Comment at: clang/lib/AST/Interp/Descriptor.h:141-145
unsigned getAllocSize() const { return AllocSize; }
/// returns the size of an element when the structure is viewed as an array.
unsigned getElemSize() const { return ElemSize; }
tbaeder added inline comments.
Comment at: clang/lib/AST/Interp/Descriptor.h:73
+ /// Flag indicating if the field is mutable (if in a record).
+ unsigned IsMutable : 1;
+
shafik wrote:
> Maybe `IsFieldMutable` b/c we call `CreateDescriptor` it is a little
> c
tbaeder updated this revision to Diff 479582.
tbaeder marked an inline comment as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135750/new/
https://reviews.llvm.org/D135750
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/
tbaeder added inline comments.
Comment at: clang/lib/AST/Interp/InterpBlock.h:97
void invokeCtor() {
-std::memset(data(), 0, getSize());
+std::memset(rawData(), 0, Desc->getAllocSize());
if (Desc->CtorFn)
aaron.ballman wrote:
> tbaeder wrote:
> >
shafik added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:835
IsTemporary = true;
Ty = E->getType();
}
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
tbaeder updated this revision to Diff 477437.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135750/new/
https://reviews.llvm.org/D135750
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/Context.cpp
clang/lib/AST/Interp/
tbaeder marked 10 inline comments as done.
tbaeder added inline comments.
Comment at: clang/lib/AST/Interp/Descriptor.h:141-145
unsigned getAllocSize() const { return AllocSize; }
/// returns the size of an element when the structure is viewed as an array.
unsigned getEl
aaron.ballman added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:749-751
+ const Descriptor::MetadataSize MDSize{sizeof(InlineDescriptor)};
+ Descriptor *D =
+ P.createDescriptor(Src, Ty, MDSize, IsConst, Src.is());
tbaeder wrote:
tbaeder marked 4 inline comments as done.
tbaeder added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:749-751
+ const Descriptor::MetadataSize MDSize{sizeof(InlineDescriptor)};
+ Descriptor *D =
+ P.createDescriptor(Src, Ty, MDSize, IsConst, Src.is(
aaron.ballman added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:749-751
+ const Descriptor::MetadataSize MDSize{sizeof(InlineDescriptor)};
+ Descriptor *D =
+ P.createDescriptor(Src, Ty, MDSize, IsConst, Src.is());
tbaeder wrote:
tbaeder updated this revision to Diff 476334.
tbaeder marked 3 inline comments as done.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135750/new/
https://reviews.llvm.org/D135750
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/
tbaeder marked 14 inline comments as done.
tbaeder added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:749-751
+ const Descriptor::MetadataSize MDSize{sizeof(InlineDescriptor)};
+ Descriptor *D =
+ P.createDescriptor(Src, Ty, MDSize, IsConst, Src.is
shafik added inline comments.
Comment at: clang/lib/AST/Interp/Context.cpp:128
InterpState State(Parent, *P, Stk, *this);
- State.Current = new InterpFrame(State, Func, nullptr, {}, {});
+ State.Current = new InterpFrame(State, Func, nullptr, {});
if (Interpret(State, Res
erichkeane added a comment.
I don't have enough knowledge about how this works to do a better review, but I
have a couple of suggestions.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:749
bool IsExtended) {
- D
aaron.ballman added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:749-751
+ const Descriptor::MetadataSize MDSize{sizeof(InlineDescriptor)};
+ Descriptor *D =
+ P.createDescriptor(Src, Ty, MDSize, IsConst, Src.is());
Double checking
tbaeder added a comment.
Ping. This review has been open for a month now and a lot of following
functionality depends on it, so if you need to decide which one of the patches
to review, choose this one :)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135750/new/
https://reviews.llvm.o
tbaeder added a comment.
Ping
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
tbaeder updated this revision to Diff 471713.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135750/new/
https://reviews.llvm.org/D135750
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/Context.cpp
clang/lib/AST/Interp/
tbaeder added a comment.
Ping
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
tbaeder updated this revision to Diff 470724.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135750/new/
https://reviews.llvm.org/D135750
Files:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/Context.cpp
clang/lib/AST/Interp/
tbaeder added a comment.
Ping
Repository:
rG LLVM Github Monorepo
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
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
This patch is a bit all over the place, sorry for t
39 matches
Mail list logo
