This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE327111: [clang-tidy] Add check: replace string::find(...)
== 0 with absl::StartsWith (authored by hokein, committed by ).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43847
Files:
hokein added a comment.
I will commit for you.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43847
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
niko added a comment.
If this is OK as it is, can someone with commit access push this? Thanks!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43847
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
niko added inline comments.
Comment at: clang-tidy/abseil/AbseilTidyModule.cpp:15
+
+#include
+
hokein wrote:
> What is this header used for?
Sorry, ended up in the wrong file, moved to StringFindStartswithCheck.cpp.
Repository:
rCTE Clang Tools Extra
niko updated this revision to Diff 137249.
niko marked 4 inline comments as done.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43847
Files:
clang-tidy/CMakeLists.txt
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.
Looks good with a few nits.
Comment at: clang-tidy/abseil/AbseilTidyModule.cpp:15
+
+#include
+
What is this header used for?
Comment
niko marked an inline comment as done.
niko added inline comments.
Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:81
+ auto include_hint = include_inserter_->CreateIncludeInsertion(
+ source.getFileID(expr->getLocStart()),
"third_party/absl/strings/match.h",
+
niko updated this revision to Diff 137008.
niko marked 11 inline comments as done.
niko added a comment.
Addressed reviewer comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43847
Files:
clang-tidy/CMakeLists.txt
clang-tidy/abseil/AbseilTidyModule.cpp
hokein added a comment.
In https://reviews.llvm.org/D43847#1025846, @Eugene.Zelenko wrote:
> In https://reviews.llvm.org/D43847#1025833, @niko wrote:
>
> > Thanks everyone for your comments! I renamed the namespace and filenames to
> > 'abseil'.
> >
> > @Eugene.Zelenko, definitely interested in
aaron.ballman added inline comments.
Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:17-19
+ auto stringClassMatcher = anyOf(cxxRecordDecl(hasName("string")),
+ cxxRecordDecl(hasName("__versa_string")),
+
Eugene.Zelenko added a comment.
In https://reviews.llvm.org/D43847#1025833, @niko wrote:
> Thanks everyone for your comments! I renamed the namespace and filenames to
> 'abseil'.
>
> @Eugene.Zelenko, definitely interested in extending this to a C++20 modernize
> check and adding
niko added a comment.
Thanks everyone for your comments! I renamed the namespace and filenames to
'abseil'.
@Eugene.Zelenko, definitely interested in extending this to a C++20 modernize
check and adding `absl::EndsWith()` support, would it be OK though to do this
in a later patch?
niko updated this revision to Diff 136821.
niko marked 23 inline comments as done.
niko added a comment.
Renamed 'absl' to 'abseil', applied reviewer suggestions
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43847
Files:
clang-tidy/CMakeLists.txt
ahedberg added a comment.
In https://reviews.llvm.org/D43847#1024018, @aaron.ballman wrote:
> In https://reviews.llvm.org/D43847#1023452, @hokein wrote:
>
> > In https://reviews.llvm.org/D43847#1021967, @aaron.ballman wrote:
> >
> > > I need a bit more context because I'm unfamiliar with `absl`.
aaron.ballman added a comment.
In https://reviews.llvm.org/D43847#1023452, @hokein wrote:
> In https://reviews.llvm.org/D43847#1021967, @aaron.ballman wrote:
>
> > I need a bit more context because I'm unfamiliar with `absl`. What is this
> > module's intended use?
>
>
> As `absl` has been
alexfh added a comment.
In https://reviews.llvm.org/D43847#1023642, @Eugene.Zelenko wrote:
> May be //abseil// is better name for module?
I'd also go for "abseil". I'll try to get abseil folks to review this.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43847
Eugene.Zelenko added a comment.
By the word, may be similar check for std::string::rfind() and
std::string::ends_with() (does abseil have analog) should be added too?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43847
___
Eugene.Zelenko added a comment.
In https://reviews.llvm.org/D43847#1023452, @hokein wrote:
> In https://reviews.llvm.org/D43847#1021967, @aaron.ballman wrote:
>
> > I need a bit more context because I'm unfamiliar with `absl`. What is this
> > module's intended use?
>
>
> As `absl` has been
hokein added inline comments.
Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:1
+#include "StringFindStartswithCheck.h"
+
nit: We need a LICENSE comment at the top of the file.
Comment at:
hokein added a comment.
In https://reviews.llvm.org/D43847#1021967, @aaron.ballman wrote:
> I need a bit more context because I'm unfamiliar with `absl`. What is this
> module's intended use?
As `absl` has been open-sourced (https://github.com/abseil/abseil-cpp), I think
there will be more
aaron.ballman added a comment.
I need a bit more context because I'm unfamiliar with `absl`. What is this
module's intended use?
Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:17-19
+ auto stringClassMatcher = anyOf(cxxRecordDecl(hasName("string")),
+
Eugene.Zelenko added a comment.
std::basic_string::starts_with() was suggested for C++20. May be will be good
idea to generalize code to create absl and modernize checks?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43847
___
22 matches
Mail list logo
