rupprecht wrote: > Can you find the template that causes this issue and reduce it down to a test > similar to the test for this PR?
The problematic type is `Future<int>` from https://github.com/google/tensorstore/blob/master/tensorstore/util/future.h, but the data formatter for it is internal, and might be necessary to reproduce. (The test case also exercises `Future<void>`, and that was fine). I'll be out next week though, so unless I get lucky in attempting to reduce this in the next day, it will be a while until I have something. @cmtice might be able to help if you want something sooner. > So the PR didn't introduce a regression — it made an existing unsupported > case visible in dev builds instead of silently wrong. Well, I slightly disagree here. Crashing in dev builds is still a form of regression, even if prod builds are unaffected. To be clear though, I'm totally fine w/ how this patch landed -- it's great that you added the assert to catch this issue. I assume we believed that all the cases were handled, and an assert is a great way to either confirm that or to discover when that is actually not the case. But... once there's an assertion failure, I think we _do_ need to call that a regression of some sort, and resolve it either by handling that case so we don't get to the assertion, or removing the assertion if we can't figure that out yet. It is probably fine to re-land this as-is but w/o the assert (assuming you resolve the windows build bot issue). https://github.com/llvm/llvm-project/pull/187598 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
