jcai19 marked 2 inline comments as done.
jcai19 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();
----------------
george.burgess.iv wrote:
> 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.
Make sense.
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
