[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

LegalizeAdulthood added inline comments. Herald added a subscriber: carlosgalvezp. Comment at: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt:19 ReplaceAutoPtrCheck.cpp + ReplaceMemcpyByStdCopy.cpp ReplaceRandomShuffleCheck.cpp In English, it's

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

xbolva00 added a comment. if(num != 0) memmove is missed optimization opportunity. Such branch can be removed (if profitable). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 ___ cfe-commits

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

lebedev.ri added a comment. Thinking about it more, i have some negative reservations about this :/ As it can be seen form https://godbolt.org/z/3BZmCM, it seems any and every(?) alternative C++ algorithm replacement is a performance pessimization in the general case, because `memcpy`

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

JonasToth added a comment. Herald added subscribers: wuzish, whisperity. You need tests for this check. Please harden them against macro-interference as well. I think the transformation should happen as function call that return `Optional` and if any failure happens the diagnostic can still be

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 205354. Blackhart marked 4 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 205341. Blackhart marked 3 inline comments as done. Blackhart added a comment. Remove "//this check//" from documentation and release note CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 Files:

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

lebedev.ri added a comment. Still missing tests Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:34-35 + callExpr(hasDeclaration(functionDecl(hasName("memcpy"), + isExpansionInSystemHeader())), +

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:192 + + This check replaces all occurrences of the C ``memcpy`` function by ``std::copy``. + Please remove //This check//. Same in first statement in documentation.

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart added a comment. Hello, I've updated the documentation with more informations about the transformations the check do. I'm not very confident in writing documentation in english. Please review it and submit me your change requests. CHANGES SINCE LAST ACTION

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 205331. Blackhart added a comment. - Reorder release note changes - Update documentation CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.h:41 + std::unique_ptr Inserter; + const utils::IncludeSorter::IncludeStyle IncludeStyle; +}; I think this could be determined from .clang-format.

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 204932. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 204930. Blackhart marked 2 inline comments as done. Blackhart added a comment. - Update releasenotes - Add documentation CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 Files:

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 204926. Blackhart marked an inline comment as done. Blackhart added a comment. - Remove unnessecary empty line - Use "array.size()" instead of numerical constant CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:11 +#include "../utils/OptionsUtils.h" + +#include Please remove unnecessary empty line. Comment at:

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 204925. Blackhart marked an inline comment as done. Blackhart added a comment. Use "std::array" instead of the raw array CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 Files:

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 204924. Blackhart marked an inline comment as done. Blackhart added a comment. Remove "auto" keyword when type is created from same statement CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 Files:

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 204922. Blackhart added a comment. Add override keyword to the destructor and set it as "= default" CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 Files:

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 204920. Blackhart added a comment. Revert "Modernize memcpy only if C++20 is enabled" CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart added a comment. In D63324#1543782 , @xbolva00 wrote: > This might not be currently ideal recommendation since std::copy produces > memmove with -O3. What do you recommend ? There is a way to disable this modernization if the user compile

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart added a comment. In D63324#1543706 , @lebedev.ri wrote: > In D63324#1543626 , @Blackhart wrote: > > > In D63324#1543609 , @lebedev.ri > > wrote: > > > > > In

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Eugene.Zelenko added a comment. New check must be mentioned in Release Notes and its documentation provided. Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:68 +const CallExpr *MemcpyNode) { + const

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

xbolva00 added a comment. This might not be currently ideal recommendation since std::copy produces memmove with -O3. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 ___ cfe-commits mailing list

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

lebedev.ri added a comment. In D63324#1543626 , @Blackhart wrote: > In D63324#1543609 , @lebedev.ri > wrote: > > > In D63324#1543607 , @Blackhart > > wrote: > > > > >

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart added a comment. In D63324#1543609 , @lebedev.ri wrote: > In D63324#1543607 , @Blackhart wrote: > > > Modernize memcpy only if C++20 is enabled > > > ... why? > This is also missing

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

lebedev.ri added a comment. In D63324#1543607 , @Blackhart wrote: > Modernize memcpy only if C++20 is enabled ... why? This is also missing documentation,releasenotes changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 204770. Blackhart added a comment. Modernize memcpy only if C++20 is enabled CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 204768. Blackhart marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 204759. Blackhart added a comment. Fix file comments typo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 204758. Blackhart added a comment. Herald added a subscriber: jsji. Add missing "override" keywords CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 Files:

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

lebedev.ri added a comment. Missing tests. Comment at: clang-tools-extra/clang-tidy/modernize/ReplaceMemcpyByStdCopy.cpp:120 +} // namespace clang \ No newline at end of file please add all the missing newlines Comment at:

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart updated this revision to Diff 204757. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63324/new/ https://reviews.llvm.org/D63324 Files: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

Re: [PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

On Fri, Jun 14, 2019 at 12:48 PM Thomas Manceau via Phabricator via cfe-commits wrote: > > Blackhart created this revision. > Blackhart created this object with edit policy "Only User: Blackhart (Thomas > Manceau)". You might want to unset that :) > Blackhart added a project:

[PATCH] D63324: [clang-tidy] Replace memcpy by std::copy

Blackhart created this revision. Blackhart created this object with edit policy "Only User: Blackhart (Thomas Manceau)". Blackhart added a project: clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Herald added a project: clang. This patch will: - replace all