[PATCH] D40543: Pass by reference NewQualifiedName in QualifiedRenameRule

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: include/clang/Tooling/Refactoring/Rename/RenamingAction.h:79
   QualifiedRenameRule(const NamedDecl *ND,
-  std::string NewQualifiedName)
+  const std::string &NewQualifiedName)
   : ND(ND), NewQualifiedName(std::move(NewQualifiedName)) {}

Quolyk wrote:
> hokein wrote:
> > Passing `std::string` object is fine here -- because we use std::move below 
> > to avoid the extra copy.
> > 
> > Is the warning caught by the clang-tidy check?
> Unfortunatelly, I haven't tested, but I believe it's checkcpp warning.
I think this is a false positive of the checkcpp.

There is a similar clang-tidy "performance-unnecessary-value-param" check, but 
I don't think the check will warn this case.




https://reviews.llvm.org/D40543



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40543: Pass by reference NewQualifiedName in QualifiedRenameRule

[Dmitry Venikov via Phabricator via cfe-commits]

Quolyk added inline comments.



Comment at: include/clang/Tooling/Refactoring/Rename/RenamingAction.h:79
   QualifiedRenameRule(const NamedDecl *ND,
-  std::string NewQualifiedName)
+  const std::string &NewQualifiedName)
   : ND(ND), NewQualifiedName(std::move(NewQualifiedName)) {}

hokein wrote:
> Passing `std::string` object is fine here -- because we use std::move below 
> to avoid the extra copy.
> 
> Is the warning caught by the clang-tidy check?
Unfortunatelly, I haven't tested, but I believe it's checkcpp warning.


https://reviews.llvm.org/D40543



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40543: Pass by reference NewQualifiedName in QualifiedRenameRule

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: include/clang/Tooling/Refactoring/Rename/RenamingAction.h:79
   QualifiedRenameRule(const NamedDecl *ND,
-  std::string NewQualifiedName)
+  const std::string &NewQualifiedName)
   : ND(ND), NewQualifiedName(std::move(NewQualifiedName)) {}

Passing `std::string` object is fine here -- because we use std::move below to 
avoid the extra copy.

Is the warning caught by the clang-tidy check?


https://reviews.llvm.org/D40543



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40543: Pass by reference NewQualifiedName in QualifiedRenameRule

[Dmitry Venikov via Phabricator via cfe-commits]

Quolyk created this revision.
Herald added a subscriber: klimek.

https://reviews.llvm.org/D40543

Files:
  include/clang/Tooling/Refactoring/Rename/RenamingAction.h


Index: include/clang/Tooling/Refactoring/Rename/RenamingAction.h
===
--- include/clang/Tooling/Refactoring/Rename/RenamingAction.h
+++ include/clang/Tooling/Refactoring/Rename/RenamingAction.h
@@ -76,7 +76,7 @@
 
 private:
   QualifiedRenameRule(const NamedDecl *ND,
-  std::string NewQualifiedName)
+  const std::string &NewQualifiedName)
   : ND(ND), NewQualifiedName(std::move(NewQualifiedName)) {}
 
   Expected


Index: include/clang/Tooling/Refactoring/Rename/RenamingAction.h
===
--- include/clang/Tooling/Refactoring/Rename/RenamingAction.h
+++ include/clang/Tooling/Refactoring/Rename/RenamingAction.h
@@ -76,7 +76,7 @@
 
 private:
   QualifiedRenameRule(const NamedDecl *ND,
-  std::string NewQualifiedName)
+  const std::string &NewQualifiedName)
   : ND(ND), NewQualifiedName(std::move(NewQualifiedName)) {}
 
   Expected
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits