[RVec] Forward declare VDT functions in RVec.hxx header

[guitargeek]

Introduce forward declarations of the vdt functions int RVec.hxx so we don't rely on the <vdt/vdtMath.h> header always being in the include path.

Closes #9736, because now there is no builtin library left that ends up included in ROOT modules.

[vgvassilev]

Should that go in vdt itself as vdt_fwd.h?

[guitargeek]

I don't think it's worth it, these forward declaration headers make more sense if the types are complicated and expected to change sometimes (e.g. template types where template arguments are expected to be added).

The VDT function signatures have not changed in 14 years though, so I wouldn't consider a forward declaration header necessary.

[guitargeek]

But I think that should be up to the maintainer of vdt, @dpiparo

[vgvassilev]

Yes, but that section can go out of sync if upstream changes. Logistically putting it in the vdt library is the right layering approach afaict.

[guitargeek]

But then it doesn't solve the problem anymore: the whole point of this change is to make RVec.hxx work if vdt is not in the include path, for most common case where you don't need the fast_* functions. Like this, it also doesn't end up in the modules.

In the case where you need VDT, you can simply include it yourself. This follows the "only pay for what you use" model that we're generally trying to follow now.

[vgvassilev]

I realize that I do not understand the whole idiom. Eg: #define RVEC_VDT_UNARY_FUNCTION(F) RVEC_UNARY_FUNCTION(F, vdt::F). Why do we carry all these in the public header file? If they are implementation defined we might be able to put that logic in a local header file.

[guitargeek]

Yes, that should work as you designed it even if the header is in vdt, no? In there if your forward declaration header is standalone we won’t include it as part of the modules setup, either.

Also the "standalone vdt forward declaration header" would have to be in a place where ROOT can find it at runtime, also increasing the include path surface that this PR tries to minimize to begin with. Alternatively, ROOT would have to ship it, which goes against the direction of making ROOT dependencies more modular.

If you strongly feel that this your approach is better I am fine with it. Just looks like a layering and invalidation problem that we might have. Probably vdt does not change a lot and the benefit is marginal…

Thanks!

I realize that I do not understand the whole idiom. Eg: #define RVEC_VDT_UNARY_FUNCTION(F) RVEC_UNARY_FUNCTION(F, vdt::F). Why do we carry all these in the public header file? If they are implementation defined we might be able to put that logic in a local header file.

These RVEC_VDT_UNARY_FUNCTION(F) RVEC_UNARY_FUNCTION(F, vdt::F) definitions available in the public headers are required for RVec to work. This is what they resolve to:

  template <typename T>
  RVec<PromoteType<T>> fast_expf(const RVec<T> &v) {
     RVec<PromoteType<T>> ret(v.size());
     std::transform(v.begin(), v.end(), ret.begin(),
                    [](const T &x) { return vdt::fast_expf(x); });
     return ret;
  }

For the most common types float and double, the template instantiations are externalized to the RVec.cxx translation unit by these RVEC_EXTERN_VDT_UNARY_FUNCTION macros. But we still need to have the definitions in the public header for the general T.

[vgvassilev]

Also the "standalone vdt forward declaration header" would have to be in a place where ROOT can find it at runtime, also increasing the include path surface that this PR tries to minimize to begin with. Alternatively, ROOT would have to ship it, which goes against the direction of making ROOT dependencies more modular.

I do not get that part. We can still have the fwd declaring header in vdt and only include it in R_HAS_VDT context. We can make it even more convenient by adding an X-macro facility that will expand for each macro defined in the RVec.hxx file.

[guitargeek]

vdt, including the supposed fwd declaration header that you suggest to add, is not part of ROOT. So it has to be in the include path if you use ROOT master, which is a nuisance. R_HAS_VDT is only telling you if ROOT was built with the vdt features, not if its header is available at runtime.

[andresailer]

I.e. one can get this error (https://cdash.cern.ch/tests/13049607)

In file included from /build/jenkins/workspace/lcg_nightly_pipeline/build/projects/ROOT-HEAD/src/ROOT/HEAD/roottest/root/io/newstl/TestOutput.h:15:
/build/jenkins/workspace/lcg_nightly_pipeline/build/projects/ROOT-HEAD/src/ROOT-HEAD-build/include/ROOT/RVec.hxx:50:10: fatal error: 'vdt/vdtMath.h' file not found
#include <vdt/vdtMath.h>
         ^~~~~~~~~~~~~~~

[github-actions]

Test Results

    23 files      23 suites   3d 18h 53m 22s ⏱️
 3 877 tests  3 875 ✅ 0 💤 2 ❌
79 862 runs  79 855 ✅ 3 💤 4 ❌

For more details on these failures, see this check.

Results for commit 3a9f593.

♻️ This comment has been updated with latest results.