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

[Zachary Turner via llvm-commits]

zturner added a comment.

I can review this in more detail later.  But my hope is that we can move to `StringRef` and improve code at the same time, so that it's more than just churn.  One thing I've found in doing this with LLDB is that it's easy to introduce bugs, because once you get `StringRef` and you decide to port some C-style string manipulation, the resulting `StringRef` code is more elegant, but since it is re-written it has the potential to introduce new bugs.  As a result, I've found smaller CLs to be much more workable.

Did you find that it was impossible to do this in a smaller CL?  For example, the first change I see is  changing this:

  const char *addTempFile(const char *Name)

to this:

  llvm::StringRef addTempFile(llvm::StringRef Name)

If instead you started from a blank slate and changed it to this:

  llvm::StringRef addTempFile(const char *Name)

You would get a lot of immediate compilation failures, which you could fix up and (theoretically) end up with a CL much smaller than what you have here.  Did you try that and find it impractical for some reason?


https://reviews.llvm.org/D25257