[xRooFit] Make headers work standalone by hageboeck · Pull Request #23407 · root-project/root

hageboeck · 2026-09-17T15:54:33Z

Note: Contains commits from #23387. It was simply more convenient to leave them here, because rebasing on master would create a merge conflict once #23387 gets merged. Only the last three need to be reviewed.

Now to what happened in xRooFit:
When checking the installed headers if they work standalone, a circular
header inclusion lead to syntax errors. It's because after a few inclusions, xRooFit.h gets included again in xRooHypoSpace which wants to include xRooFit again.

To solve this:

Minimise inclusions in xRooFit.h
Move inclusions into all .cxx where necessary.
Add required includes to XRooFit.h to make it a "complete" umbrella header.

LLM disclosure: Asked an LLM to propose the required includes. Commits and tests done the classic way.

Up for discussion

When making XRooFit.h a "complete" umbrella header, one could also include xRooBrowser.h. It's not really required, but it would include all headers at once. That's why there is the last commit that performs this inclusion, but we can remove it if it's considered unnecessary.

github-actions · 2026-09-17T18:26:22Z

Test Results

22 files 22 suites 3d 9h 50m 21s ⏱️
3 876 tests 3 875 ✅ 0 💤 1 ❌
76 368 runs 76 367 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 0db00f6.

♻️ This comment has been updated with latest results.

When checking the installed headers if they work standalone, a circular header inclusion lead to syntax errors. To solve this: - Minimise inclusions in xRooFit.h - Move them into the .cxx where necessary. - Add required includes to XRooFit.h

It wasn't there before, but this seems to make it complete.

hageboeck · 2026-09-18T14:10:27Z

@will-cern could you have a look?

will-cern · 2026-09-18T14:22:38Z

I'll need to test these changes in the main xRooFit repo @ https://gitlab.cern.ch/will/xroofit, as that is where xRooFit should ultimately be copied from for the version that ships with root.
I'll let you know when this has been checked

will-cern · 2026-09-25T08:21:07Z

Ok I've had a look at this now. On first impression I am struggling to understand how the circular dependency emerges, as there are include guards in place. As far as I can tell, the first include of xRooFit.h should cause XROOFIT_XROOFIT_H definition, which should prevent and further reinclusion.

The intended "global" header that should be the only one someone needs to include, would be the xRooFit.h one. The XRooFit.h header in the root repo doesn't even exist in the xroofit repo, so it would have to be added there if that was to become the global header. But I still struggle to understand why xRooFit.h cannot be the global header. Is the issue that the "standalone" tests you talk about aren't respecting the include guards?

Can you give me a way to reproduce the "standalone" case?

Sorry for all this, its just xRooFit's header guards are quite complicated so that we can support all the ways it gets used (from ROOT's build, built on top of ROOT, linked against in compiled code, in cling macros, etc...)

hageboeck · 2026-09-29T16:01:28Z

Hello @will-cern,

thanks for having a look!

Ok I've had a look at this now. On first impression I am struggling to understand how the circular dependency emerges, as there are include guards in place. As far as I can tell, the first include of xRooFit.h should cause XROOFIT_XROOFIT_H definition, which should prevent and further reinclusion.

Maybe my attempt at explaining it most likely wasn't fully correct / complete. There is a new attempt below. 🙂

The intended "global" header that should be the only one someone needs to include, would be the xRooFit.h one. The XRooFit.h header in the root repo doesn't even exist in the xroofit repo, so it would have to be added there if that was to become the global header. But I still struggle to understand why xRooFit.h cannot be the global header. Is the issue that the "standalone" tests you talk about aren't respecting the include guards?

Interesting, I didn't know that XRooFit.h was an addition. In that case, my first intuition would be to remove it and try to use xRooFit.h instead. @guitargeek, sound OK to you?

Can you give me a way to reproduce the "standalone" case?

Of course! The failure in the CI is produced as follows:

Build root
Install
Remove the xRooFit suppression from https://github.com/root-project/root/blob/master/test/PostInstall/check-headers.sh as done in this PR
check-headers.sh <installDir>/include
- By the way, you are not forced to run the checks on all headers. You can e.g. use bash -x checkHeaders.sh to see what commands it runs and run these manually.
- For my installation, it expands to
  clang++-22 -fsyntax-only -x c++ -std=c++20 -Wall -Wextra -Wno-unused-parameter -Wno-unused-const-variable -I/tmp/root-dbg/include/ <header>
  With small tweaks you should be able to reproduce the problem.

This results in:

