[RVec] Forward declare VDT functions in RVec.hxx header - #22927
[RVec] Forward declare VDT functions in RVec.hxx header#22927guitargeek wants to merge 1 commit into
RVec.hxx header#22927Conversation
| inline float fast_tanf(float); | ||
| inline float fast_asinf(float); | ||
| inline float fast_acosf(float); | ||
| inline float fast_atanf(float); |
There was a problem hiding this comment.
Should that go in vdt itself as vdt_fwd.h?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
But I think that should be up to the maintainer of vdt, @dpiparo
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. 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…
There was a problem hiding this comment.
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.
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 root-project#9736, because now there is no builtin library left that ends up included in ROOT modules.
Test Results 23 files 23 suites 3d 18h 53m 22s ⏱️ For more details on these failures, see this check. Results for commit 3a9f593. ♻️ This comment has been updated with latest results. |
2 participants
Introduce forward declarations of the vdt functions int
RVec.hxxso 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.