[libcxx-commits] [PATCH] D128146: [libc++] Use uninitialized algorithms for vector

[Augusto Noronha via Phabricator via libcxx-commits]

augusto2112 added a comment.

In D128146#3670042 <https://reviews.llvm.org/D128146#3670042>, @philnik wrote:

> In D128146#3670021 <https://reviews.llvm.org/D128146#3670021>, @augusto2112 wrote:
>
>> In D128146#3669984 <https://reviews.llvm.org/D128146#3669984>, @philnik wrote:
>>
>>> In D128146#3669977 <https://reviews.llvm.org/D128146#3669977>, @augusto2112 wrote:
>>>
>>>> @ldionne just by looking at the patch, I can't tell if it's clang or libcxx who is doing the wrong thing (I do see that the stack trace includes this new `__uninitialized_allocator_move_if_noexcept` function, which is hidden from the ABI, maybe that's part of the problem?) . It shouldn't be LLDB's responsibility to track down who is responsible after and it's not healthy for the project to xfail tests that are broken by one of its dependencies and investigate later on, which would just lead to more and more xfails. The LLVM policy <https://llvm.org/docs/DeveloperPolicy.html#id16> states that we should revert patches "If you break a buildbot in a way which can’t be quickly fixed, please revert.". So I think we should do that.
>>>
>>> It also states that there should be a reproducer (ideally) and more generally a way for the author to debug the issue. It should should be simple to obtain a reproducer if it's a libc++ problem. Otherwise the patch is reverted and I still don't know what the bug is; assuming there even is one in this code. If you don't give me that I'll re-land the patch without any meaningful progress, which is useless.
>>
>> The test reproduces the issue, no?
>
> I have literally no idea what the test is even doing (as I said before). I also don't know how to run it. You have given me literally 0 information regarding the test other that "it fails". If it's a libc++ issue it should be possible to reproduce it without having to run LLDB tests.

The test compiles this code:

  #include <list>
  #include <stack>
  #include <vector>
  
  struct C {
    // Constructor for testing emplace.
    C(int i) : i(i) {};
    int i;
  };
  
  int main(int argc, char **argv) {
    // std::deque is the default container.
    std::stack<C> s_deque({{1}, {2}, {3}});
    std::stack<C, std::vector<C>> s_vector({{1}, {2}, {3}});
    std::stack<C, std::list<C>> s_list({{1}, {2}, {3}});
    return 0; // Set break point at this line.
  }

Runs lldb and attaches at the line with the " // Set break point at this line." comment. From the lldb console it (among other things):

- Turns on importing the std module (settings set target.import-std-module true) - "Import the 'std' C++ module to improve expression parsing involving  C++ standard library types."



- Evaluates the following expressions: "expr s_vector.push({4})" which constructs a new vector to add to the stack.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128146/new/

https://reviews.llvm.org/D128146