In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooNLLVar.h:23:
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:238:
/tmp/root-dbg/include/RooFit/xRooFit/xRooHypoSpace.h:27:30: error: incomplete type 'ROOT::Experimental::XRooFit::xRooNLLVar' named in nested name specifier
   27 | class xRooHypoSpace : public xRooNLLVar::xRooHypoSpace {
      |                              ^~~~~~~~~~~~
/tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:57:7: note: forward declaration of 'ROOT::Experimental::XRooFit::xRooNLLVar'
   57 | class xRooNLLVar;
      |       ^
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooNLLVar.h:23:
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:238:
/tmp/root-dbg/include/RooFit/xRooFit/xRooHypoSpace.h:27:42: error: expected class name
   27 | class xRooHypoSpace : public xRooNLLVar::xRooHypoSpace {
      |                                          ^
/tmp/root-dbg/include/RooFit/xRooFit/xRooHypoSpace.h:29:10: error: incomplete type 'ROOT::Experimental::XRooFit::xRooNLLVar' named in nested name specifier
   29 |    using xRooNLLVar::xRooHypoSpace::xRooHypoSpace;
      |          ^~~~~~~~~~~~
/tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:57:7: note: forward declaration of 'ROOT::Experimental::XRooFit::xRooNLLVar'
   57 | class xRooNLLVar;
      |       ^
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooNLLVar.h:23:
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:238:
/tmp/root-dbg/include/RooFit/xRooFit/xRooHypoSpace.h:30:4: error: only virtual member functions can be marked 'override'
   30 |    ClassDefOverride(xRooHypoSpace, 0)
      |    ^
/tmp/root-dbg/include/Rtypes.h:348:31: note: expanded from macro 'ClassDefOverride'
  348 |    _ClassDefOutline_(name,id,,override)              \
      |                               ^
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooNLLVar.h:23:
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:238:
/tmp/root-dbg/include/RooFit/xRooFit/xRooHypoSpace.h:30:4: error: only virtual member functions can be marked 'override'
/tmp/root-dbg/include/Rtypes.h:348:31: note: expanded from macro 'ClassDefOverride'
  348 |    _ClassDefOutline_(name,id,,override)              \
      |                               ^
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooNLLVar.h:23:
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:238:
/tmp/root-dbg/include/RooFit/xRooFit/xRooHypoSpace.h:30:4: error: only virtual member functions can be marked 'override'
/tmp/root-dbg/include/Rtypes.h:348:31: note: expanded from macro 'ClassDefOverride'
  348 |    _ClassDefOutline_(name,id,,override)              \
      |                               ^
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooNLLVar.h:23:
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:238:
/tmp/root-dbg/include/RooFit/xRooFit/xRooHypoSpace.h:30:4: error: only virtual member functions can be marked 'override'
/tmp/root-dbg/include/Rtypes.h:348:31: note: expanded from macro 'ClassDefOverride'
  348 |    _ClassDefOutline_(name,id,,override)              \
      |                               ^
7 errors generated.

You see that the declaration of xRooNLLVar is broken in xRooHypoSpace.h, which gets included in xRooNLLVar.h via xRooFit.h.

will-cern · 2026-10-23T10:40:48Z

Hello @will-cern,

thanks for having a look!

Ok I've had a look at this now. On first impression I am struggling to understand how the circular dependency emerges, as there are include guards in place. As far as I can tell, the first include of xRooFit.h should cause XROOFIT_XROOFIT_H definition, which should prevent and further reinclusion.

Maybe my attempt at explaining it most likely wasn't fully correct / complete. There is a new attempt below. 🙂

The intended "global" header that should be the only one someone needs to include, would be the xRooFit.h one. The XRooFit.h header in the root repo doesn't even exist in the xroofit repo, so it would have to be added there if that was to become the global header. But I still struggle to understand why xRooFit.h cannot be the global header. Is the issue that the "standalone" tests you talk about aren't respecting the include guards?

Interesting, I didn't know that XRooFit.h was an addition. In that case, my first intuition would be to remove it and try to use xRooFit.h instead. @guitargeek, sound OK to you?

Can you give me a way to reproduce the "standalone" case?

Of course! The failure in the CI is produced as follows:

* Build root

* Install

* Remove the xRooFit suppression from https://github.com/root-project/root/blob/master/test/PostInstall/check-headers.sh as done in this PR

* `check-headers.sh <installDir>/include`
  
  * By the way, you are not forced to run the checks on all headers. You can e.g. use `bash -x checkHeaders.sh` to see what commands it runs and run these manually.
  * For my installation, it expands to
    `clang++-22 -fsyntax-only -x c++ -std=c++20 -Wall -Wextra -Wno-unused-parameter -Wno-unused-const-variable -I/tmp/root-dbg/include/ <header>`
    With small tweaks you should be able to reproduce the problem.

This results in:

In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooNLLVar.h:23:
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:238:
/tmp/root-dbg/include/RooFit/xRooFit/xRooHypoSpace.h:27:30: error: incomplete type 'ROOT::Experimental::XRooFit::xRooNLLVar' named in nested name specifier
   27 | class xRooHypoSpace : public xRooNLLVar::xRooHypoSpace {
      |                              ^~~~~~~~~~~~
/tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:57:7: note: forward declaration of 'ROOT::Experimental::XRooFit::xRooNLLVar'
   57 | class xRooNLLVar;
      |       ^
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooNLLVar.h:23:
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:238:
/tmp/root-dbg/include/RooFit/xRooFit/xRooHypoSpace.h:27:42: error: expected class name
   27 | class xRooHypoSpace : public xRooNLLVar::xRooHypoSpace {
      |                                          ^
/tmp/root-dbg/include/RooFit/xRooFit/xRooHypoSpace.h:29:10: error: incomplete type 'ROOT::Experimental::XRooFit::xRooNLLVar' named in nested name specifier
   29 |    using xRooNLLVar::xRooHypoSpace::xRooHypoSpace;
      |          ^~~~~~~~~~~~
/tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:57:7: note: forward declaration of 'ROOT::Experimental::XRooFit::xRooNLLVar'
   57 | class xRooNLLVar;
      |       ^
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooNLLVar.h:23:
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:238:
/tmp/root-dbg/include/RooFit/xRooFit/xRooHypoSpace.h:30:4: error: only virtual member functions can be marked 'override'
   30 |    ClassDefOverride(xRooHypoSpace, 0)
      |    ^
/tmp/root-dbg/include/Rtypes.h:348:31: note: expanded from macro 'ClassDefOverride'
  348 |    _ClassDefOutline_(name,id,,override)              \
      |                               ^
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooNLLVar.h:23:
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:238:
/tmp/root-dbg/include/RooFit/xRooFit/xRooHypoSpace.h:30:4: error: only virtual member functions can be marked 'override'
/tmp/root-dbg/include/Rtypes.h:348:31: note: expanded from macro 'ClassDefOverride'
  348 |    _ClassDefOutline_(name,id,,override)              \
      |                               ^
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooNLLVar.h:23:
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:238:
/tmp/root-dbg/include/RooFit/xRooFit/xRooHypoSpace.h:30:4: error: only virtual member functions can be marked 'override'
/tmp/root-dbg/include/Rtypes.h:348:31: note: expanded from macro 'ClassDefOverride'
  348 |    _ClassDefOutline_(name,id,,override)              \
      |                               ^
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooNLLVar.h:23:
In file included from /tmp/root-dbg/include/RooFit/xRooFit/xRooFit.h:238:
/tmp/root-dbg/include/RooFit/xRooFit/xRooHypoSpace.h:30:4: error: only virtual member functions can be marked 'override'
/tmp/root-dbg/include/Rtypes.h:348:31: note: expanded from macro 'ClassDefOverride'
  348 |    _ClassDefOutline_(name,id,,override)              \
      |                               ^
7 errors generated.

You see that the declaration of xRooNLLVar is broken in xRooHypoSpace.h, which gets included in xRooNLLVar.h via xRooFit.h.

Thanks for the reproducer! I finally had time this morning to give it a try and can confirm that the issue is indeed if someone tried to do just #include "xRooNLLVar.h" the order of the includes would cause a problem. I have now fixed this by relocating the declaration of the xRooHypoSpace class into the xRooNLLVar.h header, and the original header just includes that one. This resolves the issues and keeps everything working that was working before too.
This change is included in #23809 which also includes other updates from the main xRoofit repo!

guitargeek · 2026-10-25T12:26:16Z

Succeeded by #23809. Thanks @hageboeck and @will-cern !

hageboeck self-assigned this Sep 17, 2026

hageboeck added the clean build Ask CI to do non-incremental build on PR label Sep 17, 2026

hageboeck added 3 commits September 18, 2026 16:08

[CI] Enable xRooFit in installed header check.

9cf5dfd

[xRooFit] Add xRooBrowser to the XRooFit umbrella header.

0db00f6

It wasn't there before, but this seems to make it complete.

hageboeck force-pushed the xRooFit_headerCheck branch from c3e8a03 to 0db00f6 Compare September 18, 2026 14:09

hageboeck marked this pull request as ready for review September 18, 2026 14:09

hageboeck requested review from dpiparo and guitargeek as code owners September 18, 2026 14:09

will-cern mentioned this pull request Oct 23, 2026

[xroofit] updates from main xRooFit repo, and sort out include order issue for check-headers.sh postinstall test #23809

Merged

guitargeek closed this Oct 25, 2026

Navigation Menu

Repository navigation

[xRooFit] Make headers work standalone - #23407

[xRooFit] Make headers work standalone#23407
hageboeck wants to merge 3 commits into
root-project:masterfrom
hageboeck:xRooFit_headerCheck

hageboeck commented Sep 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 17, 2026 •

edited

Loading

Uh oh!

hageboeck commented Sep 18, 2026

Uh oh!

will-cern commented Sep 18, 2026

Uh oh!

will-cern commented Sep 25, 2026

Uh oh!

hageboeck commented Sep 29, 2026

Uh oh!

will-cern commented Oct 23, 2026

Uh oh!

guitargeek commented Oct 25, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

hageboeck commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up for discussion

Uh oh!

github-actions Bot commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

hageboeck commented Sep 18, 2026

Uh oh!

will-cern commented Sep 18, 2026

Uh oh!

will-cern commented Sep 25, 2026

Uh oh!

hageboeck commented Sep 29, 2026

Uh oh!

will-cern commented Oct 23, 2026

Uh oh!

guitargeek commented Oct 25, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hageboeck commented Sep 17, 2026 •

edited

Loading

github-actions Bot commented Sep 17, 2026 •

edited

Loading