Skip to content
5 changes: 5 additions & 0 deletions include/clang/Interpreter/CppInterOp.h
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,11 @@ namespace Cpp {
unsigned complete_line = 1U,
unsigned complete_column = 1U);

/// Reverts the last N operations performed by the interpreter.
///\param[in] N The number of operations to undo. Defaults to 1.
///\returns 0 on success, non-zero on failure.
CPPINTEROP_API int Undo(unsigned N = 1);

} // end namespace Cpp

#endif // CPPINTEROP_CPPINTEROP_H
5 changes: 4 additions & 1 deletion lib/Interpreter/CXCppInterOp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,10 @@ TInterp_t clang_Interpreter_takeInterpreterAsPtr(CXInterpreter I) {

enum CXErrorCode clang_Interpreter_undo(CXInterpreter I, unsigned int N) {
#ifdef CPPINTEROP_USE_CLING
return CXError_Failure;
auto* interp = getInterpreter(I);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If using clang_repl you can return CXError_Failure if it fails. With cling and the c api currently as it stands you cannot. Can you fix this @kr-2003

cling::Interpreter::PushTransactionRAII RAII(interp);
interp->unload(N);
return CXError_Success;
#else
return getInterpreter(I)->Undo(N) ? CXError_Failure : CXError_Success;
#endif // CPPINTEROP_USE_CLING
Expand Down
11 changes: 11 additions & 0 deletions lib/Interpreter/CppInterOp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3684,4 +3684,15 @@ namespace Cpp {
complete_column);
}

int Undo(unsigned N) {
#ifdef CPPINTEROP_USE_CLING
auto& I = getInterp();
cling::Interpreter::PushTransactionRAII RAII(&I);
I.unload(N);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vgvassilev How do I even check if unload function logs any errors, since its a void function? Should I use try-catch block(but it is disabled in the project) or use system level redirection of the output to stream using pipes(this might be an overkill for capturing)? unload function logs cling::errs(), if it fails to run.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vgvassilev Could you please guide me on how to proceed here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should assume that the operation was successful.

Copy link
Collaborator

@anutosh491 anutosh491 Mar 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return compat::Interpreter::kSuccess;
#else
return getInterp().undo(N);
#endif
}

} // end namespace Cpp
9 changes: 9 additions & 0 deletions lib/Interpreter/CppInterOpInterpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,15 @@ class Interpreter {
return ret; // TODO: Implement
}

CompilationResult undo(unsigned N = 1) {
if (llvm::Error Err = Undo(N)) {
llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
"Failed to undo via ::undo");
return kFailure;
}
return kSuccess;
}

}; // Interpreter
} // namespace Cpp

Expand Down
27 changes: 27 additions & 0 deletions unittests/CppInterOp/FunctionReflectionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1615,3 +1615,30 @@ TEST(FunctionReflectionTest, Destruct) {
clang_Interpreter_takeInterpreterAsPtr(I);
clang_Interpreter_dispose(I);
}

TEST(FunctionReflectionTest, UndoTest) {
#ifdef _WIN32
GTEST_SKIP() << "Disabled on Windows. Needs fixing.";
#endif
#ifdef EMSCRIPTEN
GTEST_SKIP() << "Test fails for Emscipten builds";
#else
Cpp::CreateInterpreter();
EXPECT_EQ(Cpp::Process("int a = 5;"), 0);
EXPECT_EQ(Cpp::Process("int b = 10;"), 0);
EXPECT_EQ(Cpp::Process("int x = 5;"), 0);
Cpp::Undo();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe unloading this first transaction here is problematic for destructors, and is unsupported by Cling, which can be seen in the error thrown in the logs. We should drop this call to Cpp::Undo()

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kr-2003 can you address this review comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added two more instructions before calling the first Cpp::Undo().

EXPECT_NE(Cpp::Process("int y = x;"), 0);
EXPECT_EQ(Cpp::Process("int x = 10;"), 0);
EXPECT_EQ(Cpp::Process("int y = 10;"), 0);
Cpp::Undo(2);
EXPECT_EQ(Cpp::Process("int x = 20;"), 0);
EXPECT_EQ(Cpp::Process("int y = 20;"), 0);
int ret = Cpp::Undo(100);
#ifdef CPPINTEROP_USE_CLING
EXPECT_EQ(ret, 0);
#else
EXPECT_EQ(ret, 1);
#endif
#endif
}
Loading