Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

There seems to be a name clash for the ROOT header and std::chrono's templates #365

Open
dpiparo opened this issue Apr 25, 2024 · 3 comments

Comments

@dpiparo
Copy link
Contributor

dpiparo commented Apr 25, 2024

From the ROOT tracker ( this is a Vc issue) root-project/root#11934
(Thanks to https://github.com/telzhov for the original report, copied here as it was!)

Describe the bug

ROOT Vc/vector.h and std::chrono both use at least four same names in their namespaces, i.e. floor, ceil, round, and abs. std:: and std::chrono:: are the different name spaces of course, but trying to instantiate an std::chrono::time_point doesn't compile if a compilation unit include's Vc/vector.h header as well (or any that includes it, such as Math/Minimizer.h etc).

Expected behavior

It should compile.

To Reproduce

Here is an example:

#include "Vc/vector.h"
#include <chrono>

void foo() {
    std::chrono::time_point<std::chrono::system_clock,
                            std::chrono::nanoseconds> tp;
    std::chrono::floor<std::chrono::seconds>(tp);
}

Trying to build

$ c++ $(root-config --cflags) -std=c++17 -c a.cpp 
In file included from a.cpp:1:
In file included from /Users/telzhov/opt/root/include/Vc/vector.h:32:
/Users/telzhov/opt/root/include/Vc/scalar/../common/../sse/../scalar/vector.h:50:5: error: static_assert failed due to requirement 'std::is_arithmetic<std::chrono::duration<long long, std::ratio<1, 1>>>::value' "Vector<T> only accepts arithmetic builtin types as template parameter T."
    static_assert(std::is_arithmetic<T>::value,
    ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/chrono:1427:67: note: in instantiation of template class 'Vc_1::Vector<std::chrono::duration<long long>, Vc_1::VectorAbi::Scalar>' requested here
    return time_point<_Clock, _ToDuration>{floor<_ToDuration>(__t.time_since_epoch())};
                                                                  ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/chrono:1427:44: note: while substituting deduced template arguments into function template 'floor' [with T = std::chrono::duration<long long>]
    return time_point<_Clock, _ToDuration>{floor<_ToDuration>(__t.time_since_epoch())};
                                           ^
a.cpp:7:18: note: in instantiation of function template specialization 'std::chrono::floor<std::chrono::duration<long long>, std::chrono::system_clock, std::chrono::duration<long long, std::ratio<1, 1000000000>>>' requested here
    std::chrono::floor<std::chrono::seconds>(tp);
                 ^
1 error generated.

Try to comment out the line which includes Vc/vector.h, and you'll find that it works

Setup

  1. ROOT version: 6.26/10
  2. Operating system: macOS 12.6.1, clang 14.0.0 (clang-1400.0.29.202)
  3. How you obtained ROOT: binary download (root_v6.26.10.macos-12.6-x86_64-clang140.tar.gz)
@dpiparo
Copy link
Contributor Author

dpiparo commented Apr 25, 2024

I guess something wrong happens when one calls std::chrono::floor by basename (i.e. without namespace prefix). The statement above ends up with using of 'using namespace' with consequent calling of the bare floor template (without prefix).

Well, let's reproduce it by ourselves:

#include "Vc/vector.h"
#include <chrono>

void foo() {
    std::chrono::nanoseconds dur;
    std::chrono::floor<std::chrono::seconds>(dur);
}

$ c++ $(root-config --cflags) -std=c++17 -c a.cpp 
$ 

Look, floor for std::chrono::duration works, unlike the example in the original message. Now let's rewrite it a bit:

#include "Vc/vector.h"
#include <chrono>

void foo() {
    using namespace std::chrono;

    nanoseconds dur;
    floor<seconds>(dur);
}

$ c++ $(root-config --cflags) -std=c++17 -c a.cpp 
In file included from a.cpp:1:
In file included from /Users/telzhov/opt/root/include/Vc/vector.h:32:
/Users/telzhov/opt/root/include/Vc/scalar/../common/../sse/../scalar/vector.h:50:5: error: static_assert failed due to requirement 'std::is_arithmetic<std::chrono::duration<long long, std::ratio<1, 1>>>::value' "Vector<T> only accepts arithmetic builtin types as template parameter T."
    static_assert(std::is_arithmetic<T>::value,
    ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
a.cpp:8:20: note: in instantiation of template class 'Vc_1::Vector<std::chrono::duration<long long>, Vc_1::VectorAbi::Scalar>' requested here
    floor<seconds>(dur);
                   ^
a.cpp:8:5: note: while substituting deduced template arguments into function template 'floor' [with T = std::chrono::duration<long long>]
    floor<seconds>(dur);
    ^
1 error generated.

In fact, floor for time_point is just a tiny wrapper for floor for duration, which calls bare floor, and that's where compilation fails:

#if _LIBCPP_STD_VER > 14
template <class _ToDuration, class _Clock, class _Duration>
typename enable_if
<
    __is_duration<_ToDuration>::value,
    time_point<_Clock, _ToDuration>
>::type
floor(const time_point<_Clock, _Duration>& __t)
{
    return time_point<_Clock, _ToDuration>{floor<_ToDuration>(__t.time_since_epoch())};
                                        // ^^^^^ here is a compiler error
}

I tried to insert std::chrono there (right to the 'chrono' header), and the code from my first message

void foo() {
    std::chrono::time_point<std::chrono::system_clock,
                            std::chrono::nanoseconds> tp;
    std::chrono::floor<std::chrono::seconds>(tp);
}

got compiled.

@mattkretz
Copy link
Member

Add -DVc_NO_STD_FUNCTIONS and I guess it should work (I think Vc got the default wrong there).

If you call floor unqualified then ADL finds Vc::floor on name lookup and on overload resolution it leads to a template instantiation that is ill-formed.

@Axel-Naumann
Copy link
Member

@dpiparo ok to close?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants