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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to