On Aug 2, 2021, at 6:46 PM, Emil Dotchevski via Boost
On Mon, Aug 2, 2021 at 5:12 PM Marshall Clow via Boost < boost@lists.boost.org> wrote:
On Aug 2, 2021, at 5:01 PM, Emil Dotchevski via Boost <
boost@lists.boost.org> wrote:
Technically not a Beta bug fix but I think it would be nice if these
fixes
make it in the next release, both reported after the Beta lock:
https://github.com/boostorg/qvm/commit/46797f11f71c625420217ef9043d6ee4a12cf... .
This one looks worrisome to me; that’s a significant change. Let’s get the bots back up, look at the results, and talk tomorrow or Wednesday.
Perhaps misunderstanding? The bots are up, tests all pass: https://github.com/boostorg/qvm/actions/runs/1054919685.
I was thinking of these bots. https://www.boost.org/development/tests/develop/developer/summary.html
They weren't passing before this commit because github had removed some gcc versions from the base ubuntu image used by GHA, so now they need to be installed manually.
To clarify, this change touches many files and looks scary but it is all along these lines:
We had:
T const mag=sqrt<T>(m2);
Now we have:
T const mag=sqrt(m2);
The sqrt function template is defined in, and invoked from, namespace boost::qvm. The difference is that now we're using ADL, which could bind some other sqrt function (m2 may be of user-defined type).
The same is done for other math functions, e.g. sin, cos, etc.
Yeah, I got that. My concern is wholly about finding “other" sqrt functions.
[ I’m not against merging it, I’m just concerned ]
Yeah, I'm on the edge for this one too. Seems simple and safe though. :)
https://github.com/boostorg/qvm/commit/36ab7dfd000d8100727ceee98df233dc7e5de...
(Issue 33).
This one is fine. Go ahead.
I'd rather not merge this one if the other one isn't merged in also.
That’s your call. — Marshall