Add cling 1.2 support #580

mcbarton · 2025-05-16T09:25:59Z

Description

Please include a summary of changes, motivation and context for this PR.

This PR makes CppInterOp compatible with cling 1.2, and jobs the cling 1.0 jobs. I will remove cling 1.0 support from CppInterOp in a separate PR soon.

Fixes # (issue)

Type of change

Please tick all options which are relevant.

Bug fix
New feature
Requires documentation updates

Testing

Please describe the test(s) that you added and ran to verify your changes.

Checklist

I have read the contribution guide recently

mcbarton · 2025-05-16T09:43:09Z

As this PR will involve deleting things from the cache, once this PR has been approved, please just leave it to me to merge, since I know exactly what needs to be deleted before merging.

aaronj0 · 2025-05-16T12:27:38Z

Hi @mcbarton, looks like this build won't work unless we move to cling master or wait for the v1.3 release. If you don't mind tracking a patch for the timebeing, I can push that here. I do still face some weird issues with googletest in cmake, but I'm sure you can take it from there.

aaronj0 · 2025-05-16T13:10:11Z

I do still face some weird issues with googletest in cmake, but I'm sure you can take it from there.

@mcbarton never mind that, came from me not passing the CMAKE_BUILD_TYPE, which was an error on my end. Now with the patch, lets see if the jobs pass

mcbarton · 2025-05-16T13:16:13Z

Just a note that as part of this PR we will want to update the documentation too, to remove references to cling 1.0, and mention the need for the patch if they use cling 1.2 . I plan to put in a PR on Monday to eliminate the code needed for 1.0 cling support.

codecov · 2025-05-16T14:15:05Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.36%. Comparing base (6951212) to head (843a3d4).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #580   +/-   ##
=======================================
  Coverage   76.36%   76.36%           
=======================================
  Files           9        9           
  Lines        3655     3655           
=======================================
  Hits         2791     2791           
  Misses        864      864

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

mcbarton · 2025-05-16T15:07:14Z

@aaronj0 @Vipul-Cariappa The macOS cling 1.2 job passes for CppInterOp with the new patch. To get the whole job to pass an identical PR will need to be opened in cppyy to fix the test tags there.

mcbarton · 2025-05-16T16:17:52Z

DynamicLibraryManagerTest.Sanity is failing for Windows cling 1.2 . That test will need to be disabled for Windows cling llvm = 18 . I am away from my computer this weekend, so this will need to be taken care of by someone else if needed before Monday.

github-actions · 2025-05-16T17:14:41Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2025-05-16T17:19:03Z

clang-tidy review says "All clean, LGTM! 👍"

unittests/CppInterOp/DynamicLibraryManagerTest.cpp

github-actions · 2025-05-20T08:55:50Z

clang-tidy review says "All clean, LGTM! 👍"

lib/Interpreter/Compatibility.h

github-actions

clang-tidy made some suggestions

github-actions · 2025-05-20T09:09:14Z

unittests/CppInterOp/DynamicLibraryManagerTest.cpp

@@ -1,5 +1,6 @@
 #include "gtest/gtest.h"


warning: 'gtest/gtest.h' file not found [clang-diagnostic-error]

#include "gtest/gtest.h" ^

aaronj0 · 2025-05-20T11:23:07Z

@mcbarton I disagree with you dropping my commit updating the LLJIT guards on this PR. This is code I worked on earlier and something I wanted to address with a specific commit and a functional description as to what changed there. Either way, opening PRs with broad goals, expecting others to fix all the issues on those PRs (which have been doing) and then dropping their commits gets in the way of people who work on these things as a part of their job, and is in poor taste.

mcbarton · 2025-05-20T11:58:50Z

@mcbarton I disagree with you dropping my commit updating the LLJIT guards on this PR. This is code I worked on earlier and something I wanted to address with a specific commit and a functional description as to what changed there. Either way, opening PRs with broad goals, expecting others to fix all the issues on those PRs (which have been doing) and then dropping their commits gets in the way of people who work on these things as a part of their job, and is in poor taste.

I didn't mean to cause any offence. I certainly am not expecting you solve the issues on PRs I open (I am happy for you to close this PR, and open up a completely new one). I did it because I see updating to cling 1.2 to be separate to dropping support for 1.0 . I saw this commit as something which didn't have anything to do with adding 1.2 support, and more to do with dropping 1.0 support (and I had already made this change in my 1.0 cling support dropping PR).

