Page MenuHomePhabricator

[LLD] Do not print additional newlines after reaching error limit
ClosedPublic

Authored by arichardson on Jul 24 2019, 4:02 AM.

Details

Summary

This could previously happen if errors that are emitted after reaching the
error limit. In that case, the flag inside the newline() function will be
set to true which causes the next call to print a newline even though the
actual message will be discarded.

Diff Detail

Repository
rL LLVM

Event Timeline

arichardson created this revision.Jul 24 2019, 4:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 24 2019, 4:02 AM

Can you add a test case?

MaskRay added inline comments.Jul 24 2019, 5:50 AM
lld/Common/ErrorHandler.cpp
166 ↗(On Diff #211461)

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.)

arichardson marked an inline comment as done.Jul 24 2019, 9:06 AM
arichardson added inline comments.
lld/Common/ErrorHandler.cpp
166 ↗(On Diff #211461)

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.

ruiu accepted this revision.Jul 24 2019, 12:54 PM

LGTM

lld/Common/ErrorHandler.cpp
166 ↗(On Diff #211461)

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.

This revision is now accepted and ready to land.Jul 24 2019, 12:54 PM
This revision was automatically updated to reflect the committed changes.