[PATCH] D65198: [LLD] Do not print additional newlines after reaching error limit

[Rui Ueyama via Phabricator via llvm-commits]

ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: lld/Common/ErrorHandler.cpp:166
   } else if (errorCount == errorLimit) {
+    // Note: even though we call newline() in both branches, we can't place it
+    // before the conditions. If we did that, we might emit a newlines (without
----------------
arichardson wrote:
> MaskRay wrote:
> > The comment may be too verbose... It should be obvious from the
> > 
> > if .. else if ...
> > 
> > pattern (there is no else branch).
> > 
> > The test may look like:
> > 
> > `env LLD_IN_TEST=0 ld.lld --error-limit=1 %t.o -o /dev/null 2>&1 | FileCheck %s`
> > 
> > (You need to find a multi-line error that can be easily replicated.)
> I agree this is quite long. I added the comment here to avoid future refactoring from introducing errors. With the test it is probably fine without a comment.
> I'll see if I can find a message.
Comments work best when explaining why code works some way. In this case, the correct behavior is somewhat self-explanatory, and a unit test works best.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65198