george.burgess.iv added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:27
+void CloexecPipeCheck::check(const MatchFinder::MatchResult &Result) {
+ const std::string &ReplacementText =
+ (Twine("pipe2(") + getSpellingArg(Result, 0) + ", O_CLOEXEC)").str();
----------------
jcai19 wrote:
> george.burgess.iv wrote:
> > simplicity nit: can this be a `std::string`?
> replaceFunc takes a llvm::StringRef as the third parameter. StringRef is
> "expected to be used in situations where the character data resides in some
> other buffer, whose lifetime extends past that of the StringRef", which is
> true in this case, so I think it should be fine.
Both should be functionally equivalent in this code. My point was that it's not
common to rely on temporary lifetime extension when writing the non-`const&`
type would be equivalent (barring maybe cases with `auto`, but that's not
applicable), and I don't see why we should break with that here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61967/new/
https://reviews.llvm.org/D61967
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
