ldd::COFF: initalize ErrorHandler with CanExitEarly value

[andrewrk]

Summary

Previously, the COFF driver would call exit(1) from the
ErrorHandler in the case of a link error, even if
CanExitEarly=false was specified. Now it initializes
the ErrorHandler in the same way that the ELF driver does.

[andrewrk]

I would also like to request that this is backported to the release_60 branch.

[smeenai]

LGTM.

For future reference, the standard procedure is to add the relevant -commits mailing list (llvm-commits in this case) as a subscriber when uploading the patch, so that a mail gets sent out to the list. Over here the patch is trivial enough that I'm comfortable accepting regardless.

How did you upload this patch, out of curiosity? There are mechanisms in place to automatically add the relevant -commits list as a subscriber now, so I'm wondering why those weren't triggered. CC @benhamilton

Do you have commit access, or do you need me to commit this for you?

[andrewrk]

Thank you, I will CC llvm-commits next time. I followed these instructions: https://llvm.org/docs/Phabricator.html

I do not have commit access; I would be grateful for you to commit it for me.

[smeenai]

Yeah, that page mentions mailing lists to use for LLVM and clang, but doesn't mention other projects. The wording could be clearer there.

Did you upload via the command line (arcanist) or the web interface?

I'm traveling right now. I'll be able to commit this late Sunday or early Monday Pacific Time, if no one gets to it before that.

[andrewrk]

I'll be careful next time, thanks. I used the web interface. Arcanist seemed more involved, but if that is preferable I can do it next time.

Take your time merging the code. My main concern is getting it into LLD 6.0.0. After this patch is merged my LLD fork goes from having 2 patches against upstream to 1.

[smeenai]

Not your fault. I think the right thing would happen if you added a Repository when uploading the change, and I'll update the instructions to reflect that. arcanist shouldn't be necessary. It's kinda unfortunate that Phabricator allows submitting a diff without having a repository ...

[andrewrk]

I would have selected a repository but the instructions explicitly say not to:

Leave the Repository field blank.

Probably removing this instruction would be sufficient.

[benhamilton]

How did you upload this patch, out of curiosity? There are mechanisms in place to automatically add the relevant -commits list as a subscriber now, so I'm wondering why those weren't triggered. CC @benhamilton

Thanks for the reminder! I made https://reviews.llvm.org/H270 for this a while ago, but forgot to add the main LLVM repo. Fixed.

For background, I had to wait until D40502, D40501, D40500, D40499, D40498, D40496, D40495, and D40494 landed before doing so, because otherwise revisions for those repos would all Cc: llvm-commits even if they were for Clang tools, and I forgot to follow up.

COFF/Driver.cpp

 
 bool link(ArrayRef<const char *> Args, bool CanExitEarly, raw_ostream &Diag) {
   errorHandler().LogName = Args[0];
   errorHandler().ErrorOS = &Diag;
   errorHandler().ColorDiagnostics = Diag.has_colors();
   errorHandler().ErrorLimitExceededMsg =
       "too many errors emitted, stopping now"
       " (use /ERRORLIMIT:0 to see all errors)";
+  errorHandler().ExitEarly = CanExitEarly;
   Config = make<Configuration>();
   Config->Argv = {Args.begin(), Args.end()};
   Config->CanExitEarly = CanExitEarly;
 
   Symtab = make<SymbolTable>();
 
   Driver = make<LinkerDriver>();
   Driver->link(Args);