JDevlieghere abandoned this revision.
JDevlieghere added a comment.
I'm abandoning this revision because I think this check is getting overly
complex. There's still the problem of supporting arguments that can have side
effects, and then there's also the unaddressed issue of code possibly using
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Removing from my dashboard. Ping me once you have a new patch.
Repository:
rL LLVM
https://reviews.llvm.org/D22725
___
cfe-commits m
malcolm.parsons added a subscriber: malcolm.parsons.
malcolm.parsons added a comment.
This check looks like it implements some of CppCoreGuidelines Bounds.4
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#bounds4-dont-use-standard-library-functions-and-types-that-are-
JDevlieghere updated this revision to Diff 71835.
JDevlieghere added a comment.
Herald added subscribers: mgorny, beanz.
Still working on comment #2 from Alex but wanted to update my diff since it's
been a while and I haven't gotten around to looking into it further. So no need
to review yet.
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:37
@@ +36,3 @@
+static bool areTypesCompatible(QualType Left, QualType Right) {
+ return getStrippedType(Left) == getStrippedType(Right
2016-08-09 5:49 GMT-07:00 Aaron Ballman :
>
> I think this boils down to personal preference, which is why I'm
> concerned about the check. Either mechanism is correct, so this is
> purely a stylistic check in many regards.
>
> About warnings - well, if someone choose this check to be run, then he
JDevlieghere updated this revision to Diff 67391.
JDevlieghere added a comment.
Removed anonymous namespaces in test file. I was playing around with it but
forgot to remove it before making my last diff.
Repository:
rL LLVM
https://reviews.llvm.org/D22725
Files:
clang-tidy/modernize/CMake
JDevlieghere updated this revision to Diff 67390.
JDevlieghere marked 5 inline comments as done.
JDevlieghere added a comment.
Fixes issues raised by Alexander and Aaron
Repository:
rL LLVM
https://reviews.llvm.org/D22725
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/Mo
On Mon, Aug 8, 2016 at 11:36 PM, Piotr Padlewski
wrote:
>
>
> 2016-08-08 8:33 GMT-07:00 Aaron Ballman :
>>
>> aaron.ballman added inline comments.
>>
>>
>> Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:59-61
>> @@ +58,5 @@
>> + IncludeStyle(utils::IncludeSorter::pars
2016-08-08 8:33 GMT-07:00 Aaron Ballman :
> aaron.ballman added inline comments.
>
>
> Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:59-61
> @@ +58,5 @@
> + IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
> + Options.get("IncludeStyle", "llvm"))) {
> +
alexfh added inline comments.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:59-61
@@ +58,5 @@
+ IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+ Options.get("IncludeStyle", "llvm"))) {
+
+ for (const auto &KeyValue :
+ {std::make_pair("memcpy",
aaron.ballman added inline comments.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:59-61
@@ +58,5 @@
+ IncludeStyle(utils::IncludeSorter::parseIncludeStyle(
+ Options.get("IncludeStyle", "llvm"))) {
+
+ for (const auto &KeyValue :
+ {std::make_pair("m
alexfh requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/modernize/ModernizeTidyModule.cpp:59
@@ -57,1 +58,3 @@
"modernize-use-bool-literals");
+CheckFactories.registerCheck(
+"modernize-use-algorithm");
aaron.ballman added a comment.
In https://reviews.llvm.org/D22725#506058, @JDevlieghere wrote:
> - Added function pointer test case
> - Used placeholders for diagnostics
>
> I extended the matchers to include `::memcpy` and `::memset` as well
> because the check otherwise does not work on my m
JDevlieghere updated this revision to Diff 66835.
JDevlieghere marked 9 inline comments as done.
JDevlieghere added a comment.
- Added function pointer test case
- Used placeholders for diagnostics
I extended the matchers to include `::memcpy` and `::memset` as well because
the check otherwise d
aaron.ballman added a comment.
In https://reviews.llvm.org/D22725#505020, @JDevlieghere wrote:
> Addresses comments from Aaron Ballman
>
> @aaron.ballman Thanks for the thorough review! Can you check whether the
> tests I added address your concerns? Could you also elaborate on the case
> with
Prazek added a comment.
In https://reviews.llvm.org/D22725#505020, @JDevlieghere wrote:
> Addresses comments from Aaron Ballman
>
> @aaron.ballman Thanks for the thorough review! Can you check whether the
> tests I added address your concerns? Could you also elaborate on the case
> with the C-f
JDevlieghere updated this revision to Diff 66688.
JDevlieghere marked 21 inline comments as done.
JDevlieghere added a comment.
Addresses comments from Aaron Ballman
@aaron.ballman Thanks for the thorough review! Can you check whether the tests
I added address your concerns? Could you also elabo
aaron.ballman requested changes to this revision.
This revision now requires changes to proceed.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:25
@@ +24,3 @@
+
+static QualType getStrippedType(QualType T) {
+ while (const auto *PtrType = T->getAs())
I'd
Prazek added a comment.
In https://reviews.llvm.org/D22725#502374, @JDevlieghere wrote:
> Addressed comments from Piotr Padlewski
>
> LLVM compiles with the latest version of this check, however some tests are
> failing. I'll investigate the cause of this and update this check if it can
> be pr
JDevlieghere updated this revision to Diff 66350.
JDevlieghere added a comment.
Addressed comments from Piotr Padlewski
Repository:
rL LLVM
https://reviews.llvm.org/D22725
Files:
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/modernize/UseAl
Prazek added a comment.
I see you solved the void and conmpatible types problems. Excellent!
Can you post a patch with changes after running LLVM? I would wait for Alex or
Aaron to accept it.
Comment at: clang-tidy/modernize/UseAlgorithmCheck.cpp:58-60
@@ +57,5 @@
+
+ for (co
JDevlieghere retitled this revision from "[clang-tidy] Add check
'misc-replace-memcpy'" to "[clang-tidy] Add check 'modernize-use-algorithm'".
JDevlieghere updated the summary for this revision.
JDevlieghere set the repository for this revision to rL LLVM.
JDevlieghere updated this revision to Dif
23 matches
Mail list logo
