https://github.com/philnik777 approved this pull request.
> > I'd really like to know what the "fallout" here is before we merge > > something into a release branch that has perfectly defined behaviour. > > The fallout is that some people are running with > `-fsanitize=unsigned-integer-overflow` and that started breaking. In our > case, some folks were even running with that sanitizer in production and that > caused runtime issues. > > I disagree that this has perfectly well defined behavior. That's a pedantic > way to view things. Sure, it is, but that doesn't make it not be the case. > We all agree that `log(0)` is undefined, and in fact our call to `log(0)` > returned something that made no sense. The code only happened to work because > we were then ignoring that invalid result due to other conditions (`first == > last` inside `__introsort`). I would agree that what we returned was not intentional, but that doesn't make it wrong or undefined. Anyways, my real objection here was that the underlying issue isn't addressed - namely that we have no coverage for the integer sanitizer. Since there is a party willing to fix that now I'm happy with merging this. https://github.com/llvm/llvm-project/pull/155932 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
