Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.71%. Comparing base (33bfa39) to head (57ab2c0).
Report is 1 commits behind head on main.

@@           Coverage Diff           @@
##             main     #483   +/-   ##
=======================================
  Coverage   72.71%   72.71%           
=======================================
  Files           9        9           
  Lines        3618     3618           
=======================================
  Hits         2631     2631           
  Misses        987      987           

I also just want to make it clear just like I did in our private Discord chat that the goal of this PR is not to have all tests passing.

Hi,

just curious here. Do we atleast know why some work and some don't ?

In the case of the crashing tests it almost always due to memory access issues. In the case of the few failing tests, the tests do run, but they don't give the result that what is expected. It could be said that in the Emscripten case, a different result is to be expected for some of these tests, but I do not plan to fix the tests as part of this PR.

I also just want to make it clear just like I did in our private Discord chat that the goal of this PR is not to have all tests passing.

Hi,

just curious here. Do we atleast know why some work and some don't ?

Also thought I'd mentioned that @Vipul-Cariappa mentioned that the failing tests do give some explanation as to why an Emscripten version of cppyy doesn't work at the moment. I believe he plans to investigate them in the future as part of his work to get a working Emscripten build of cppyy, once he has time. He is busy with other things at the moment though.

Regarding my last 3 reviews. Obviously as you said the goal might not be to get all tests runnings here, so no issues.

I am just trying to get an idea on why something would not work.

For eg

1. regarding the first review ... we've marked an evaluate call as "fails for emscripten" but evaluate technically just calls process and we haven't had any problems with process. So should kinda track down what might be going wrong here !

Maybe we might be overlooking something obvious 😬

Hey @mcbarton

I just realized #440 went in. This should also do the job for us and expose all symbols from the C API we need.

So we no longer need #518 (and I am closing that issue)

Dumping all the symbols should be enough to see that they are being exposed correctly

anutosh491@Anutoshs-MacBook-Air lib % emnm --debug-syms libclangCppInterOp.so > test.txt

004b3c48 T _Z14linkComponentsv
.........
0048640d T clang_Interpreter_addIncludePath
0048636a T clang_Interpreter_addSearchPath
004864a4 T clang_Interpreter_declare
00486308 T clang_Interpreter_dispose
004867a4 T clang_Interpreter_evaluate
004861f7 T clang_Interpreter_getClangInterpreter
00486a6f T clang_Interpreter_loadLibrary
004868c7 T clang_Interpreter_lookupLibrary
00486625 T clang_Interpreter_process
00486234 T clang_Interpreter_takeInterpreterAsPtr
00486249 T clang_Interpreter_undo
00486b8f T clang_Interpreter_unloadLibrary
00486789 T clang_Value_dispose
0048786f T clang_allocate
00487885 T clang_construct
0048609d T clang_createInterpreter
004861e1 T clang_createInterpreterFromRawPtr
0048674c T clang_createValue
0048787a T clang_deallocate
00487a63 T clang_destruct
004874f2 T clang_existsFunctionTemplate
00486e15 T clang_getComplexType
00486f70 T clang_getDefaultConstructor
00487103 T clang_getDestructor
00487241 T clang_getFunctionSignature
00486c2e T clang_getTypeAsString
00486f29 T clang_hasDefaultConstructor
0048769e T clang_instantiateTemplate
004878d3 T clang_invoke
0048749f T clang_isTemplatedFunction
00486f1b T clang_scope_dump
............
0401e06d T llvm_strlcpy

If you rebase your PR on latest main and remove all symbols relevant to the c API (https://github.com/compiler-research/CppInterOp/pull/483/files#diff-054935aee7c0887220251f89b2179dbc4a0c418a69bca9f74854305ae7c0f967R31)
we should be good here.

P.S: That should leave us with only symbols coming of static libs from clang/llvm