Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/root-ci-config/build_root.py
Original file line number Diff line number Diff line change
@@ -77,7 +77,8 @@ def main():
# file below overwrites values from above
**load_config(f'{this_script_dir}/buildconfig/{args.platform}.txt')
}

if WINDOWS and pull_request:
options_dict["clingtest"] = 'on' if args.clingtest else 'off'
options = build_utils.cmake_options_from_dict(options_dict)

if WINDOWS:
@@ -197,6 +198,7 @@ def parse_args():
parser.add_argument("--dockeropts", default=None, help="Extra docker options, if any")
parser.add_argument("--incremental", default="false", help="Do incremental build")
parser.add_argument("--buildtype", default="Release", help="Release|Debug|RelWithDebInfo")
parser.add_argument("--clingtest", default="false", help="Build with -Dclingtest=ON for Windows PR CI")
parser.add_argument("--coverage", default="false", help="Create Coverage report in XML")
parser.add_argument("--sha", default=None, help="sha that triggered the event")
parser.add_argument("--base_ref", default=None, help="Ref to target branch")
@@ -212,6 +214,7 @@ def parse_args():

# Set argument to True if matched
args.incremental = args.incremental.lower() in ('yes', 'true', '1', 'on')
args.clingtest = args.clingtest.lower() in ('yes', 'true', '1', 'on')
args.coverage = args.coverage.lower() in ('yes', 'true', '1', 'on')
args.binaries = args.binaries.lower() in ('yes', 'true', '1', 'on')

50 changes: 45 additions & 5 deletions .github/workflows/root-ci.yml
Original file line number Diff line number Diff line change
@@ -77,7 +77,36 @@ concurrency:
cancel-in-progress: true

jobs:
git_diff:
name: Detect changes in Cling
# https://www.meziantou.net/executing-github-actions-jobs-or-steps-only-when-specific-files-change.htm
runs-on: ubuntu-latest
# Declare output for next jobs
outputs:
interpreter_changed: ${{ steps.check_file_changed.outputs.interpreter_changed }}
steps:
- uses: actions/checkout@v4
with:
# Checkout as many commits as needed for the diff
fetch-depth: 2
- shell: pwsh
id: check_file_changed
run: |
# Diff HEAD with the latest commit of master
$diff = git diff --name-only HEAD^ HEAD
# (works with HEAD^ as the last commit fetched is virtual merge commit by GH which shows the full diff for the PR.)

# Check if a file under interpreter/ has changed
$SourceDiff = $diff | Where-Object { $_ -match '^interpreter/' }
$HasDiff = $SourceDiff.Length -gt 0

# Set the output named "interpreter_changed"
Write-Host "::set-output name=interpreter_changed::$HasDiff"
#echo "interpreter_changed=$HasDiff" >> $GITHUB_OUTPUT
echo "interpreter_changed=$HasDiff"

build-macos:
needs: [ git_diff ]
# For any event that is not a PR, the CI will always run. In PRs, the CI
# can be skipped if the tag [skip-ci] or [skip ci] is written in the title.
if: |
@@ -95,18 +124,19 @@ jobs:
# Common configs: {Release,Debug,RelWithDebInfo)
# Build options: https://root.cern/install/build_from_source/#all-build-options
include:
- platform: mac13
- platform: mac13
arch: ARM64
overrides: ["LLVM_ENABLE_ASSERTIONS=On", "builtin_zlib=ON"]
- platform: mac14
arch: X64
overrides: ["LLVM_ENABLE_ASSERTIONS=On", "CMAKE_CXX_STANDARD=20"]
overrides: ["LLVM_ENABLE_ASSERTIONS=On", "CMAKE_CXX_STANDARD=20", "clingtest=${{needs.git_diff.outputs.interpreter_changed == 'true' && 'ON' || 'OFF'}}"]
- platform: mac15
arch: ARM64
overrides: ["LLVM_ENABLE_ASSERTIONS=On", "CMAKE_CXX_STANDARD=20"]
- platform: mac-beta
arch: ARM64
overrides: ["LLVM_ENABLE_ASSERTIONS=On", "CMAKE_CXX_STANDARD=20"]
# https://stackoverflow.com/questions/76292948/github-action-boolean-input-with-default-value

runs-on: # Using '[self-hosted, ..., ...]' does not work for some reason :)
- self-hosted
@@ -115,7 +145,7 @@ jobs:
- ${{ matrix.platform }}

name: |
${{ matrix.platform }} ${{ matrix.arch }}
${{ matrix.platform }} ${{ matrix.arch }}
${{ (github.event_name != 'schedule' && github.event_name != 'workflow_dispatch' && join( matrix.overrides, ', ' )) || '' }}

steps:
@@ -217,6 +247,7 @@ jobs:


build-windows:
needs: [ git_diff ]
# For any event that is not a PR, the CI will always run. In PRs, the CI
# can be skipped if the tag [skip-ci] or [skip ci] is written in the title.
if: |
@@ -271,6 +302,7 @@ jobs:
run: "C:\\setenv.bat ${{ matrix.target_arch }} &&
python .github/workflows/root-ci-config/build_root.py
--buildtype ${{ matrix.config }}
--clingtest ${{ needs.git_diff.outputs.interpreter_changed }}
--platform windows10
--incremental $INCREMENTAL
--base_ref ${{ github.base_ref }}
@@ -314,6 +346,7 @@ jobs:
run: "C:\\setenv.bat ${{ matrix.target_arch }} &&
python .github/workflows/root-ci-config/build_root.py
--buildtype ${{ matrix.config }}
--clingtest ${{ needs.git_diff.outputs.interpreter_changed }}
--platform windows10
--incremental false
--base_ref ${{ github.ref_name }}
@@ -338,6 +371,7 @@ jobs:


build-linux:
needs: [ git_diff ]
# For any event that is not a PR, the CI will always run. In PRs, the CI
# can be skipped if the tag [skip-ci] or [skip ci] is written in the title.
if: |
@@ -350,6 +384,11 @@ jobs:
strategy:
fail-fast: false
matrix:
# https://github.com/orgs/community/discussions/26253
# https://stackoverflow.com/questions/65384420/how-do-i-make-a-github-action-matrix-element-conditional
# https://stackoverflow.com/questions/68994484/how-to-skip-a-configuration-of-a-matrix-with-github-actions
# https://github.com/joaomcteixeira/python-project-skeleton/pull/31/files

# Specify image + (optional) build option overrides
#
# Available images: https://github.com/root-project/root-ci-images
@@ -363,7 +402,8 @@ jobs:
- image: alma8
overrides: ["LLVM_ENABLE_ASSERTIONS=On"]
- image: alma9
overrides: ["LLVM_ENABLE_ASSERTIONS=On", "CMAKE_BUILD_TYPE=Debug"]
overrides: ["LLVM_ENABLE_ASSERTIONS=On", "CMAKE_BUILD_TYPE=Debug", "clingtest=${{needs.git_diff.outputs.interpreter_changed == 'true' && 'ON' || 'OFF'}}"]
# https://stackoverflow.com/questions/76292948/github-action-boolean-input-with-default-value
- image: ubuntu22
overrides: ["imt=Off", "LLVM_ENABLE_ASSERTIONS=On", "CMAKE_BUILD_TYPE=Debug"]
- image: ubuntu2404
@@ -580,4 +620,4 @@ jobs:
uses: actions/upload-artifact@v4
with:
name: Event File
path: ${{ github.event_path }}
path: ${{ github.event_path }}
Original file line number Diff line number Diff line change
@@ -22,6 +22,14 @@
#include "clang/Sema/Sema.h"
#include "clang/Sema/SemaDiagnostic.h"

// Implements the CValueExtractionPrinter interface.
extern "C" {
CLING_LIB_EXPORT
void cling_SetValueNoAlloc(void* /*cling::Value* V*/) {}
CLING_LIB_EXPORT
void cling_SetValueWithAlloc(void* /*cling::Value* V*/) {}
}

using namespace clang;

