hokein closed this revision.
hokein added a comment.
Thanks @aaron.ballman. I'm closing it now as it is committed in
https://reviews.llvm.org/rL340915.
https://reviews.llvm.org/D51061
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://l
aaron.ballman added a comment.
In https://reviews.llvm.org/D51061#1215917, @hugoeg wrote:
> @aaron.ballman when you get the chance could you take another look at this
> and commit if it is ready? My internship ends rather soon and this is a tad
> time sensitive and I don't have commit access. T
hugoeg added a comment.
@aaron.ballman when you get the chance could you take another look at this and
commit if it is ready? My internship ends rather soon and this is a tad time
sensitive and I don't have commit access. Thank you for your time!
https://reviews.llvm.org/D51061
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM!
https://reviews.llvm.org/D51061
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman
hugoeg updated this revision to Diff 162403.
hugoeg marked an inline comment as done.
https://reviews.llvm.org/D51061
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/StrCatAppendCheck.cpp
clang-tidy/abseil/StrCatAppendCheck.h
docs/Release
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/StrCatAppendCheck.cpp:91-92
+ diag(Op->getBeginLoc(),
+ "please use absl::StrAppend instead of absl::StrCat when appending to "
+ "an existing string")
+ << FixItHint::CreateReplacement(
hugoeg updated this revision to Diff 162395.
hugoeg added a comment.
ran svn update
https://reviews.llvm.org/D51061
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/StrCatAppendCheck.cpp
clang-tidy/abseil/StrCatAppendCheck.h
docs/Release
hugoeg updated this revision to Diff 162390.
hugoeg marked 9 inline comments as done.
hugoeg added a comment.
applied corrections from comments
https://reviews.llvm.org/D51061
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/StrCatAppendChec
aaron.ballman added inline comments.
Comment at: clang-tidy/abseil/StrCatAppendCheck.cpp:82
+ if (Call->getNumArgs() == 1) {
+diag(Op->getBeginLoc(), "call to absl::StrCat does nothing");
+return;
JonasToth wrote:
> please use 'absl::StrCat' in the diagn
JonasToth added inline comments.
Comment at: clang-tidy/abseil/StrCatAppendCheck.cpp:21
+namespace {
+
+// Skips any combination of temporary materialization, temporary binding and
I think you can elide the empty line around the matcher.
Commen
hugoeg updated this revision to Diff 162253.
hugoeg added a comment.
minor fixes, style improvements
https://reviews.llvm.org/D51061
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/StrCatAppendCheck.cpp
clang-tidy/abseil/StrCatAppendCheck
hugoeg updated this revision to Diff 162231.
https://reviews.llvm.org/D51061
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/StrCatAppendCheck.cpp
clang-tidy/abseil/StrCatAppendCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil
Eugene.Zelenko added inline comments.
Comment at: docs/clang-tidy/checks/abseil-str-cat-append.rst:11
+
+ a = StrCat(a, b); // Use StrAppend(&a, b) instead.
+
Please add namespace.
https://reviews.llvm.org/D51061
hugoeg added a comment.
Let me know if there's anything else I can fix to move the process along.
https://reviews.llvm.org/D51061
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hugoeg updated this revision to Diff 161995.
hugoeg marked 2 inline comments as done.
https://reviews.llvm.org/D51061
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/StrCatAppendCheck.cpp
clang-tidy/abseil/StrCatAppendCheck.h
docs/Release
Eugene.Zelenko added inline comments.
Comment at: docs/ReleaseNotes.rst:70
+
+ Flags uses of ``absl::StrCat()`` to append to a std::string. Suggests
+ ``absl::StrAppend()`` should be used instead.
Please enclose std::string into ``
Comment a
hugoeg updated this revision to Diff 161982.
https://reviews.llvm.org/D51061
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/StrCatAppendCheck.cpp
clang-tidy/abseil/StrCatAppendCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil
JonasToth added inline comments.
Comment at: test/clang-tidy/abseil-str-cat-append.cpp:91
+
+void bar() {
+ std::string a, b;
hugoeg wrote:
> JonasToth wrote:
> > What happens if `StrCat` is used e.g. in an `std::accumulate` call as the
> > binary operator? (ht
hugoeg updated this revision to Diff 161974.
hugoeg added a comment.
minor fixes
https://reviews.llvm.org/D51061
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/StrCatAppendCheck.cpp
clang-tidy/abseil/StrCatAppendCheck.h
docs/ReleaseNot
hugoeg added inline comments.
Comment at: test/clang-tidy/abseil-str-cat-append.cpp:91
+
+void bar() {
+ std::string a, b;
JonasToth wrote:
> What happens if `StrCat` is used e.g. in an `std::accumulate` call as the
> binary operator? (https://en.cppreference.c
hugoeg updated this revision to Diff 161970.
hugoeg marked 11 inline comments as done.
hugoeg added a comment.
applied corrections from comments
https://reviews.llvm.org/D51061
Files:
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/StrCatAppendChe
JonasToth added inline comments.
Comment at: clang-tidy/abseil/StrCatAppendCheck.cpp:28
+ for (;;) {
+if (const auto *MTE = dyn_cast(E)) {
+ E = MTE->getTemporary();
You can ellide the braces for the single stmt ifs
Comment at: clang-
hokein added a comment.
Please make sure the code follows the LLVM code style, e.g. the variable name
format "CamelName".
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51061
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
23 matches
Mail list logo
