root icon indicating copy to clipboard operation
root copied to clipboard

[cling] Provide C value printer, adopted from Ruben De Smet!

Open ferdymercury opened this issue 11 months ago • 15 comments

This Pull request:

Changes or fixes:

Rebased version of https://github.com/root-project/root/pull/9272/

Fixes root-project/cling#364 Fixes root-project/cling#419 and helps for https://github.com/jupyter-xeus/xeus-cling/issues/404

ferdymercury avatar Jan 14 '25 08:01 ferdymercury

It seems that this PR does not increase the number of errors in clingtest-alma9, taking as reference the errors in https://github.com/root-project/root/pull/16917

ferdymercury avatar Jan 14 '25 10:01 ferdymercury

@bellenot Do you happen to know why check-cling doesn't work in-tree for Windows? thks!

ferdymercury avatar Jan 14 '25 11:01 ferdymercury

Test Results

    20 files      20 suites   3d 23h 30m 19s ⏱️  3 656 tests  3 654 ✅ 0 💤 2 ❌ 71 438 runs  71 431 ✅ 5 💤 2 ❌

For more details on these failures, see this check.

Results for commit 3176b352.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Jan 14 '25 12:01 github-actions[bot]

It looks like clingtest-check-cling fails on several platforms...

bellenot avatar Jan 20 '25 08:01 bellenot

It looks like clingtest-check-cling fails on several platforms...

Sorry, what I meant is that, on Windows, it looks as if it doesn't even compile. It gives this error:

  MSBuild version 17.9.5+33de0b227 for .NET Framework
  MSBUILD : error MSB1009: Project file does not exist.
  Switch: check-cling.vcxproj
  CMake Error at C:/ROOT-CI/src/cmake/modules/RootTestDriver.cmake:232 (message):
    error code: 1

(On other platforms, it fails because of a specific subtest does not pass.)

ferdymercury avatar Jan 20 '25 08:01 ferdymercury

It looks like clingtest-check-cling fails on several platforms...

Sorry, what I meant is that, on Windows, it looks as if it doesn't even compile. It gives this error:

  MSBuild version 17.9.5+33de0b227 for .NET Framework
  MSBUILD : error MSB1009: Project file does not exist.
  Switch: check-cling.vcxproj
  CMake Error at C:/ROOT-CI/src/cmake/modules/RootTestDriver.cmake:232 (message):
    error code: 1

(On other platforms, it fails because of a specific subtest does not pass.)

OK, I'll check the branch locally (but can't test the CI related changes...)

bellenot avatar Jan 20 '25 08:01 bellenot

OK, I'll check the branch locally (but can't test the CI related changes...)

Thanks. Maybe you don't need to check out any specific branch, just recompiling master with flag -Dclingtest=ON might show it.

ferdymercury avatar Jan 20 '25 09:01 ferdymercury

Well, it cannot work on Windows. The PATH is wrong, the command is wrong, and then, when fixing those errors, even running the test fails with:

 C:\Python311\python.exe: can't open file 'C:\\root-dev\\build\\x64\\cvalueprinter\\interpreter\\llvm-project\\llvm\\bin\\llvm-lit.py': [Errno 2] No such file or directory

Still working on it...

bellenot avatar Jan 20 '25 11:01 bellenot

Was all this designed to run on Windows at all?:

   Running the Cling regression tests
1:   llvm-lit.py: C:\root-dev\github\cvalueprinter\interpreter\llvm-project\llvm\utils\lit\lit\llvm\config.py:57: note: using lit tools: C:\Program Files\Git\usr\bin
1:   llvm-lit.py: C:\root-dev\github\cvalueprinter\interpreter\llvm-project\llvm\utils\lit\lit\TestingConfig.py:152: fatal: unable to parse config file 'C:/root-dev/github/cvalueprinter/interpreter/cling/test/lit.cfg', traceback: Traceback (most recent call last):
1:     File "C:\root-dev\github\cvalueprinter\interpreter\llvm-project\llvm\utils\lit\lit\TestingConfig.py", line 140, in load_from_path
1:       exec(compile(data, path, "exec"), cfg_globals, None)
1:     File "C:/root-dev/github/cvalueprinter/interpreter/cling/test/lit.cfg", line 20, in <module>
1:       if IsWindows and not execute_external:
1:                            ^^^^^^^^^^^^^^^^
1:   NameError: name 'execute_external' is not defined

