Replace printf with cout in JitCall tests #501
Conversation
|
clang-tidy review says "All clean, LGTM! 👍" |
aaronj0
force-pushed
the
funccall-test
branch
from
d110b29 to
b49ee3d
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
| // Here we replace printf with cout for JitCall tests on Windows, where the stdout buffer is not flushed when we utilise JitCall::Invoke | ||
| // This hints towards a larger underlying problem in the driver when going through cling: On windows, the `stdout` buffer is not flushed. With gtest, all the printf statements in functions invoked with `JitCall` like: | ||
| // `void f2(std::string &s) { std::cout << s.c_str(); };` appear at the end of the test suite run. Attempting to fix this issue by force flushing does not work, and printf statements in the test code itself appear on the screen. This indicates issues with linking, since the Interpreter runtime operating on a different buffer than the compile runtime. We ideally want to support the usage of printf statements when invoked through `JitCall`. | ||
|
|
There was a problem hiding this comment.
I'd open an issue with that text and mark disable these tests on windows.
|
clang-tidy review says "All clean, LGTM! 👍" |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #501 +/- ##
=======================================
Coverage 71.17% 71.17%
=======================================
Files 9 9
Lines 3552 3552
=======================================
Hits 2528 2528
Misses 1024 1024 |
|
@aaronj0 @vgvassilev Basically anything that tries to write to stdout and stdcerr will fail on Windows. Tests that try to do this are the ones I had to disable previously, to remove continue on error on Windows. This is not just a Cling issue. The PR I am referencing is here https://github.com/compiler-research/CppInterOp/pull/313/files#diff-8c5a89c16b6f30f9e3333ab9dd311a7b915d32a4acf44fdd8fa4b5077f99dfca |
aaronj0
force-pushed
the
funccall-test
branch
from
9efcd09 to
3b91c7b
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
aaronj0
force-pushed
the
funccall-test
branch
from
3b91c7b to
36a1258
Compare
| @@ -875,9 +875,14 @@ TEST(FunctionReflectionTest, GetFunctionCallWrapper) { | |||
|
|
|||
| GetAllTopLevelDecls(code, Decls); | |||
|
|
|||
| // Here we replace printf with cout for JitCall tests on Windows, where the stdout buffer is not flushed when we utilise JitCall::Invoke | |||
There was a problem hiding this comment.
@mcbarton, how could clang-format be happy with these lengthy lines?
There was a problem hiding this comment.
@vgvassilev You can adjust ColumnLimit (see https://stackoverflow.com/questions/47683910/can-you-set-clang-formats-line-length)
There was a problem hiding this comment.
If your asking why doesn't it complain, then its because for some reason the clang-format workflow isn't detecting the changed files (see https://github.com/compiler-research/CppInterOp/actions/runs/13305769758/job/37156307205?pr=501#step:5:25)
There was a problem hiding this comment.
I think maybe we exclude the folder from formatting... which is probably fine...
There was a problem hiding this comment.
Ah yes your right. We don't technically disclude it, but only reference the include and lib folder in the clang-format command (see https://github.com/compiler-research/CppInterOp/blob/b797dbb4c8134349a1207d01b24dc3c4d93158de/.github/workflows/clang-format.yml#L44C1-L44C93). Maybe its something that was overlooked in the workflow.
There was a problem hiding this comment.
I think that was intentional.
|
clang-tidy review says "All clean, LGTM! 👍" |
This hints towards a larger underlying problem in the driver when going through cling: On windows, the `stdout` buffer is not flushed. With gtest, all the printf statements in functions invoked with `JitCall` like:
`void f2(std::string &s) { std::cout << s.c_str(); };` appear at the end of the test suite run. Attempting to fix this issue by force flushing does not work, and printf statements in the test code itself appear on the screen. This indicates issues with linking, since the Interpreter runtime operating on a different buffer than the compile runtime. We ideally want to support the usage of printf statements when invoked through `JitCall`.
aaronj0
force-pushed
the
funccall-test
branch
from
36a1258 to
6e9dafe
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
This hints towards a larger underlying problem in the driver when going through cling: On windows, the
stdoutbuffer is not flushed. With gtest, all the printf statements in functions invoked withJitCalllike:void f2(std::string &s) { std::cout << s.c_str(); };appear at the end of the test suite run. Attempting to fix this issue by force flushing does not work, and printf statements in the test code itself appear on the screen. This indicates issues with linking, since the Interpreter runtime operating on a different buffer than the compile runtime. We ideally want to support the usage of printf statements when invoked throughJitCall