[PATCH] D78819: [gold] Simplify with StringRef::consume_front. NFC

[Teresa Johnson via Phabricator via llvm-commits]

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

lgtm



================
Comment at: llvm/tools/gold/gold-plugin.cpp:270
+    } else if (opt.consume_front("jobs=")) {
       StringRef Num(opt_ + 5);
       if (!get_threadpool_strategy(Num))
----------------
MaskRay wrote:
> tejohnson wrote:
> > I think this one needs a fix, presumably just Num(opt_) now?
> > 
> > Looks like there isn't a good way to test this unfortunately, unless the garbage read causes the get_threadpool_strategy call below to fail. Can you find or add a way to test this parameter?
> This is correct, because `opt_` is the original `const char *` (unmodified). As you said, using `opt` is better. I'll fix that. Actually, 4 tests check that we can't pass garbage to get_threadpool_strategy:
> 
> ```
> /usr/bin/ld.gold: fatal error: Invalid parallelism level: jobs=1
> 
> --
> 
> ********************
> ********************
> Failing Tests (4):
>   LLVM :: tools/gold/X86/thinlto.ll
>   LLVM :: tools/gold/X86/thinlto_afdo.ll
>   LLVM :: tools/gold/X86/thinlto_archive.ll
>   LLVM :: tools/gold/X86/thinlto_cspgo.ll
> ```
Ah right, missed the fact that it used the original char pointer. Glad the tests catch any issues here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78819