[llvm-dev] Clarification on expectations of buildbot email notifications (original) (raw)

Reid Kleckner via llvm-dev llvm-dev at lists.llvm.org
Wed Feb 20 14:16:44 PST 2019

On Wed, Feb 20, 2019 at 10:29 AM Michael Kruse <llvmdev at meinersbur.de> wrote:

maybe another interesting case is when there is disagreement over whether there is a regression.

For instance, one of my patches made clang emit an additional warning when compiling a popular project. This was not intentional by my patch, but due to an inconsistent implementation of the warning in clang. However, the warning was legitimate. I reverted the patch (it had another problem), but before I recommitted it, I put up a patch for review that fixed the inconsistent implementation such that the warning is always emitted. My question here: Should the patch be reverted even if it did not have the other problem?

I would say not necessarily. If a new warning is added that fires on a popular project and the warning is working as intended, that may be a signal that the warning shouldn't be on by default (or shouldn't be part of -Wall). We obviously need to allow ourselves the freedom to add new warnings over time. Just because a project uses "-Werror -Wall" doesn't mean that their code will compile cleanly with new compilers. However, if the warning really is low value, then we may want to remove it. If someone wants to revert a new warning because it is too noisy or has false positives, they need to actively engage the patch author to support their position.

Elaborating on your question of what constitutes a regression in general, I would say that everything is case-by-case, but we have some clear cut ones, like miscompiles or new compiler crashes. People often misdiagnose UB as a miscompile, so to revert for a miscompile, you need some evidence that the code doesn't exercise UB. Reverting for a performance regression would need to come with a strong rationale to motivate why the slowdown outweighs the supposed benefits of the patch in question. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190220/49146c25/attachment.html>

More information about the llvm-dev mailing list