VcDevel / Vc

SIMD Vector Classes for C++
BSD 3-Clause "New" or "Revised" License
1.45k stars 152 forks source link

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

Open dpiparo opened 4 months ago

dpiparo commented 4 months ago

From the ROOT tracker ( this is a Vc issue) https://github.com/root-project/root/issues/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 commented 4 months ago

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 commented 4 months ago

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 commented 2 months ago

@dpiparo ok to close?