Skip to content

Enable interop #218

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

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
cd36aed
[RF] Outline destructor of TJSONTree::Node
hahnjo Feb 17, 2025
4c1dff5
[RF] Add missing include to test
hahnjo Feb 17, 2025
be8a98d
[ntuple] Rename `RNTupleIndex` to `RNTupleJoinTable`
enirolf Feb 14, 2025
c4f1a13
[ntuple] Rename `RNTupleJoinTable` data members
enirolf Feb 14, 2025
66c9fd0
[ntuple] Update `RNTupleJoinTable` error messages
enirolf Feb 14, 2025
1e0b3f9
[ntuple] Update `RNTupleJoinTable` documentation
enirolf Feb 14, 2025
4bb3964
[ntuple] Update `ntuple_join_table` test variables
enirolf Feb 14, 2025
2f06c04
[RF] Support non-uniform binning in RooAbsReal::fillHistogram()
guitargeek Feb 11, 2025
16b33f6
[RF] Don't disable bin volume scaling when creating yields histogram
guitargeek Dec 2, 2024
93607ba
[RF] Consistently use universal `RooAbsData::reduce()` overload
guitargeek Jan 24, 2025
2d090af
[RF] Fix some design issues with the RooTreeDataStore
guitargeek Dec 2, 2024
b0e7089
[RF] Remove `addColumns()` member function of dataset classes
guitargeek Dec 4, 2024
6166d3f
[RF] Deprecate `RooStats::HLFactory`
guitargeek Nov 26, 2024
8bf3b0b
[RF] Remove RooStats::HLFactory tutorials
guitargeek Nov 26, 2024
bb4fde0
[cling] Print correct LLVM CMake directory
hahnjo Feb 17, 2025
5c51aaf
[interpreter] Use find_package({LLVM,Clang})
hahnjo Feb 17, 2025
85bba3e
[cmake] Remove warning about python option
hahnjo Feb 17, 2025
6a6a17e
[cmake] Add cxxmodules to list of removed options
hahnjo Feb 17, 2025
a53ff61
[cmake] Clean remainders of removed cxxmodules option
hahnjo Feb 17, 2025
4f088de
Remove more unset options from config/Makefile.in
hahnjo Feb 17, 2025
64a148d
[cling][NFC] Replace use of deprecated `AddFileDependencies`
devajithvs Feb 17, 2025
e47e2cc
Reset TBrowser::fImp pointer when close browser window
linev Feb 17, 2025
7b9527f
Implement TBrowser::Draw()
linev Feb 18, 2025
0229f00
Do not send init message twice in RBrowser
linev Feb 18, 2025
950607a
[skip-ci] fix link to PRs (#17742)
ferdymercury Feb 18, 2025
471007c
Enable the CppInterOp library
aaronj0 Jan 20, 2025
d85b42f
[TCling] Allow InterOp to access ROOT's interpreter
aaronj0 Jan 18, 2025
2887ee4
[ci] Add github workflow to check diff against upstream CppInterOp
aaronj0 Jan 29, 2025
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
26 changes: 26 additions & 0 deletions .github/workflows/cppinterop-diff.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
name: Diff CppInterOp against upstream
on:
pull_request:
paths:
- 'interpreter/CppInterOp/**'

jobs:
cppinterop-diff:
runs-on: ubuntu-latest
steps:
- name: Check out ROOT
uses: actions/checkout@v4
with:
path: root
- name: Check out CppInterOp
uses: actions/checkout@v4
with:
repository: compiler-research/CppInterOp
ref: 6c6f94a22bd971520a249e2c02e4259cdd3a5be6
path: CppInterOp
- name: Drop directories that are not added to ROOT
working-directory: CppInterOp
run: |
rm -rf .git .github discord.svg
- name: Compare
run: diff -ur CppInterOp/ root/interpreter/CppInterOp/
1 change: 0 additions & 1 deletion .github/workflows/root-ci-config/buildconfig/global.txt
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ clingtest=OFF
cocoa=OFF
coverage=OFF
cuda=OFF
cxxmodules=OFF
daos=OFF
dataframe=ON
davix=ON
Expand Down
9 changes: 9 additions & 0 deletions README/ReleaseNotes/v636/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ The following people have contributed to this new version:
* The build option `html` has been removed.
* The ClassImp macro and derived macros have no effect now and will be deprecated in ROOT 6.38.
* The default TTreeFormula constructor (without arguments) is now deleted, since it lead to an unusable and unsafe object. Instead, this implementation has been reserved now for ROOT I/O exclusively via the TRootIOCtor argument tag.
* The `RooStats::HLFactory` class was deprecated will be removed in ROOT 6.38. It provided little advantage over using the RooWorkspace directly or any of the other higher-level frameworks that exist in the RooFit ecosystem.

## Python Interface

Expand All @@ -56,6 +57,14 @@ The following people have contributed to this new version:
This pattern was not appropriate for a modern C++ library.
If you absolutely need the old return type, wrap the call to `format()` inside `new TString{var.format(..)}`. However, this is not recommended.

### Fix for `RooAbsReal::createHistogram()` with extended pdfs

There was a problem with [RooAbsReal::createHistogram()](https://root.cern.ch/doc/master/classRooAbsReal.html#a9451168bb4159899fe1854f591f69814) when using it to get histograms with predicted yields for extended pdfs.
The `Scale(bool)` argument was always set internally to `false` in case `createHistogram()` was called on an extended pdf. There was no way for the user to override that.
This meant that one could not get yield histograms that were correctly scaled by the bin volumes using that function.
This release changes that behavior, meaning the `Scale(bool)` command argument is now respected for extended pdfs.


## IO

* New options have been added to TFileMerger (which can be passed as whitespace-separated TStrings via `TFileMerger::SetMergeOptions`)
Expand Down
7 changes: 1 addition & 6 deletions cmake/modules/RootBuildOptions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ endif()
#---Removed options------------------------------------------------------------
# Please notify SPI when adding to this list
foreach(opt afdsmgrd afs alien bonjour builtin_afterimage castor chirp cxx11 cxx14 cxx17
exceptions geocad gfal glite globus gsl_shared hdfs html ios jemalloc krb5
cxxmodules exceptions geocad gfal glite globus gsl_shared hdfs html ios jemalloc krb5
ldap memstat minuit2 monalisa oracle pyroot-python2 pyroot_legacy
pythia6 pythia6_nolink python qt qtgsi rfio ruby sapdb srp table
tcmalloc vmc xproofd)
Expand All @@ -409,11 +409,6 @@ foreach(opt minuit2_omp minuit2_mpi)
endif()
endforeach()

#---Replaced options--------------------------------------------------------------------------
if(python)
message(STATUS ">>> INFO: 'python' option was removed. Instead, please check, that it was enabled a 'pyroot' option (by default it is ON).")
endif()

#---Avoid creating dependencies to 'non-standard' header files -------------------------------
include_regular_expression("^[^.]+$|[.]h$|[.]icc$|[.]hxx$|[.]hpp$")

Expand Down
9 changes: 0 additions & 9 deletions cmake/modules/RootConfiguration.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -605,15 +605,6 @@ if(MSVC)
string(REPLACE "-I${CMAKE_SOURCE_DIR}/cmake/win" "" __cflags "${__cflags}")
endif()

if (cxxmodules)
# Re-add the -Wno-module-import-in-extern-c which we just filtered out.
# We want it because it changes the module cache hash and causes modules to be
# rebuilt.
# FIXME: We should review how we do the regex.
set(ROOT_CXX_FLAGS "${ROOT_CXX_FLAGS} -Wno-module-import-in-extern-c")
set(ROOT_C_FLAGS "${ROOT_C_FLAGS} -Wno-module-import-in-extern-c")
endif()

string(REGEX REPLACE "(^|[ ]*)-W[^ ]*" "" __fflags "${CMAKE_Fortran_FLAGS}")
string(REGEX MATCHALL "(-Wp,)?-(D|U)[^ ]*" __defs "${CMAKE_CXX_FLAGS}")
set(ROOT_COMPILER_FLAG_HINTS "#
Expand Down
2 changes: 0 additions & 2 deletions cmake/modules/RootMacros.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,6 @@ function(ROOT_GENERATE_DICTIONARY dictionary)
endforeach()
endif()

# if (cxxmodules OR runtime_cxxmodules)
# Comments from Vassil:
# FIXME: We prepend ROOTSYS/include because if we have built a module
# and try to resolve the 'same' header from a different location we will
Expand All @@ -335,7 +334,6 @@ function(ROOT_GENERATE_DICTIONARY dictionary)
list(FILTER incdirs EXCLUDE REGEX "^${CMAKE_BINARY_DIR}/externals")
list(FILTER incdirs EXCLUDE REGEX "^${CMAKE_BINARY_DIR}/builtins")
list(INSERT incdirs 0 ${CMAKE_BINARY_DIR}/include)
# endif()

# this instruct rootcling do not store such paths in dictionary
set(excludepaths ${CMAKE_SOURCE_DIR} ${CMAKE_BINARY_DIR}/ginclude ${CMAKE_BINARY_DIR}/externals ${CMAKE_BINARY_DIR}/builtins)
Expand Down
5 changes: 0 additions & 5 deletions config/Makefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,6 @@ OSXSDK := @osxsdk@
IOSVERS := @iosvers@
IOSSDK := @iossdk@

CXX11 := @c++11@
CXX14 := @c++14@
CXXMODULES := @cxxmodules@
LIBCXX := @libc++@

ENABLETHREAD := @enable_thread@
OSTHREADFLAG := @threadflag@
OSTHREADLIBDIR := @threadlibdir@
Expand Down
1 change: 1 addition & 0 deletions core/gui/inc/TBrowser.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ class TBrowser : public TNamed {
void Show() { if (fImp) fImp->Show(); }
void SetDrawOption(Option_t *option="") override { if (fImp) fImp->SetDrawOption(option); }
Option_t *GetDrawOption() const override { return (fImp) ? fImp->GetDrawOption() : nullptr; }
void Draw(Option_t *option="") override;

Longptr_t ExecPlugin(const char *name = nullptr, const char *fname = nullptr,
const char *cmd = nullptr, Int_t pos = 1, Int_t subpos = -1) {
Expand Down
2 changes: 1 addition & 1 deletion core/gui/inc/TBrowserImp.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class TBrowserImp {
TBrowserImp(TBrowser *b = nullptr);
TBrowserImp(TBrowser *b, const char *title, UInt_t width, UInt_t height, Option_t *opt = "");
TBrowserImp(TBrowser *b, const char *title, Int_t x, Int_t y, UInt_t width, UInt_t height, Option_t *opt = "");
virtual ~TBrowserImp() = default;
virtual ~TBrowserImp();

virtual void Add(TObject *, const char *, Int_t) { }
virtual void AddCheckBox(TObject *, Bool_t = kFALSE) { }
Expand Down
19 changes: 19 additions & 0 deletions core/gui/src/TBrowser.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,25 @@ void TBrowser::Create(TObject *obj)
// with all browsable classes from TROOT
}

////////////////////////////////////////////////////////////////////////////////
/// Draw browser again if it was closed before
/// If browser exists - just call Show method

void TBrowser::Draw(Option_t *)
{
if (fImp) {
fImp->Show();
} else {
Float_t cx = gStyle->GetScreenFactor();
UInt_t w = UInt_t(cx*800);
UInt_t h = UInt_t(cx*500);
fImp = gGuiFactory->CreateBrowserImp(this, "ROOT Object Browser", w, h, "");
if (fImp)
fImp->BrowseObj(gROOT);
}
}


////////////////////////////////////////////////////////////////////////////////
/// Execute default action for selected object (action is specified
/// in the `$HOME/.root.mimes` or `$ROOTSYS/etc/root.mimes file`).
Expand Down
10 changes: 10 additions & 0 deletions core/gui/src/TBrowserImp.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ ABC describing GUI independent browser implementation protocol.
*/

#include "TBrowserImp.h"
#include "TBrowser.h"

ClassImp(TBrowserImp);

Expand All @@ -42,3 +43,12 @@ TBrowserImp::TBrowserImp(TBrowser *, const char *, Int_t, Int_t, UInt_t, UInt_t,
: fBrowser(nullptr), fShowCycles(kFALSE)
{
}

///////////////////////////////////////////////////////////////////
/// Destructor

TBrowserImp::~TBrowserImp()
{
if (fBrowser && fBrowser->GetBrowserImp() == this)
fBrowser->SetBrowserImp(nullptr);
}
9 changes: 7 additions & 2 deletions core/metacling/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ target_include_directories(MetaCling SYSTEM PRIVATE
${CLANG_INCLUDE_DIRS}
${LLVM_INCLUDE_DIRS}
${CLAD_INCLUDE_DIRS}
${CPPINTEROP_INCLUDE_DIRS}
)

target_include_directories(MetaCling PRIVATE
Expand Down Expand Up @@ -74,7 +75,7 @@ if(MSVC)
set_source_files_properties(TCling.cxx COMPILE_FLAGS /bigobj)
endif()

add_dependencies(MetaCling CLING)
add_dependencies(MetaCling CLING clangCppInterOp)

##### libCling #############################################################

Expand Down Expand Up @@ -113,11 +114,15 @@ ROOT_LINKER_LIBRARY(Cling
$<TARGET_OBJECTS:ClingUtils>
$<TARGET_OBJECTS:Dictgen>
$<TARGET_OBJECTS:MetaCling>
LIBRARIES ${CLING_LIBRARIES} ${LINK_LIBS} ${CLING_PLUGIN_LINK_LIBS})
LIBRARIES ${CLING_LIBRARIES} clangCppInterOp ${LINK_LIBS} ${CLING_PLUGIN_LINK_LIBS})

# When these two link at the same time, they can exhaust the RAM on many machines, since they both link against llvm.
add_dependencies(Cling rootcling_stage1)

if(testing)
add_dependencies(Cling CppInterOpUnitTests)
endif()

if(MSVC)
set_target_properties(Cling PROPERTIES WINDOWS_EXPORT_ALL_SYMBOLS TRUE)
set(cling_exports ${cling_exports}
Expand Down
7 changes: 7 additions & 0 deletions core/metacling/src/TCling.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ clang/LLVM technology.
#include "cling/Utils/SourceNormalization.h"
#include "cling/Interpreter/Exception.h"

#include "clang/Interpreter/CppInterOp.h"

#include "llvm/IR/GlobalValue.h"
#include "llvm/IR/Module.h"

Expand Down Expand Up @@ -1539,6 +1541,11 @@ TCling::TCling(const char *name, const char *title, const char* const argv[], vo
if (!fInterpreter->getCI()) { // Compiler instance could not be created. See https://its.cern.ch/jira/browse/ROOT-10239
return;
}

// Tell CppInterOp that the cling::Interpreter instance is managed externally by ROOT
// Sets the interpreter by passing the fInterpreter handle as soon as TCling is initialized
Cpp::UseExternalInterpreter((Cpp::TInterp_t*)fInterpreter.get());

// Don't check whether modules' files exist.
fInterpreter->getCI()->getPreprocessorOpts().DisablePCHOrModuleValidation =
DisableValidationForModuleKind::All;
Expand Down
4 changes: 2 additions & 2 deletions documentation/doxygen/mainpage.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,6 @@ to Reference Guides for all %ROOT versions.
### Provide your feedback
If you have suggestions about how to improve this documentation, you can let us know:

- With a [PR](https://github.com/root-project/root)
- With a [pull request](https://github.com/root-project/root/pulls)
- On the [ROOT Forum](https://root-forum.cern.ch)
- On [our tracker](https://github.com/root-project/root/issues)
- On [our issue tracker](https://github.com/root-project/root/issues)
8 changes: 0 additions & 8 deletions graf3d/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,6 @@
# For the licensing terms see $ROOTSYS/LICENSE.
# For the list of contributors see $ROOTSYS/README/CREDITS.

if(cxxmodules AND APPLE)
# FIXME: Any header of graf3d can trigger building of the OSX OpenGL module.
# It looks like our glew infrastructure combined with the OpenGL modulemap
# cannot be compiled with -fmodules-local-submodule-visibility yet.
# Strip out that flag.
string(REPLACE "-Xclang -fmodules-local-submodule-visibility" "" CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
endif()

add_subdirectory(g3d) # special CMakeLists.txt
if(NOT WIN32 AND x11)
add_subdirectory(x3d) # special CMakeLists.txt
Expand Down
2 changes: 0 additions & 2 deletions gui/browserv7/src/RBrowser.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -510,8 +510,6 @@ void RBrowser::Show(const RWebDisplayArgs &args, bool always_start_new_browser)
{
if (!fWebWindow->NumConnections() || always_start_new_browser) {
fWebWindow->Show(args);
} else {
SendInitMsg(0);
}
}

Expand Down
Loading