namespace cling {
@@ -436,6 +444,10 @@ namespace {
return VSError(m_Sema, E, "cling::runtime::gCling");
if (!(NSD = utils::Lookup::Namespace(m_Sema, "internal", NSD)))
return VSError(m_Sema, E, "cling::runtime::internal namespace");
} else {
// C, ObjC,...
if (!(NSD = utils::Lookup::Namespace(m_Sema, "cling")))
return VSError(m_Sema, E, "cling namespace");
}
LookupResult R(*m_Sema, &m_Context->Idents.get("setValueNoAlloc"),
SourceLocation(), Sema::LookupOrdinaryName,
10 changes: 5 additions & 5 deletions interpreter/cling/test/Driver/C.c
Original file line number Diff line number Diff line change
@@ -11,13 +11,13 @@

// Validate cling C mode.

// Fix value printing!

int printf(const char*,...);
printf("CHECK 123 %p\n", gCling); // CHECK: CHECK 123

12 // expected-error {{ValueExtractionSynthesizer could not find: 'cling::runtime::internal::setValueNoAlloc'.}}

32 // expected-error {{ValueExtractionSynthesizer could not find: 'cling::runtime::internal::setValueNoAlloc'.}}
int i = 1 // CHECK: (int) 1
sizeof(int) // CHECK: (unsigned long) 4
int x = sizeof(int);
printf("CHECK %d\n", x); // CHECK: CHECK 4

// expected-no-diagnostics
.q

Unchanged files with check annotations Beta

if (isOffset()) {
assert(Source &&
"Cannot deserialize a lazy pointer without an AST source");
Ptr = reinterpret_cast<uint64_t>((Source->*Get)(Ptr >> 1));

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / fedora40 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / fedora40 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / fedora40 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / fedora40 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / fedora40 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / fedora40 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / fedora41 LLVM_ENABLE_ASSERTIONS=On

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / fedora41 LLVM_ENABLE_ASSERTIONS=On

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / fedora41 LLVM_ENABLE_ASSERTIONS=On

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / fedora41 LLVM_ENABLE_ASSERTIONS=On

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / fedora41 LLVM_ENABLE_ASSERTIONS=On

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / fedora41 LLVM_ENABLE_ASSERTIONS=On

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / debian125 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / debian125 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / debian125 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / debian125 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / debian125 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / debian125 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / debian125 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20

‘this’ pointer is null [-Wnonnull]

Check warning on line 378 in interpreter/llvm-project/clang/include/clang/AST/ExternalASTSource.h

GitHub Actions / debian125 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20

‘this’ pointer is null [-Wnonnull]
}
return reinterpret_cast<T*>(Ptr);
}
} else if (hit_right) {
top = right.index;
}
else [[unlikely]] {

Check warning on line 152 in geom/geom/inc/bvh/v2/bvh.h

GitHub Actions / alma8 LLVM_ENABLE_ASSERTIONS=On

attributes at the beginning of statement are ignored [-Wattributes]
goto restart;
}
}
// CHECK-NEXT: (char) 'L'
"Line1\nLine2\nLine3"
// CHECK-NEXT: (const char[18]) "Line1

Check failure on line 49 in interpreter/cling/test/Prompt/ValuePrinter/Strings.C

GitHub Actions / alma9 LLVM_ENABLE_ASSERTIONS=On, CMAKE_BUILD_TYPE=Debug, clingtest=ON

CHECK-NEXT: expected string not found in input
// CHECK-NEXT: Line2
// CHECK-NEXT: Line3"
// CHECK-NEXT: (const char[7]) "\xed\xaf\xbf\xed\xbf\xbf"
std::string(u8"UTF-8")
// CHECK-NEXT: (std::string) "UTF-8"

Check failure on line 119 in interpreter/cling/test/Prompt/ValuePrinter/Strings.C

GitHub Actions / mac14 X64 LLVM_ENABLE_ASSERTIONS=On, CMAKE_CXX_STANDARD=20, clingtest=ON

CHECK-NEXT: expected string not found in input
std::u16string(u"UTF-16 " u"\x394" u"\x3a6" u"\x3a9")
// CHECK-NEXT: (std::u16string) u"UTF-16 ΔΦΩ"
// CHECK: incomplete type 'B'
struct B { int v; };
std::is_default_constructible_v<B>
// CHECK: (const bool) true

Check failure on line 22 in interpreter/cling/test/ErrorRecovery/IncompleteType.C

GitHub Actions / alma9 LLVM_ENABLE_ASSERTIONS=On, CMAKE_BUILD_TYPE=Debug, clingtest=ON

CHECK: expected string not found in input
template <typename T> struct C;
template <> struct C<int>;
std::unique_ptr<cling::test::SymbolResolverCallback> SRC;
SRC.reset(new cling::test::SymbolResolverCallback(gCling))
gCling->setCallbacks(std::move(SRC));
jksghdgsjdf->getVersion() // CHECK: {{.*Interpreter.*}}

Check failure on line 18 in interpreter/cling/test/Extensions/Lookup/SimpleDynamicExprs.C

GitHub Actions / alma9 LLVM_ENABLE_ASSERTIONS=On, CMAKE_BUILD_TYPE=Debug, clingtest=ON

CHECK: expected string not found in input
hsdghfjagsp->Draw() // CHECK: (int) 12
h->PrintString(std::string("test")); // CHECK: test