[libcxx-commits] [libcxx] [libc++][NFC] Inline std::function members into the class body (PR #209461)

[Nikolas Klauser via libcxx-commits]

philnik777 wrote:

> > didn't expect that inlining some functions would cause any failures,
> 
> LLDB and libcxx have worked together to create tests precisely because a lot of libcxx changes cause unexpected failures. So it is just natural that you were not expecting these failures, and that is the purpose the bots serve.

It's really not. I'm merging patches before all of the CI finished all the time, and it's basically never an issue.

> > I have other patches depending on this.
> 
> This is not a valid argument. Tests exist, we wait for bots. Please don't merge any follow ups until it is decided whether this needs to be reverted or not.

I think it is. I didn't merge any follow-ups and don't plan to for now, but PR stacking is absolute trash on GitHub, so merging patches without 100% CI testing significantly eases the workflow. It pretty much always works without issues, since I'm quite familiar with what sort of change causes what sort of problems. Just because there is an issue once doesn't mean this has to be changed.

> > The part that this causes a failure in LLDB feels ridiculous
> 
> No, it's not ridiculous, it's natural. LLDB _needs_ to know the inner workings of libcxx, that is the nature of stepping through libcxx code.

IMO something feeling ridiculous and being ridiculous are quite different, and I'd like to avoid conflating the two. That being said, LLDB tends to very much avoid depending on any particular symbols being available, so I don't understand why it's different here.


FWIW I'm also not sure why you care so much in this particular case. Something breaking a bot isn't exactly unheard of; it's basically the standard throughout the rest of LLVM.


https://github.com/llvm/llvm-project/pull/209461