[PATCH] D25257: Use StringRef in Option library instead of raw pointers (NFC)

[Reid Kleckner via llvm-commits]

rnk added inline comments.


> Job.h:76
>    /// argv, excluding the ones passed in a response file.
> -  void buildArgvForResponseFile(llvm::SmallVectorImpl<const char *> &Out) const;
> +  template <typename T>
> +  void buildArgvForResponseFile(llvm::SmallVectorImpl<T> &Out) const;

I don't like declaring a template in the header without defining it in the header. Let's just keep it const char *. It calls llvm::sys::ExecuteAndWait, which takes const char * because it calls execv/posix_spawn, which take C strings. The only other use is to implement Command::Print, which can suffer the const char *'s.

> Compilation.cpp:105
>           it = Files.begin(), ie = Files.end(); it != ie; ++it)
> -    Success &= CleanupFile(*it, IssueErrors);
> +    Success &= CleanupFile(it->data(), IssueErrors);
>    return Success;

You can change CleanupFile to take StringRef, and not do the dangerous data call.

> Driver.cpp:97
> +  for (StringRef Arg : Args) {
>      // Ingore nullptrs, they are response file's EOL markers
> +    if (Arg.empty())

Hang on, nullptr is different from an empty string in the response file parsing logic. We have to be careful not to affect that.

> CommandLine.h:1793-1798
>  /// \param [in,out] Argv Command line into which to expand response files.
>  /// \param [in] MarkEOLs Mark end of lines and the end of the response file
>  /// with nullptrs in the Argv vector.
>  /// \return true if all @files were expanded successfully or there were none.
>  bool ExpandResponseFiles(StringSaver &Saver, TokenizerCallback Tokenizer,
> +                         SmallVectorImpl<StringRef> &Argv,

I don't think this is the same for MarkEOLs = true. Command lines can totally contain empty string arguments that do not mark response file line endings.

Probably the right way to do this is to have a second optional output vector that has the indices of all the response file line endings.

https://reviews.llvm.org/D25257