[chrono][timer][release] Non-inline functions in inlined headers
Hi, We've had a link problem reported. for this code on Visual Studio, using auto-linking: #include "boost\chrono.hpp" #include "boost\timer\timer.hpp" int main() { auto now = boost::chrono::steady_clock::now(); boost::timer::cpu_timer cput; return 0; } I haven't got the binaries installed yet, so I haven't reproduced it, but I think it's caused because timer now includes the chrono headers using BOOST_CHRONO_HEADER_ONLY, and in the chrono inlined headers not all functions are BOOST_CHRONO_INLINE. It looks like this problem was always in chrono, but BOOST_CHRONO_HEADER_ONLY was widely used enough for it to be detected. I'd quite like to fix this for 1.66.0, which requires either reverting the change to timer, or fixing the problem in chrono. The error is in chrono, but I'm more inclined to revert the change in timer for this release, as it's a less disruptive way of quickly fixing the problem. https://github.com/boostorg/timer/pull/5
On 13/12/2017 23:24, Daniel James wrote:
I haven't got the binaries installed yet, so I haven't reproduced it, but I think it's caused because timer now includes the chrono headers using BOOST_CHRONO_HEADER_ONLY, and in the chrono inlined headers not all functions are BOOST_CHRONO_INLINE. It looks like this problem was always in chrono, but BOOST_CHRONO_HEADER_ONLY was widely used enough for it to be detected.
I'm having trouble parsing this. I think you're missing some negations somewhere.
On 13 December 2017 at 23:43, Gavin Lambert via Boost
On 13/12/2017 23:24, Daniel James wrote:
I haven't got the binaries installed yet, so I haven't reproduced it, but I think it's caused because timer now includes the chrono headers using BOOST_CHRONO_HEADER_ONLY, and in the chrono inlined headers not all functions are BOOST_CHRONO_INLINE. It looks like this problem was always in chrono, but BOOST_CHRONO_HEADER_ONLY was widely used enough for it to be detected.
I'm having trouble parsing this. I think you're missing some negations somewhere.
Should have been "wasn't widely used".
Le 13/12/2017 à 11:24, Daniel James via Boost a écrit :
Hi,
We've had a link problem reported. for this code on Visual Studio, using auto-linking:
#include "boost\chrono.hpp" #include "boost\timer\timer.hpp"
int main() { auto now = boost::chrono::steady_clock::now(); boost::timer::cpu_timer cput; return 0; }
I haven't got the binaries installed yet, so I haven't reproduced it, but I think it's caused because timer now includes the chrono headers using BOOST_CHRONO_HEADER_ONLY, and in the chrono inlined headers not all functions are BOOST_CHRONO_INLINE. It looks like this problem was always in chrono, but BOOST_CHRONO_HEADER_ONLY was widely used enough for it to be detected.
I'd quite like to fix this for 1.66.0, which requires either reverting the change to timer, or fixing the problem in chrono. The error is in chrono, but I'm more inclined to revert the change in timer for this release, as it's a less disruptive way of quickly fixing the problem.
Hi, Sorry, I forgot completely about BOOST_CHRONO_INLINE. I see two uses of inline mac/process_cpu_clocks.hpp: inline long tick_factor() // multiplier to convert ticks posix/process_cpu_clocks.hpp: inline nanoseconds::rep tick_factor() // multiplier to convert ticks Please, tell me if you want a fix for 1.66. Vicente
Le 14/12/2017 à 10:52, Vicente J. Botet Escriba via Boost a écrit :
Le 13/12/2017 à 11:24, Daniel James via Boost a écrit :
Hi,
We've had a link problem reported. for this code on Visual Studio, using auto-linking:
#include "boost\chrono.hpp" #include "boost\timer\timer.hpp"
int main() { auto now = boost::chrono::steady_clock::now(); boost::timer::cpu_timer cput; return 0; }
I haven't got the binaries installed yet, so I haven't reproduced it, but I think it's caused because timer now includes the chrono headers using BOOST_CHRONO_HEADER_ONLY, and in the chrono inlined headers not all functions are BOOST_CHRONO_INLINE. It looks like this problem was always in chrono, but BOOST_CHRONO_HEADER_ONLY was widely used enough for it to be detected.
I'd quite like to fix this for 1.66.0, which requires either reverting the change to timer, or fixing the problem in chrono. The error is in chrono, but I'm more inclined to revert the change in timer for this release, as it's a less disruptive way of quickly fixing the problem.
Hi,
Sorry, I forgot completely about BOOST_CHRONO_INLINE.
I see two uses of inline
mac/process_cpu_clocks.hpp: inline long tick_factor() // multiplier to convert ticks posix/process_cpu_clocks.hpp: inline nanoseconds::rep tick_factor() // multiplier to convert ticks This is not the source of any trouble, isn't it?
Sorry for the noise. Vicente
On 14 December 2017 at 10:51, Vicente J. Botet Escriba via Boost
This is not the source of any trouble, isn't it?
No, after posting I did some testing, and it needs to be fixed in timer. We're going to have a second release candidate with the change in timer reverted. Hopefully later today if the test results look good.
On 14 December 2017 at 11:07, Daniel James
No, after posting I did some testing, and it needs to be fixed in timer. We're going to have a second release candidate with the change in timer reverted. Hopefully later today if the test results look good.
Btw. what I didn't notice was that the functions have a BOOST_CHRONO_INLINE in the declaration in the class. Sorry for blaming you.
Le 13/12/2017 à 11:24, Daniel James via Boost a écrit :
Hi,
We've had a link problem reported. for this code on Visual Studio, using auto-linking:
#include "boost\chrono.hpp" #include "boost\timer\timer.hpp"
int main() { auto now = boost::chrono::steady_clock::now(); boost::timer::cpu_timer cput; return 0; }
I haven't got the binaries installed yet, so I haven't reproduced it, but I think it's caused because timer now includes the chrono headers using BOOST_CHRONO_HEADER_ONLY, and in the chrono inlined headers not all functions are BOOST_CHRONO_INLINE. It looks like this problem was always in chrono, but BOOST_CHRONO_HEADER_ONLY was widely used enough for it to be detected.
I'd quite like to fix this for 1.66.0, which requires either reverting the change to timer, or fixing the problem in chrono. The error is in chrono, but I'm more inclined to revert the change in timer for this release, as it's a less disruptive way of quickly fixing the problem.
https://github.com/boostorg/timer/pull/5 Hi again,
I believe that Boost.Timer shouldn't define BOOST_CHRONO_HEADER_ONLY in the src code. See https://github.com/boostorg/timer/pull/5/commits/98954984a4b55d01380bd8f18c8... Vicente
Le 13/12/2017 à 11:24, Daniel James via Boost a écrit :
Hi,
We've had a link problem reported. for this code on Visual Studio, using auto-linking:
#include "boost\chrono.hpp" #include "boost\timer\timer.hpp"
int main() { auto now = boost::chrono::steady_clock::now(); boost::timer::cpu_timer cput; return 0; }
I haven't got the binaries installed yet, so I haven't reproduced it, but I think it's caused because timer now includes the chrono headers using BOOST_CHRONO_HEADER_ONLY, and in the chrono inlined headers not all functions are BOOST_CHRONO_INLINE. It looks like this problem was always in chrono, but BOOST_CHRONO_HEADER_ONLY was widely used enough for it to be detected.
I'd quite like to fix this for 1.66.0, which requires either reverting the change to timer, or fixing the problem in chrono. The error is in chrono, but I'm more inclined to revert the change in timer for this release, as it's a less disruptive way of quickly fixing the problem.
What is the real problem in Boost.Chrono? Vicente
participants (3)
-
Daniel James -
Gavin Lambert -
Vicente J. Botet Escriba