llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.03k stars 11.57k forks source link

Overload resolution failure with multi-element initializer_list #38928

Open itollefsen opened 5 years ago

itollefsen commented 5 years ago
Bugzilla Link 39580
Version 7.0
OS All
CC @DougGregor,@mclow,@zygoloid

Extended Description

Test case:

#include <iostream>
#include <string>
#include <vector>

namespace {

struct Attribute
{
    explicit Attribute(std::string name, std::string value)
        : name(name), value(value) {}
    std::string name;
    std::string value;
};

struct Print
{
    void print(const std::string& s)
    { std::cout << "[String] " << s << std::endl; }

    void print(const std::vector<Attribute>& v)
    { std::cout << "[Vector] " << v[0].name << std::endl; }
};

}

int main()
{
    const Attribute foo("foo", "True");
    const Attribute bar("bar", "True");

    Print p;
    p.print( {foo} );
    p.print( {foo, bar} );

    return 0;
}

Compiling this with Clang 7 (or 6 for that matter), using "-std=c++17 -stdlib=libc++" produces:

error: call to member function 'print' is ambiguous
    p.print( { foo, bar } );

note: candidate function
    void print(const std::string& s)

note: candidate function
    void print(const std::vector<Attribute>& v)

I am not sure whether this is a Clang bug or a libc++ bug. Switching to libstdc++ makes it compile and run just fine. And you get the same error using g++ with libc++ as well, so it's at least in some ways related to the library implementation.

AFAIU, the call with the single-element initializer_list could hit std::string's copy constructor (which might make this related to bug 23812) and therefore discard that overload. That would leave std::vector's initializer_list constructor as the only viable option (I believe), and therefore it works.

But the failing multi-element initializer_list call... It's not telling me which std::string constructor it is considering, so I find it hard to speculate further what's going on there.

There's several ways to work around the problem. Using double braces works, presumably as that reduces it to a single-element initializer_list again. Going via a initializer_list variable also works; auto li = { foo, bar }; t.test(li);

itollefsen commented 5 years ago

After some more testing, I'm reclassifying this a libc++ bug.

It appears that it's the basic_string constructor taking a iterator pair that is too broad and gets picked up in this case.

Looking at differences between libc++ and libstdc++, libstdc++ has the iterator requirements on the constructor itself while libc++ has them on the internal __init() function.

With the following change applied locally, my test case compiles and works.

--- string.orig 2018-11-08 11:55:15.107597678 +0100 +++ string 2018-11-08 12:33:18.977010558 +0100 @@ -804,10 +804,12 @@ basic_string(self_view sv); _LIBCPP_INLINE_VISIBILITY basic_string(self_view sv, const _Allocator& __a);

This just copied the requirement from __init() and put it on the constructor itself. Which seems to be enough to discard that constructor from the overload set.

Note that I am not suggesting this change "as is". I'm merely trying to find out what's happening and why.