Prazek updated this revision to Diff 8.
Prazek marked an inline comment as done.
Prazek added a comment.
merged with boost module
http://reviews.llvm.org/D18136
Files:
clang-tidy/CMakeLists.txt
clang-tidy/boost/BoostTidyModule.cpp
clang-tidy/boost/CMakeLists.txt
Prazek marked 2 inline comments as done.
Comment at: clang-tidy/boost/UseToStringCheck.cpp:38
@@ +37,3 @@
+ argumentCountIs(1), unless(isInTemplateInstantiation()))
+ .bind("to_string"),
+ this);
alexfh wrote:
> clang-format?
nope,
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Looks good with a couple of nits. Please submit this as a single patch, there's
no need to add an empty "boost" module separately.
Comment at:
Prazek updated this revision to Diff 55280.
http://reviews.llvm.org/D18136
Files:
clang-tidy/boost/BoostTidyModule.cpp
clang-tidy/boost/CMakeLists.txt
clang-tidy/boost/UseToStringCheck.cpp
clang-tidy/boost/UseToStringCheck.h
docs/ReleaseNotes.rst
Prazek marked 4 inline comments as done.
Prazek added a comment.
http://reviews.llvm.org/D18136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/boost/BoostTidyModule.cpp:26
@@ -22,3 +25,3 @@
ClangTidyOptions getModuleOptions() override {
ClangTidyOptions Options;
This method doesn't
Prazek added a comment.
Alex, if you accept this revision, please accept this also
http://reviews.llvm.org/D18274
http://reviews.llvm.org/D18136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
Prazek updated this revision to Diff 55077.
Prazek marked an inline comment as done.
Prazek added a comment.
I hope it is final
http://reviews.llvm.org/D18136
Files:
clang-tidy/boost/BoostTidyModule.cpp
clang-tidy/boost/CMakeLists.txt
clang-tidy/boost/UseToStringCheck.cpp
Prazek marked 5 inline comments as done.
Comment at: clang-tidy/boost/UseToStringCheck.cpp:60
@@ +59,3 @@
+ else
+return;
+
alexfh wrote:
> Please add a reduced test case for this.
I don't see it crashing right now on the same test when it was crashing
alexfh added inline comments.
Comment at: test/clang-tidy/boost-use-to-string.cpp:2
@@ +1,3 @@
+// RUN: %check_clang_tidy %s boost-use-to-string %t
+
+
nit: Remove one empty line.
http://reviews.llvm.org/D18136
___
alexfh added a comment.
FYI, an alternative fix has been submitted in http://reviews.llvm.org/D19231.
Please check whether it fixes the issue.
http://reviews.llvm.org/D18136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
alexfh added inline comments.
Comment at: clang-tidy/boost/UseToStringCheck.cpp:53
@@ +52,3 @@
+ CharType->isSpecificBuiltinType(BuiltinType::Char_U))
+// Is CharType 'char'.
+StringType = "string";
These comments don't seem to be useful, but if you
Prazek added inline comments.
Comment at: clang-tidy/boost/UseToStringCheck.cpp:59
@@ +58,3 @@
+
+ if (CharType.isNull())
+return;
alexfh wrote:
> When can `CharType` be `isNull()`? Do you have a test case for this?
I think it's because of some libstdc++
Prazek updated the summary for this revision.
Prazek updated this revision to Diff 53891.
Prazek added a comment.
Small tests update. Ping me when your patch will be in trunk
http://reviews.llvm.org/D18136
Files:
clang-tidy/boost/BoostTidyModule.cpp
clang-tidy/boost/CMakeLists.txt
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
FYI, a fix for the assertion failure you see is sent for review as
http://reviews.llvm.org/D19144. Let's wait for it to land, before going on with
this patch.
Prazek updated the summary for this revision.
Prazek updated this revision to Diff 53121.
Prazek marked 5 inline comments as done.
http://reviews.llvm.org/D18136
Files:
clang-tidy/boost/BoostTidyModule.cpp
clang-tidy/boost/CMakeLists.txt
clang-tidy/boost/UseToStringCheck.cpp
curdeius added a subscriber: curdeius.
curdeius added a comment.
Minor remark.
Comment at: docs/clang-tidy/checks/list.rst:7
@@ -6,2 +6,3 @@
.. toctree::
+ boost-use-to-string
cert-dcl03-c (redirects to misc-static-assert)
toctree directive needs the
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/boost/UseToStringCheck.cpp:55
@@ +54,3 @@
+void UseToStringCheck::check(const MatchFinder::MatchResult ) {
+ const auto *MatchedToString =
Prazek marked an inline comment as done.
Prazek added a comment.
http://reviews.llvm.org/D18136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Prazek updated the summary for this revision.
Prazek updated this revision to Diff 52378.
Prazek added a comment.
Updated ReleaseNotes and also fixed bug.
After lgtm please lgtm also this http://reviews.llvm.org/D18274 because I want
to send them together, but in separate commits.
LegalizeAdulthood added a comment.
Please summarize this check in `docs/ReleaseNotes.rst` in the clang-tidy
section.
http://reviews.llvm.org/D18136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
Prazek added inline comments.
Comment at: test/clang-tidy/boost-use-to-string.cpp:5-11
@@ +4,9 @@
+namespace std {
+
+template class basic_string {};
+
+using string = basic_string;
+using wstring = basic_string;
+}
+
+namespace boost {
I don't see any test
Prazek marked 12 inline comments as done.
Prazek added a comment.
http://reviews.llvm.org/D18136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
LegalizeAdulthood added inline comments.
Comment at: clang-tidy/boost/UseToStringCheck.cpp:56
@@ +55,3 @@
+ const auto *MatchedToString = Result.Nodes.getNodeAs("to_string");
+ auto Q = Result.Nodes.getNodeAs("char_type")->getAsType();
+
Q seems pretty obtuse
Prazek updated the summary for this revision.
Prazek updated this revision to Diff 51057.
Prazek added a comment.
fixed many things thaks to Alexander
http://reviews.llvm.org/D18136
Files:
clang-tidy/boost/BoostTidyModule.cpp
clang-tidy/boost/CMakeLists.txt
Prazek added inline comments.
Comment at: test/clang-tidy/boost-use-to-string.cpp:76-81
@@ +75,8 @@
+// CHECK-FIXES: fun(std::to_string(f));
+ fun(boost::lexical_cast(g));
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use std::to_string instead of
boost::lexical_cast
Prazek added a comment.
There is also big issue here - I don't check if the type of argument is
builtinType. I don't know why but matcher "builtinType" didn't work when I
tried to use it. (like I wrote in another email)
Comment at: clang-tidy/boost/UseToStringCheck.cpp:33
@@
hokein added a subscriber: hokein.
hokein added a comment.
> Is there any better solution for the basic_string problem?
The current solution looks fine to me. I don't have a better solution. Maybe
@alexfh has.
Comment at: docs/clang-tidy/checks/boost-use-to-string.rst:6
@@
28 matches
Mail list logo
