[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
tbaeder marked an inline comment as done.
Closed by commit rG0ddd13acc9e9: [clang][Interp] Implement This pointer passing
to methods (authored by tbaeder).
Changed prior to commit:
https://reviews.llvm.org/D134699?vs=465966=467708#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134699/new/
https://reviews.llvm.org/D134699
Files:
clang/lib/AST/Interp/ByteCodeEmitter.cpp
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/Disasm.cpp
clang/lib/AST/Interp/EvalEmitter.cpp
clang/lib/AST/Interp/Function.cpp
clang/lib/AST/Interp/Function.h
clang/lib/AST/Interp/Interp.cpp
clang/lib/AST/Interp/InterpFrame.cpp
clang/lib/AST/Interp/InterpFrame.h
clang/test/AST/Interp/records.cpp
Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -1,7 +1,6 @@
// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
// RUN: %clang_cc1 -verify=ref %s
-// ref-no-diagnostics
// expected-no-diagnostics
struct BoolPair {
@@ -38,7 +37,6 @@
static_assert(ints2.b == -30, "");
static_assert(!ints2.c, "");
-#if 0
constexpr Ints getInts() {
return {64, 128, true};
}
@@ -46,7 +44,6 @@
static_assert(ints3.a == 64, "");
static_assert(ints3.b == 128, "");
static_assert(ints3.c, "");
-#endif
constexpr Ints ints4 = {
.a = 40 * 50,
@@ -103,3 +100,38 @@
return
}
static_assert(getPointer()->a == 100, "");
+
+constexpr C RVOAndParams(const C *c) {
+ return C();
+}
+constexpr C RVOAndParamsResult = RVOAndParams();
+
+constexpr int locals() {
+ C c;
+ c.a = 10;
+
+ // Assignment, not an initializer.
+ // c = C(); FIXME
+ c.a = 10;
+
+
+ // Assignment, not an initializer.
+ //c = RVOAndParams(); FIXME
+
+ return c.a;
+}
+static_assert(locals() == 10, "");
+
+namespace thisPointer {
+ struct S {
+constexpr int get12() { return 12; }
+ };
+
+ constexpr int foo() { // ref-error {{never produces a constant expression}}
+S *s = nullptr;
+return s->get12(); // ref-note 2{{member call on dereferenced null pointer}}
+ }
+ // FIXME: The new interpreter doesn't reject this currently.
+ static_assert(foo() == 12, ""); // ref-error {{not an integral constant expression}} \
+ // ref-note {{in call to 'foo()'}}
+};
Index: clang/lib/AST/Interp/InterpFrame.h
===
--- clang/lib/AST/Interp/InterpFrame.h
+++ clang/lib/AST/Interp/InterpFrame.h
@@ -35,6 +35,11 @@
InterpFrame(InterpState , Function *Func, InterpFrame *Caller,
CodePtr RetPC, Pointer &);
+ /// Creates a new frame with the values that make sense.
+ /// I.e., the caller is the current frame of S,
+ /// and the This() pointer is the current Pointer on the top of S's stack,
+ InterpFrame(InterpState , Function *Func, CodePtr RetPC);
+
/// Destroys the frame, killing all live pointers to stack slots.
~InterpFrame();
Index: clang/lib/AST/Interp/InterpFrame.cpp
===
--- clang/lib/AST/Interp/InterpFrame.cpp
+++ clang/lib/AST/Interp/InterpFrame.cpp
@@ -36,6 +36,36 @@
}
}
+InterpFrame::InterpFrame(InterpState , Function *Func, CodePtr RetPC)
+: Caller(S.Current), S(S), Func(Func), RetPC(RetPC),
+ ArgSize(Func ? Func->getArgSize() : 0),
+ Args(static_cast(S.Stk.top())), FrameOffset(S.Stk.size()) {
+ assert(Func);
+
+ // As per our calling convention, the this pointer is
+ // part of the ArgSize.
+ // If the function has RVO, the RVO pointer is first.
+ // If the fuction has a This pointer, that one is next.
+ // Then follow the actual arguments (but those are handled
+ // in getParamPointer()).
+ if (Func->hasThisPointer()) {
+if (Func->hasRVO())
+ This = stackRef(sizeof(Pointer));
+else
+ This = stackRef(0);
+ }
+
+ if (unsigned FrameSize = Func->getFrameSize()) {
+Locals = std::make_unique(FrameSize);
+for (auto : Func->scopes()) {
+ for (auto : Scope.locals()) {
+Block *B = new (localBlock(Local.Offset)) Block(Local.Desc);
+B->invokeCtor();
+ }
+}
+ }
+}
+
InterpFrame::~InterpFrame() {
if (Func && Func->isConstructor() && This.isBaseClass())
This.initialize();
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -55,8 +55,7 @@
template ::T>
static bool Call(InterpState , CodePtr , const Function *Func) {
- S.Current =
- new InterpFrame(S, const_cast(Func), S.Current, PC, {});
+ S.Current = new
[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
tbaeder marked an inline comment as done.
tbaeder added inline comments.
Comment at: clang/lib/AST/Interp/EvalEmitter.cpp:108
- S.Current =
- new InterpFrame(S, const_cast(Func), S.Current, {}, {});
+ S.Current = new InterpFrame(S, const_cast(Func), {});
// Result of call will be on the stack and needs to be handled by the caller.
aaron.ballman wrote:
> Not related to this review: it'd be nice to fix the interface so we get const
> correct behavior rather than needing to use `const_cast` like this.
Already have a local patch for this!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134699/new/
https://reviews.llvm.org/D134699
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: clang/lib/AST/Interp/EvalEmitter.cpp:108
- S.Current =
- new InterpFrame(S, const_cast(Func), S.Current, {}, {});
+ S.Current = new InterpFrame(S, const_cast(Func), {});
// Result of call will be on the stack and needs to be handled by the caller.
Not related to this review: it'd be nice to fix the interface so we get const
correct behavior rather than needing to use `const_cast` like this.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134699/new/
https://reviews.llvm.org/D134699
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
tbaeder added inline comments.
Comment at: clang/test/AST/Interp/records.cpp:139
+ // ref-note {{in call to 'foo()'}}
+};
aaron.ballman wrote:
> The other thing I think we need some tests for are constructor and destructor
> calls where the `this` pointer may be a bit surprising because it needs
> adjustments. For example, with multiple inheritance where the `this` pointer
> may need to be adjusted to get to the fields of the object, and ensuring the
> correct constructors are called in the correct order.
>
> Another case is with virtual functions (I'm assuming there's no vtable
> support yet and so that's less interesting, but it will become interesting
> once we get there so you may want to keep it in mind).
For constructors, see https://reviews.llvm.org/D135025. Although it doesn't
test any sort of order I believe and destructors aren't part of that either.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134699/new/
https://reviews.llvm.org/D134699
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
aaron.ballman added inline comments.
Comment at: clang/test/AST/Interp/records.cpp:139
+ // ref-note {{in call to 'foo()'}}
+};
The other thing I think we need some tests for are constructor and destructor
calls where the `this` pointer may be a bit surprising because it needs
adjustments. For example, with multiple inheritance where the `this` pointer
may need to be adjusted to get to the fields of the object, and ensuring the
correct constructors are called in the correct order.
Another case is with virtual functions (I'm assuming there's no vtable support
yet and so that's less interesting, but it will become interesting once we get
there so you may want to keep it in mind).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134699/new/
https://reviews.llvm.org/D134699
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
tbaeder updated this revision to Diff 465966.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134699/new/
https://reviews.llvm.org/D134699
Files:
clang/lib/AST/Interp/ByteCodeEmitter.cpp
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/Disasm.cpp
clang/lib/AST/Interp/EvalEmitter.cpp
clang/lib/AST/Interp/Function.cpp
clang/lib/AST/Interp/Function.h
clang/lib/AST/Interp/Interp.cpp
clang/lib/AST/Interp/InterpFrame.cpp
clang/lib/AST/Interp/InterpFrame.h
clang/test/AST/Interp/records.cpp
Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -102,3 +102,38 @@
return
}
static_assert(getPointer()->a == 100, "");
+
+constexpr C RVOAndParams(const C *c) {
+ return C();
+}
+constexpr C RVOAndParamsResult = RVOAndParams();
+
+constexpr int locals() {
+ C c;
+ c.a = 10;
+
+ // Assignment, not an initializer.
+ // c = C(); FIXME
+ c.a = 10;
+
+
+ // Assignment, not an initializer.
+ //c = RVOAndParams(); FIXME
+
+ return c.a;
+}
+static_assert(locals() == 10, "");
+
+namespace thisPointer {
+ struct S {
+constexpr int get12() { return 12; }
+ };
+
+ constexpr int foo() { // ref-error {{never produces a constant expression}}
+S *s = nullptr;
+return s->get12(); // ref-note 2{{member call on dereferenced null pointer}}
+ }
+ // FIXME: The new interpreter doesn't reject this currently.
+ static_assert(foo() == 12, ""); // ref-error {{not an integral constant expression}} \
+ // ref-note {{in call to 'foo()'}}
+};
Index: clang/lib/AST/Interp/InterpFrame.h
===
--- clang/lib/AST/Interp/InterpFrame.h
+++ clang/lib/AST/Interp/InterpFrame.h
@@ -35,6 +35,11 @@
InterpFrame(InterpState , Function *Func, InterpFrame *Caller,
CodePtr RetPC, Pointer &);
+ /// Creates a new frame with the values that make sense.
+ /// I.e., the caller is the current frame of S,
+ /// and the This() pointer is the current Pointer on the top of S's stack,
+ InterpFrame(InterpState , Function *Func, CodePtr RetPC);
+
/// Destroys the frame, killing all live pointers to stack slots.
~InterpFrame();
Index: clang/lib/AST/Interp/InterpFrame.cpp
===
--- clang/lib/AST/Interp/InterpFrame.cpp
+++ clang/lib/AST/Interp/InterpFrame.cpp
@@ -36,6 +36,36 @@
}
}
+InterpFrame::InterpFrame(InterpState , Function *Func, CodePtr RetPC)
+: Caller(S.Current), S(S), Func(Func), RetPC(RetPC),
+ ArgSize(Func ? Func->getArgSize() : 0),
+ Args(static_cast(S.Stk.top())), FrameOffset(S.Stk.size()) {
+ assert(Func);
+
+ // As per our calling convention, the this pointer is
+ // part of the ArgSize.
+ // If the function has RVO, the RVO pointer is first.
+ // If the fuction has a This pointer, that one is next.
+ // Then follow the actual arguments (but those are handled
+ // in getParamPointer()).
+ if (Func->hasThisPointer()) {
+if (Func->hasRVO())
+ This = stackRef(sizeof(Pointer));
+else
+ This = stackRef(0);
+ }
+
+ if (unsigned FrameSize = Func->getFrameSize()) {
+Locals = std::make_unique(FrameSize);
+for (auto : Func->scopes()) {
+ for (auto : Scope.locals()) {
+Block *B = new (localBlock(Local.Offset)) Block(Local.Desc);
+B->invokeCtor();
+ }
+}
+ }
+}
+
InterpFrame::~InterpFrame() {
if (Func && Func->isConstructor() && This.isBaseClass())
This.initialize();
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -55,8 +55,7 @@
template ::T>
static bool Call(InterpState , CodePtr , const Function *Func) {
- S.Current =
- new InterpFrame(S, const_cast(Func), S.Current, PC, {});
+ S.Current = new InterpFrame(S, const_cast(Func), PC);
APValue CallResult;
// Note that we cannot assert(CallResult.hasValue()) here since
// Ret() above only sets the APValue if the curent frame doesn't
@@ -66,8 +65,7 @@
static bool CallVoid(InterpState , CodePtr , const Function *Func) {
APValue VoidResult;
- S.Current =
- new InterpFrame(S, const_cast(Func), S.Current, PC, {});
+ S.Current = new InterpFrame(S, const_cast(Func), PC);
bool Success = Interpret(S, VoidResult);
assert(VoidResult.isAbsent());
Index: clang/lib/AST/Interp/Function.h
===
--- clang/lib/AST/Interp/Function.h
+++ clang/lib/AST/Interp/Function.h
@@ -56,6 +56,21 @@
///
/// Contains links to the bytecode of the function, as well as metadata
/// describing all arguments and stack-local variables.
+///
+/// # Calling Convention
[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
aaron.ballman added inline comments.
Comment at: clang/test/AST/Interp/records.cpp:106
+
+constexpr C RVOAndParams(const C *c) {
+ return C();
tbaeder wrote:
> aaron.ballman wrote:
> > We're missing a fair amount of test coverage here in terms of calling
> > member functions. Can you add some tests for that, as well as a test where
> > the this pointer is invalid and causes UB? e.g.,
> > ```
> > struct S {
> > constexpr void member() {}
> > };
> >
> > constexpr void func(S *s) {
> > s->member(); // Should not be a constant expression
> > }
> >
> > int main() {
> > func(nullptr);
> > }
> > ```
> I just checked and we indeed don't reject that example. There are already
> functions to detect such scenarios in `Interp.h/.cpp`. But actually using
> them seems to require a bigger surgery since `Call`/`CallVoid` are currenly
> in both `EvalEmitter.cpp` and `Interp.cpp`.
>
> I've written this down but I think for this patch it makes sense to add the
> failing test case and ignore the failure for now.
I'm okay with adding a failure test case with a fixme comment on it for now.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134699/new/
https://reviews.llvm.org/D134699
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
tbaeder added inline comments.
Comment at: clang/test/AST/Interp/records.cpp:106
+
+constexpr C RVOAndParams(const C *c) {
+ return C();
aaron.ballman wrote:
> We're missing a fair amount of test coverage here in terms of calling member
> functions. Can you add some tests for that, as well as a test where the this
> pointer is invalid and causes UB? e.g.,
> ```
> struct S {
> constexpr void member() {}
> };
>
> constexpr void func(S *s) {
> s->member(); // Should not be a constant expression
> }
>
> int main() {
> func(nullptr);
> }
> ```
I just checked and we indeed don't reject that example. There are already
functions to detect such scenarios in `Interp.h/.cpp`. But actually using them
seems to require a bigger surgery since `Call`/`CallVoid` are currenly in both
`EvalEmitter.cpp` and `Interp.cpp`.
I've written this down but I think for this patch it makes sense to add the
failing test case and ignore the failure for now.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134699/new/
https://reviews.llvm.org/D134699
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
tbaeder marked an inline comment as done.
tbaeder added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:638
if (const auto CtorExpr = dyn_cast(Initializer)) {
-const CXXConstructorDecl *Ctor = CtorExpr->getConstructor();
-const RecordDecl *RD = Ctor->getParent();
-const Record *R = getRecord(RD);
-
-for (const auto *Init : Ctor->inits()) {
- const FieldDecl *Member = Init->getMember();
- const Expr *InitExpr = Init->getInit();
-
- if (Optional T = classify(InitExpr->getType())) {
-const Record::Field *F = R->getField(Member);
-
-if (!this->emitDupPtr(Initializer))
- return false;
+const Function *Func = getFunction(CtorExpr->getConstructor());
aaron.ballman wrote:
> What happens if `Func` is null?
We need to bail out!
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:668
const Decl *Callee = CE->getCalleeDecl();
-const Function *Func = P.getFunction(dyn_cast(Callee));
+const Function *Func = getFunction(dyn_cast(Callee));
+
aaron.ballman wrote:
> Any reason not to use `cast` here instead, given that `getFunction()` expects
> a nonnull pointer anyway?
Not particularly, I guess the generated assertion output is a little nicer if
it reaches the one in `getFunction()`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134699/new/
https://reviews.llvm.org/D134699
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
aaron.ballman added inline comments.
Comment at: clang/lib/AST/Interp/ByteCodeEmitter.cpp:47
+ bool HasThisPointer = false;
+ if (const auto *MD = dyn_cast(F); MD && !MD->isStatic()) {
+HasThisPointer = true;
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:638
if (const auto CtorExpr = dyn_cast(Initializer)) {
-const CXXConstructorDecl *Ctor = CtorExpr->getConstructor();
-const RecordDecl *RD = Ctor->getParent();
-const Record *R = getRecord(RD);
-
-for (const auto *Init : Ctor->inits()) {
- const FieldDecl *Member = Init->getMember();
- const Expr *InitExpr = Init->getInit();
-
- if (Optional T = classify(InitExpr->getType())) {
-const Record::Field *F = R->getField(Member);
-
-if (!this->emitDupPtr(Initializer))
- return false;
+const Function *Func = getFunction(CtorExpr->getConstructor());
What happens if `Func` is null?
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:668
const Decl *Callee = CE->getCalleeDecl();
-const Function *Func = P.getFunction(dyn_cast(Callee));
+const Function *Func = getFunction(dyn_cast(Callee));
+
Any reason not to use `cast` here instead, given that `getFunction()` expects a
nonnull pointer anyway?
Comment at: clang/test/AST/Interp/records.cpp:106
+
+constexpr C RVOAndParams(const C *c) {
+ return C();
We're missing a fair amount of test coverage here in terms of calling member
functions. Can you add some tests for that, as well as a test where the this
pointer is invalid and causes UB? e.g.,
```
struct S {
constexpr void member() {}
};
constexpr void func(S *s) {
s->member(); // Should not be a constant expression
}
int main() {
func(nullptr);
}
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134699/new/
https://reviews.llvm.org/D134699
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
tbaeder added a comment. @aaron.ballman This one has working precommit CI \o/ (array filler patch is not needed for it though) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134699/new/ https://reviews.llvm.org/D134699 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
tbaeder updated this revision to Diff 465279.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134699/new/
https://reviews.llvm.org/D134699
Files:
clang/lib/AST/Interp/ByteCodeEmitter.cpp
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/Disasm.cpp
clang/lib/AST/Interp/EvalEmitter.cpp
clang/lib/AST/Interp/Function.cpp
clang/lib/AST/Interp/Function.h
clang/lib/AST/Interp/Interp.cpp
clang/lib/AST/Interp/InterpFrame.cpp
clang/lib/AST/Interp/InterpFrame.h
clang/test/AST/Interp/records.cpp
Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -102,3 +102,24 @@
return
}
static_assert(getPointer()->a == 100, "");
+
+constexpr C RVOAndParams(const C *c) {
+ return C();
+}
+constexpr C RVOAndParamsResult = RVOAndParams();
+
+constexpr int locals() {
+ C c;
+ c.a = 10;
+
+ // Assignment, not an initializer.
+ // c = C(); FIXME
+ c.a = 10;
+
+
+ // Assignment, not an initializer.
+ //c = RVOAndParams(); FIXME
+
+ return c.a;
+}
+static_assert(locals() == 10, "");
Index: clang/lib/AST/Interp/InterpFrame.h
===
--- clang/lib/AST/Interp/InterpFrame.h
+++ clang/lib/AST/Interp/InterpFrame.h
@@ -35,6 +35,11 @@
InterpFrame(InterpState , Function *Func, InterpFrame *Caller,
CodePtr RetPC, Pointer &);
+ /// Creates a new frame with the values that make sense.
+ /// I.e., the caller is the current frame of S,
+ /// and the This() pointer is the current Pointer on the top of S's stack,
+ InterpFrame(InterpState , Function *Func, CodePtr RetPC);
+
/// Destroys the frame, killing all live pointers to stack slots.
~InterpFrame();
Index: clang/lib/AST/Interp/InterpFrame.cpp
===
--- clang/lib/AST/Interp/InterpFrame.cpp
+++ clang/lib/AST/Interp/InterpFrame.cpp
@@ -36,6 +36,36 @@
}
}
+InterpFrame::InterpFrame(InterpState , Function *Func, CodePtr RetPC)
+: Caller(S.Current), S(S), Func(Func), RetPC(RetPC),
+ ArgSize(Func ? Func->getArgSize() : 0),
+ Args(static_cast(S.Stk.top())), FrameOffset(S.Stk.size()) {
+ assert(Func);
+
+ // As per our calling convention, the this pointer is
+ // part of the ArgSize.
+ // If the function has RVO, the RVO pointer is first.
+ // If the fuction has a This pointer, that one is next.
+ // Then follow the actual arguments (but those are handled
+ // in getParamPointer()).
+ if (Func->hasThisPointer()) {
+if (Func->hasRVO())
+ This = stackRef(sizeof(Pointer));
+else
+ This = stackRef(0);
+ }
+
+ if (unsigned FrameSize = Func->getFrameSize()) {
+Locals = std::make_unique(FrameSize);
+for (auto : Func->scopes()) {
+ for (auto : Scope.locals()) {
+Block *B = new (localBlock(Local.Offset)) Block(Local.Desc);
+B->invokeCtor();
+ }
+}
+ }
+}
+
InterpFrame::~InterpFrame() {
if (Func && Func->isConstructor() && This.isBaseClass())
This.initialize();
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -55,8 +55,7 @@
template ::T>
static bool Call(InterpState , CodePtr , const Function *Func) {
- S.Current =
- new InterpFrame(S, const_cast(Func), S.Current, PC, {});
+ S.Current = new InterpFrame(S, const_cast(Func), PC);
APValue CallResult;
// Note that we cannot assert(CallResult.hasValue()) here since
// Ret() above only sets the APValue if the curent frame doesn't
@@ -66,8 +65,7 @@
static bool CallVoid(InterpState , CodePtr , const Function *Func) {
APValue VoidResult;
- S.Current =
- new InterpFrame(S, const_cast(Func), S.Current, PC, {});
+ S.Current = new InterpFrame(S, const_cast(Func), PC);
bool Success = Interpret(S, VoidResult);
assert(VoidResult.isAbsent());
Index: clang/lib/AST/Interp/Function.h
===
--- clang/lib/AST/Interp/Function.h
+++ clang/lib/AST/Interp/Function.h
@@ -56,6 +56,21 @@
///
/// Contains links to the bytecode of the function, as well as metadata
/// describing all arguments and stack-local variables.
+///
+/// # Calling Convention
+///
+/// When calling a function, all argument values must be on the stack.
+///
+/// If the function has a This pointer (i.e. hasThisPointer() returns true,
+/// the argument values need to be preceeded by a Pointer for the This object.
+///
+/// If the function uses Return Value Optimization, the arguments (and
+/// potentially the This pointer) need to be proceeded by a Pointer pointing
+/// to the location to construct the returned value.
+///
+/// After the function has been called, it will
[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
tbaeder added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134699/new/ https://reviews.llvm.org/D134699 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
tbaeder updated this revision to Diff 464179.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134699/new/
https://reviews.llvm.org/D134699
Files:
clang/lib/AST/Interp/ByteCodeEmitter.cpp
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/Disasm.cpp
clang/lib/AST/Interp/EvalEmitter.cpp
clang/lib/AST/Interp/Function.cpp
clang/lib/AST/Interp/Function.h
clang/lib/AST/Interp/Interp.cpp
clang/lib/AST/Interp/InterpFrame.cpp
clang/lib/AST/Interp/InterpFrame.h
clang/lib/AST/Interp/Pointer.h
clang/test/AST/Interp/records.cpp
Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -102,3 +102,24 @@
return
}
static_assert(getPointer()->a == 100, "");
+
+constexpr C RVOAndParams(const C *c) {
+ return C();
+}
+constexpr C RVOAndParamsResult = RVOAndParams();
+
+constexpr int locals() {
+ C c;
+ c.a = 10;
+
+ // Assignment, not an initializer.
+ // c = C(); FIXME
+ c.a = 10;
+
+
+ // Assignment, not an initializer.
+ //c = RVOAndParams(); FIXME
+
+ return c.a;
+}
+static_assert(locals() == 10, "");
Index: clang/lib/AST/Interp/Pointer.h
===
--- clang/lib/AST/Interp/Pointer.h
+++ clang/lib/AST/Interp/Pointer.h
@@ -317,7 +317,7 @@
/// Prints the pointer.
void print(llvm::raw_ostream ) const {
-OS << "{" << Base << ", " << Offset << ", ";
+OS << (void *)Pointee << "{" << Base << ", " << Offset << ", ";
if (Pointee)
OS << Pointee->getSize();
else
Index: clang/lib/AST/Interp/InterpFrame.h
===
--- clang/lib/AST/Interp/InterpFrame.h
+++ clang/lib/AST/Interp/InterpFrame.h
@@ -35,6 +35,11 @@
InterpFrame(InterpState , Function *Func, InterpFrame *Caller,
CodePtr RetPC, Pointer &);
+ /// Creates a new frame with the values that make sense.
+ /// I.e., the caller is the current frame of S,
+ /// and the This() pointer is the current Pointer on the top of S's stack,
+ InterpFrame(InterpState , Function *Func, CodePtr RetPC);
+
/// Destroys the frame, killing all live pointers to stack slots.
~InterpFrame();
Index: clang/lib/AST/Interp/InterpFrame.cpp
===
--- clang/lib/AST/Interp/InterpFrame.cpp
+++ clang/lib/AST/Interp/InterpFrame.cpp
@@ -36,6 +36,36 @@
}
}
+InterpFrame::InterpFrame(InterpState , Function *Func, CodePtr RetPC)
+: Caller(S.Current), S(S), Func(Func), RetPC(RetPC),
+ ArgSize(Func ? Func->getArgSize() : 0),
+ Args(static_cast(S.Stk.top())), FrameOffset(S.Stk.size()) {
+ assert(Func);
+
+ // As per our calling convention, the this pointer is
+ // part of the ArgSize.
+ // If the function has RVO, the RVO pointer is first.
+ // If the fuction has a This pointer, that one is next.
+ // Then follow the actual arguments (but those are handled
+ // in getParamPointer()).
+ if (Func->hasThisPointer()) {
+if (Func->hasRVO())
+ This = stackRef(sizeof(Pointer));
+else
+ This = stackRef(0);
+ }
+
+ if (unsigned FrameSize = Func->getFrameSize()) {
+Locals = std::make_unique(FrameSize);
+for (auto : Func->scopes()) {
+ for (auto : Scope.locals()) {
+Block *B = new (localBlock(Local.Offset)) Block(Local.Desc);
+B->invokeCtor();
+ }
+}
+ }
+}
+
InterpFrame::~InterpFrame() {
if (Func && Func->isConstructor() && This.isBaseClass())
This.initialize();
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -55,8 +55,7 @@
template ::T>
static bool Call(InterpState , CodePtr , const Function *Func) {
- S.Current =
- new InterpFrame(S, const_cast(Func), S.Current, PC, {});
+ S.Current = new InterpFrame(S, const_cast(Func), PC);
APValue CallResult;
// Note that we cannot assert(CallResult.hasValue()) here since
// Ret() above only sets the APValue if the curent frame doesn't
@@ -66,8 +65,7 @@
static bool CallVoid(InterpState , CodePtr , const Function *Func) {
APValue VoidResult;
- S.Current =
- new InterpFrame(S, const_cast(Func), S.Current, PC, {});
+ S.Current = new InterpFrame(S, const_cast(Func), PC);
bool Success = Interpret(S, VoidResult);
assert(VoidResult.isAbsent());
Index: clang/lib/AST/Interp/Function.h
===
--- clang/lib/AST/Interp/Function.h
+++ clang/lib/AST/Interp/Function.h
@@ -56,6 +56,21 @@
///
/// Contains links to the bytecode of the function, as well as metadata
/// describing all arguments and stack-local variables.
+///
+/// # Calling Convention
+///
+/// When calling a
[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods
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.
Seems like I completely forgot to post this to phab!
Implement passing the `this` pointer to member functions and constructors. The
`this` pointer is passed via the stack. This changes the functions to
explicitly track whether they have a RVO pointer and a `this` pointer.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D134699
Files:
clang/lib/AST/Interp/ByteCodeEmitter.cpp
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/Disasm.cpp
clang/lib/AST/Interp/EvalEmitter.cpp
clang/lib/AST/Interp/Function.cpp
clang/lib/AST/Interp/Function.h
clang/lib/AST/Interp/Interp.cpp
clang/lib/AST/Interp/InterpFrame.cpp
clang/lib/AST/Interp/InterpFrame.h
clang/lib/AST/Interp/Pointer.h
clang/test/AST/Interp/records.cpp
Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -102,3 +102,24 @@
return
}
static_assert(getPointer()->a == 100, "");
+
+constexpr C RVOAndParams(const C *c) {
+ return C();
+}
+constexpr C RVOAndParamsResult = RVOAndParams();
+
+constexpr int locals() {
+ C c;
+ c.a = 10;
+
+ // Assignment, not an initializer.
+ // c = C(); FIXME
+ c.a = 10;
+
+
+ // Assignment, not an initializer.
+ //c = RVOAndParams(); FIXME
+
+ return c.a;
+}
+static_assert(locals() == 10, "");
Index: clang/lib/AST/Interp/Pointer.h
===
--- clang/lib/AST/Interp/Pointer.h
+++ clang/lib/AST/Interp/Pointer.h
@@ -317,7 +317,7 @@
/// Prints the pointer.
void print(llvm::raw_ostream ) const {
-OS << "{" << Base << ", " << Offset << ", ";
+OS << (void *)Pointee << "{" << Base << ", " << Offset << ", ";
if (Pointee)
OS << Pointee->getSize();
else
Index: clang/lib/AST/Interp/InterpFrame.h
===
--- clang/lib/AST/Interp/InterpFrame.h
+++ clang/lib/AST/Interp/InterpFrame.h
@@ -35,6 +35,11 @@
InterpFrame(InterpState , Function *Func, InterpFrame *Caller,
CodePtr RetPC, Pointer &);
+ /// Creates a new frame with the values that make sense.
+ /// I.e., the caller is the current frame of S,
+ /// and the This() pointer is the current Pointer on the top of S's stack,
+ InterpFrame(InterpState , Function *Func, CodePtr RetPC);
+
/// Destroys the frame, killing all live pointers to stack slots.
~InterpFrame();
Index: clang/lib/AST/Interp/InterpFrame.cpp
===
--- clang/lib/AST/Interp/InterpFrame.cpp
+++ clang/lib/AST/Interp/InterpFrame.cpp
@@ -36,6 +36,36 @@
}
}
+InterpFrame::InterpFrame(InterpState , Function *Func, CodePtr RetPC)
+: Caller(S.Current), S(S), Func(Func), RetPC(RetPC),
+ ArgSize(Func ? Func->getArgSize() : 0),
+ Args(static_cast(S.Stk.top())), FrameOffset(S.Stk.size()) {
+ assert(Func);
+
+ // As per our calling convention, the this pointer is
+ // part of the ArgSize.
+ // If the function has RVO, the RVO pointer is first.
+ // If the fuction has a This pointer, that one is next.
+ // Then follow the actual arguments (but those are handled
+ // in getParamPointer()).
+ if (Func->hasThisPointer()) {
+if (Func->hasRVO())
+ This = stackRef(sizeof(Pointer));
+else
+ This = stackRef(0);
+ }
+
+ if (unsigned FrameSize = Func->getFrameSize()) {
+Locals = std::make_unique(FrameSize);
+for (auto : Func->scopes()) {
+ for (auto : Scope.locals()) {
+Block *B = new (localBlock(Local.Offset)) Block(Local.Desc);
+B->invokeCtor();
+ }
+}
+ }
+}
+
InterpFrame::~InterpFrame() {
if (Func && Func->isConstructor() && This.isBaseClass())
This.initialize();
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -55,8 +55,7 @@
template ::T>
static bool Call(InterpState , CodePtr , const Function *Func) {
- S.Current =
- new InterpFrame(S, const_cast(Func), S.Current, PC, {});
+ S.Current = new InterpFrame(S, const_cast(Func), PC);
APValue CallResult;
// Note that we cannot assert(CallResult.hasValue()) here since
// Ret() above only sets the APValue if the curent frame doesn't
@@ -66,8 +65,7 @@
static bool CallVoid(InterpState , CodePtr , const Function *Func) {
APValue VoidResult;
- S.Current =
- new InterpFrame(S, const_cast(Func), S.Current, PC, {});
+ S.Current = new InterpFrame(S, const_cast(Func), PC);
bool Success =
