[PATCH] D156978: [symbolizer][NFC] Move file argument parsing into separate function

[Serge Pavlov via Phabricator via llvm-commits]

sepavloff added inline comments.


================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:138
 
+static std::pair<std::string, StringRef>
+getSpaceDelimitedWord(StringRef Source) {
----------------
ikudrin wrote:
> The function returns a part of its input, so there is no need to allocate new memory for that, `StringRef` is enough. It should be the responsibility of the caller to copy the result if necessary.
> 
> Also, the function can update `Source` if it is passed as a reference, which would simplify the code even more.
Indeed, `StringRef` can be used here.

> the function can update Source if it is passed as a reference

Passing by reference can bring new issues. In this case it prevents from using constructs like `getSpaceDelimitedWord(Source.ltrim())`. 


================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:164
                          uint64_t &ModuleOffset) {
   const char kDelimiters[] = " \n\r";
   ModuleName = "";
----------------
ikudrin wrote:
> It looks like `kDelimiters` can be removed.
It is true. Removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156978