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

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.


================
Comment at: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:101-105
+  // FIXME: Somehow when we build this test in release mode argc is not 0.
+  // printf("%d\n", argc);
+  // for (int I = 0; I < argc; ++I)
+  //   printf("arg[%d]='%s'\n", I, argv[I]);
+
----------------
rsmith wrote:
> Isn't this because you're passing no arguments to `main` when you call it later in the test? You're not passing any arguments on line 123/124.
Ah, indeed. Thanks!


================
Comment at: clang/unittests/Interpreter/ExceptionTests/InterpreterExceptionTest.cpp:123-124
+  testing::internal::CaptureStdout();
+  auto Main = (int (*)(...))llvm::cantFail(Interp->getSymbolAddress("main"));
+  EXPECT_THROW(Main(), std::exception);
+  std::string CapturedStdOut = testing::internal::GetCapturedStdout();
----------------
rsmith wrote:
> I think this should probably be cast to `int(*)(int, const char**)` instead. But the name `main` is special, and might not have its declared type, so selecting a different function name would have less risk of weird behavior. I'd also suggest you remove the parameters if you're not going to use them.
Fixed. The past intent was to keep it as close as possible to the original example.


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

https://reviews.llvm.org/D107049