aaronj0 · 2025-05-21T12:51:24Z

I didn't mean to cause any offence. I certainly am not expecting you solve the issues on PRs I open (I am happy for you to close this PR, and open up a completely new one). I did it because I see updating to cling 1.2 to be separate to dropping support for 1.0 . I saw this commit as something which didn't have anything to do with adding 1.2 support, and more to do with dropping 1.0 support (and I had already made this change in my 1.0 cling support dropping PR).

No worries, I have updated the Windows failures, and we have the cppyy tags ready here: compiler-research/cppyy#145. We can merge this since all jobs fail on the cppyy step, and follow up on the cppyy side for the tags. I will also open a PR with the patch dropped here, so we no longer have that pointer arithmetic for Cling in ROOT with the immediate CppInterOp release.

github-actions

clang-tidy made some suggestions

github-actions · 2025-05-21T13:06:53Z

unittests/CppInterOp/ScopeReflectionTest.cpp

@@ -163,6 +163,10 @@ TEST(ScopeReflectionTest, SizeOf) {


 TEST(ScopeReflectionTest, IsBuiltin) {
+#if CLANG_VERSION_MAJOR == 18 && defined(CPPINTEROP_USE_CLING) && defined(_WIN32) && (defined(_M_ARM) || defined(_M_ARM64))


warning: no header providing "CLANG_VERSION_MAJOR" is directly included [misc-include-cleaner]

unittests/CppInterOp/ScopeReflectionTest.cpp:21:

- #include <string> + #include <clang/Basic/Version.h> + #include <string>

github-actions

clang-tidy made some suggestions

unittests/CppInterOp/ScopeReflectionTest.cpp

github-actions

clang-tidy made some suggestions

unittests/CppInterOp/TypeReflectionTest.cpp

github-actions · 2025-05-21T15:14:16Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2025-05-21T15:32:36Z

clang-tidy review says "All clean, LGTM! 👍"

github-actions · 2025-05-21T15:53:49Z

clang-tidy review says "All clean, LGTM! 👍"

mcbarton · 2025-05-21T16:05:58Z

@vgvassilev @aaronj0 can one of you approve this PR now that the Windows arm part of the ci passes? I'm planning on dealing with deleting old unneeded cache builds tonight and merge together with this PR #583 which removes the cross compile emscripten patch, when things are quiet, since both PRs are likely to be disruptive while the cache rebuilds. @aaronj0 can rerun his cppyy PR tomorrow to show that it passes after this goes in.

I will rebase my PR which removes cling 1.0 support on top of this one, with a plan for that PR to be ready either tomorrow or Friday.

mcbarton · 2025-05-21T16:06:25Z

I will squash the commits on merge if approved.

Co-authored-by: mcbarton <[email protected]>

aaronj0

LGTM! I have rebased on main and squashed into meaningful commits, we can merge this once the jobs pass, and move on to updating cppyy. Thanks for this effort @mcbarton

aaronj0 · 2025-05-22T07:14:30Z

LGTM! I have rebased on main and squashed into meaningful commits, we can merge this once the jobs pass, and move on to updating cppyy. Thanks for this effort @mcbarton

I guess there is no point in waiting for the cache to rebuild again, so I am going ahead and merging this PR

github-actions · 2025-05-22T07:17:33Z

clang-tidy review says "All clean, LGTM! 👍"

mcbarton mentioned this pull request May 16, 2025

Update cling in ci to version 1.1 using roots llvm 16 #358

Closed

Vipul-Cariappa self-requested a review May 16, 2025 09:32

mcbarton force-pushed the Add-cling-1.2-support branch 2 times, most recently from bfee029 to ea1c61f Compare May 16, 2025 09:39

mcbarton requested review from vgvassilev and aaronj0 May 16, 2025 09:44

mcbarton force-pushed the Add-cling-1.2-support branch from ea1c61f to 90d4bf0 Compare May 16, 2025 10:13

aaronj0 force-pushed the Add-cling-1.2-support branch from 5cbcf3d to 2fe65fc Compare May 16, 2025 14:20

aaronj0 force-pushed the Add-cling-1.2-support branch from cf3aa84 to dfaf2bd Compare May 16, 2025 17:13

mcbarton commented May 16, 2025

View reviewed changes

unittests/CppInterOp/DynamicLibraryManagerTest.cpp Outdated Show resolved Hide resolved

mcbarton mentioned this pull request May 19, 2025

Remove cling 1.0 support + Make llvm 16 minimum version #581

Open

4 tasks

aaronj0 force-pushed the Add-cling-1.2-support branch from dfaf2bd to 155edcf Compare May 20, 2025 08:49

mcbarton commented May 20, 2025

View reviewed changes

lib/Interpreter/Compatibility.h Outdated Show resolved Hide resolved

aaronj0 force-pushed the Add-cling-1.2-support branch from 155edcf to f7f9c63 Compare May 20, 2025 09:03

github-actions bot reviewed May 20, 2025

View reviewed changes

mcbarton force-pushed the Add-cling-1.2-support branch from c46b04b to 232a4a8 Compare May 20, 2025 10:08

aaronj0 force-pushed the Add-cling-1.2-support branch from 232a4a8 to 693d288 Compare May 21, 2025 12:48

github-actions bot reviewed May 21, 2025

View reviewed changes

aaronj0 force-pushed the Add-cling-1.2-support branch from 693d288 to 1a4f0aa Compare May 21, 2025 14:29

github-actions bot reviewed May 21, 2025

View reviewed changes

unittests/CppInterOp/ScopeReflectionTest.cpp Outdated Show resolved Hide resolved

github-actions bot reviewed May 21, 2025

View reviewed changes

unittests/CppInterOp/TypeReflectionTest.cpp Show resolved Hide resolved

mcbarton and others added 2 commits May 22, 2025 09:01

Add cling 1.2 support

9202c34

[patch] Fix ci for Cling v1.2

54ce2e8

aaronj0 force-pushed the Add-cling-1.2-support branch from 843a3d4 to 130b481 Compare May 22, 2025 07:09

[test] disable failing tests on Win32 ARM for Cling

82069ff

Co-authored-by: mcbarton <[email protected]>

aaronj0 force-pushed the Add-cling-1.2-support branch from 130b481 to 82069ff Compare May 22, 2025 07:11

aaronj0 approved these changes May 22, 2025

View reviewed changes

aaronj0 merged commit e5d06de into main May 22, 2025
33 of 47 checks passed

Vipul-Cariappa mentioned this pull request May 22, 2025

[test] update tags compiler-research/cppyy#146

Merged

vgvassilev deleted the Add-cling-1.2-support branch May 27, 2025 07:09

		@@ -163,6 +163,10 @@ TEST(ScopeReflectionTest, SizeOf) {


		TEST(ScopeReflectionTest, IsBuiltin) {
		#if CLANG_VERSION_MAJOR == 18 && defined(CPPINTEROP_USE_CLING) && defined(_WIN32) && (defined(_M_ARM) \|\| defined(_M_ARM64))

Add cling 1.2 support #580

Add cling 1.2 support #580

Uh oh!

Conversation

mcbarton commented May 16, 2025

Description

Type of change

Testing

Checklist

Uh oh!

mcbarton commented May 16, 2025

Uh oh!

aaronj0 commented May 16, 2025

Uh oh!

aaronj0 commented May 16, 2025

Uh oh!

mcbarton commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mcbarton commented May 16, 2025

Uh oh!

mcbarton commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented May 16, 2025

Uh oh!

github-actions bot commented May 16, 2025

Uh oh!

Uh oh!

github-actions bot commented May 20, 2025

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot May 20, 2025

Choose a reason for hiding this comment

Uh oh!

aaronj0 commented May 20, 2025

Uh oh!

mcbarton commented May 20, 2025

Uh oh!

aaronj0 commented May 21, 2025

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot May 21, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented May 21, 2025

Uh oh!

github-actions bot commented May 21, 2025

Uh oh!

github-actions bot commented May 21, 2025

Uh oh!

mcbarton commented May 21, 2025

Uh oh!

mcbarton commented May 21, 2025

Uh oh!

aaronj0 left a comment

Choose a reason for hiding this comment

Uh oh!

aaronj0 commented May 22, 2025

Uh oh!

Uh oh!

github-actions bot commented May 22, 2025

Uh oh!

mcbarton commented May 16, 2025 •

edited

Loading

codecov bot commented May 16, 2025 •

edited

Loading

mcbarton commented May 16, 2025 •

edited

Loading