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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 24 12:26:45 PDT 2020


MaskRay marked an inline comment as done.
MaskRay added inline comments.


================
Comment at: llvm/tools/gold/gold-plugin.cpp:270
+    } else if (opt.consume_front("jobs=")) {
       StringRef Num(opt_ + 5);
       if (!get_threadpool_strategy(Num))
----------------
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
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78819





More information about the llvm-commits mailing list