[PATCH] D148799: [clang] Return `StringRef` from `TargetInfo::getClobbers()`

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

> Context not available.

Please update the diff with context, https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface.

I'm not sure what the benefit of this change is. While I did say less raw pointers = better, perhaps I'd make an exception for `const char *` for constant strings. In this case, no one is manipulating the string until (I assume) after it's converted to std::string, so we're not removing risky accesses in that way. If the API were returning a `const char *` that we then did a bunch of find first, split at, strlen, etc on, then this would make more sense.

https://llvm.org/docs/ProgrammersManual.html#passing-strings-the-stringref-and-twine-classes

Also this says you can implicitly construct from a string literal, so I am surprised you need the `llvm::StringLiteral`. Unless this is just a thing that asserts that it is in fact, a literal (I've not used this before myself).

Overall I applaud the effort but in this particular case it may not be needed.


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

https://reviews.llvm.org/D148799