And I would need to patch LLVM, so not sure I'm going to embark into this...

bellenot avatar Jan 20 '25 11:01 bellenot

I would suggest to completely disable it on Windows, like:

diff --git a/interpreter/CMakeLists.txt b/interpreter/CMakeLists.txt
index 419ff4028a..3d1319b414 100644
--- a/interpreter/CMakeLists.txt
+++ b/interpreter/CMakeLists.txt
@@ -114,8 +114,10 @@ if(clingtest)
   if (NOT builtin_llvm)
     set(CLINGTEST_EXECUTABLE CLING=${CMAKE_CURRENT_BINARY_DIR}/llvm-project/llvm/${CMAKE_CFG_INTDIR}/bin/cling)
   endif()
-  ROOT_ADD_TEST(clingtest-check-cling COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --target check-cling
-                                       ENVIRONMENT ${CLINGTEST_EXECUTABLE})
+  if(NOT MSVC)
+    ROOT_ADD_TEST(clingtest-check-cling COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --target check-cling
+                                         ENVIRONMENT ${CLINGTEST_EXECUTABLE})
+  endif()
 else()
   #---Build LLVM/Clang with symbol visibility=hidden--------------------------------------------------
   set(CMAKE_CXX_VISIBILITY_PRESET hidden)

And remove the windows related changes in your PR, unless someone disagrees (@vgvassilev ?)

bellenot avatar Jan 20 '25 13:01 bellenot

Ok. But how can I then check if the test I changed in this PR gives a correct result in Windows or breaks sth? Or could you do that locally for this PR? (I have no Windows machine to check)

ferdymercury avatar Jan 20 '25 13:01 ferdymercury

Ok. But how can I then check if the test I changed in this PR gives a correct result in Windows or breaks sth? Or could you do that locally for this PR? (I have no Windows machine to check)

I can do it locally (hopefully tomorrow)

bellenot avatar Jan 20 '25 13:01 bellenot

@ferdymercury BTW, interpreter\cling\test\Driver\C.c is not going to work on Windows the way it is:

// RUN: cat %s | %cling -x c -Xclang -verify 2>&1 | FileCheck %s
// RUN: cat %s | %cling -x c -fsyntax-only -Xclang -verify 2>&1

There is no cat on Windows, and I don't even know if the flags are supported. Anyway, I'll build cling (since it is not built with ROOT) and run it on Windows (I'll find a way) and I'll let you know

bellenot avatar Jan 20 '25 14:01 bellenot

So here is what I get on Windows with your changes:

C:\root-dev\build\x64\llvm-project>type C.c | cling -x c -Xclang -verify

***************** CLING *****************
* Type C code and press enter to run it *
*            Type .q to exit            *
*****************************************
CHECK 123 0000007E45B8FA40
CHECK 4
error: 'expected-error' diagnostics seen but not expected:
  File input_line_14 Line 3: ValueExtractionSynthesizer could not find: 'cling namespace'.
  File input_line_15 Line 2: ValueExtractionSynthesizer could not find: 'cling namespace'.
error: 'expected-warning' diagnostics seen but not expected:
  File input_line_1 Line 5: omitting the parameter name in a function definition is a C23 extension
  File input_line_1 Line 5: omitting the parameter name in a function definition is a C23 extension

C:\root-dev\build\x64\llvm-project>type C.c | cling -x c -fsyntax-only -Xclang -verify

***************** CLING *****************
* Type C code and press enter to run it *
*            Type .q to exit            *
*****************************************
error: 'expected-error' diagnostics seen but not expected:
  File input_line_14 Line 3: ValueExtractionSynthesizer could not find: 'cling namespace'.
  File input_line_15 Line 2: ValueExtractionSynthesizer could not find: 'cling namespace'.

C:\root-dev\build\x64\llvm-project>

But at least it doesn't print these errors anymore:

error: 'expected-error' diagnostics seen but not expected:
  File input_line_14 Line 3: ValueExtractionSynthesizer could not find: 'cling::runtime::internal::setValueNoAlloc'.
  File input_line_15 Line 2: ValueExtractionSynthesizer could not find: 'cling::runtime::internal::setValueNoAlloc'.

bellenot avatar Jan 21 '25 10:01 bellenot

Thanks a lot Bertrand for checking!

ferdymercury avatar Jan 21 '25 10:01 ferdymercury