Single-element initializer_list invokes wrong constructor

[llvmbot]

Bugzilla Link	23812
Version	trunk
OS	All
Reporter	LLVM Bugzilla Contributor
CC	@DougGregor,@Dushistov,@itollefsen,@zygoloid,@timsong-cpp,@jwakely

Extended Description

Test case:

#include <initializer_list>
#include <iostream>
 
struct Q {
    Q() { std::cout << "default\n"; }
    Q(Q const&) { std::cout << "copy\n"; }
    Q(Q&&) { std::cout << "move\n"; }
    Q(std::initializer_list<Q>) { std::cout << "initializer list\n"; }
};
 
int main() {
    Q x = Q { Q() };
}

Q { Q() } should invoke the initializer_list constructor, but recent Clang (trunk as of today, r239482) treats it as a move instead. For comparison, clang 3.6 and older, gcc 4.9, 5.1, and MSVC2015 all select the initializer_list constructor.

This is really bad for us, as it silently changes the meaning of code in a subtle way (manifests as making our JSON library elide single-element array literals). :(

[zygoloid]

Clang is correct here per DR1467 (http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1467), which we implemented recently. I'll take your example to the C++ committee and see if they want to reconsider that change.

[llvmbot]

Looking at it further I think you might actually be right w.r.t. the original test case. My original example was oversimplified, though - here's something closer to what we have:

#include <initializer_list>
#include <iostream>

template <typename T>
struct Container {
    Container(std::initializer_list<T>) { std::cout << "Container(initializer_list)\n"; }
};

struct Q {
    Q() { std::cout << "Q()\n"; }
    Q(Q const&) { std::cout << "Q(Q const&)\n"; }
    Q(Q&&) { std::cout << "Q(Q&&)\n"; }
    Q(Container<Q>) { std::cout << "Q(Container)\n"; }
};

int main() {
    Q x = Container<Q> { Container<Q> {} };
}

We also see a change in behavior here (one invocation of the Container constructor in recent clang, two elsewhere).

(for background: in our code Container is std::vector, Q is json11::Json, and the line is something like Json a = vector<Json> { vector<Json> { } };, which should have produced [[]] but now produces [].)

As I read it, 13.3.3.1.5 applies here, but the new paragraph shouldn't kick in: "If the parameter type is a class X and the initializer list has a single element of type cv U, where U is X or a class derived from X, ...". In the original example the parameter type was Q and the element was also Q, but in this case the parameter type is Q and the single element is Container<Q> (which is implicitly convertible to Q, but not derived from Q.) Does that seem right?

[zygoloid]

The thing that changes behavior is:

  Container<Q> { Container<Q> {} }

... which now calls the Container<Q> move constructor instead of its initializer_list constructor.

[llvmbot]

Ah, I see. Definitely seems like an unintended result of DR1467 then :/

[llvmbot]

Since it seems like the committee is revisiting this (I'm told that Mike Miller is opening a new CWG issue, though it hasn't appeared on http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html yet), and the current behavior is dangerous in how it changes existing code, would the right thing for Clang to do be to revert DR1467 until the committee decides on a better fix?

[llvmbot]

This is a simpler case I ran into. I think it is related. If it is not I can file a different bug.

The following code produces different results on GCC and on Clang. On all versions of GCC up to master, The initializer list constructor is called and this prints "1". On clang, this calls the copy constructor, and this prints "10".

I am not sure by the spec what should be the right behavior here.

#include <iostream>
#include <vector>

class V {
 public:
  V() = default;
  V(const std::vector<V>&) {};
};

int main() {
  std::vector<V> a(10, V());
  std::vector<V> b{a};
  std::cout << b.size();
}

[llvmbot]

Update on the standards committee. There is now an open issue for this behavior, but I haven't seen any updates on when it might be discussed and resolved:

http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2137

The clang behavior is technically correct as defined by the prior issue resolution, but might need to change again if the committee revises it. Meanwhile developers with existing code which worked before DR1467 are in a tricky situation where code can silently break on a compiler upgrade, and behaves differently in GCC vs clang.

[Dushistov]

At now for code int comment 6 MSVC update 2 print 10 and use "move" constructor for code in comment 1.

[llvmbot]

This post (https://stackoverflow.com/questions/51665368/c-constructor-taking-an-stdinitializer-list-of-size-one) on Stackoverflow suggests that the defect report 2137 (https://wg21.link/CWG2137) (raised by Richard Smith and based on this bug report) was released into the standard. That being said the behavior of Clang has not been updated accordingly since then, is there any reason for that?

[zygoloid]

[...] the behavior of Clang has not been updated accordingly since then,
is there any reason for that?

No reason beyond the fact that no-one has done it yet. We'd be happy to take a patch.

[zygoloid]

There is an approved patch that appears to address this: https://reviews.llvm.org/D156032
Any reason that can't be landed? @MitalAshok @cor3ntin

[shafik]

There is an approved patch that appears to address this: https://reviews.llvm.org/D156032 Any reason that can't be landed? @MitalAshok @cor3ntin

It looks like @cor3ntin wanted conformance review, if you think the PR looks good, we should land it.

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: None (llvmbot)

| | | | --- | --- | | Bugzilla Link | [23812](https://llvm.org/bz23812) | | Version | trunk | | OS | All | | Reporter | LLVM Bugzilla Contributor | | CC | @DougGregor,@dushistov,@itollefsen,@zygoloid,@timsong-cpp,@jwakely |

Extended Description

Test case:

#include <initializer_list>
#include <iostream>
 
struct Q {
    Q() { std::cout << "default\n"; }
    Q(Q const&) { std::cout << "copy\n"; }
    Q(Q&&) { std::cout << "move\n"; }
    Q(std::initializer_list<Q>) { std::cout << "initializer list\n"; }
};
 
int main() {
    Q x = Q { Q() };
}

Q { Q() } should invoke the initializer_list constructor, but recent Clang (trunk as of today, r239482) treats it as a move instead. For comparison, clang 3.6 and older, gcc 4.9, 5.1, and MSVC2015 all select the initializer_list constructor.

This is really bad for us, as it silently changes the meaning of code in a subtle way (manifests as making our JSON library elide single-element array literals). :(

[yxsamliu]

There is an approved patch that appears to address this: https://reviews.llvm.org/D156032 Any reason that can't be landed? @MitalAshok @cor3ntin

It looks like @cor3ntin wanted conformance review, if you think the PR looks good, we should land it.

I hope this PR could be landed soon. Some HIP users are affected by this issue too.

[cor3ntin]

FYI, this was reverted by 6e4930c , and the author seems to not be currently contributing actively to clang