[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

[Axel Naumann via Phabricator via cfe-commits]

karies added inline comments.


================
Comment at: clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt:2
+# The interpreter can throw an exception from user input. The test binary needs
+# to be compiled with exception support to expect and catch the thrown
+# exception.
----------------
I don't understand the term "to expect" the thrown exception. Please ignore if that's a known term of art.


================
Comment at: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:93
+static void ThrowerAnError(const char* Name) {
+  throw std::runtime_error(Name);
+}
----------------
Why not just `throw Name;` to avoid `#include <stdexcept>`?


================
Comment at: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:100
+  } catch (const std::exception& E) {
+    printf("Caught: '%s'\n", E.what());
+  } catch (...) {
----------------
Consider fwd declaring `printf` to avoid inclusion of `stdio.h`.


================
Comment at: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:102
+  } catch (...) {
+    printf("Unknown exception\n");
+  }
----------------
How is that provoking a test failure? What about `exit(1)` or whatever works for gtest?


================
Comment at: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:104
+  }
+  ThrowerAnError("From JIT");
+  return 0;
----------------
To me, the wording difference between "In JIT" and "From JIT" doesn't signal anything. Maybe "the first could be "To be caught in JIT" and the second "To be caught in binary"?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107049/new/

https://reviews.llvm.org/D107049