search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.21k stars 11.64k forks source link

include/clang/Tooling/Refactoring/Rename/RenamingAction.h:79]: (performance) #34652

Open llvmbot opened 6 years ago

llvmbot commented 6 years ago
Bugzilla Link 35304
Version trunk
OS Linux
Reporter LLVM Bugzilla Contributor
CC @JoeLoser

Extended Description

trunk/llvm/tools/clang/include/clang/Tooling/Refactoring/Rename/RenamingAction.h:79]: (performance) Function parameter 'NewQualifiedName' should be passed by reference.

Source code is

QualifiedRenameRule(const NamedDecl *ND, std::string NewQualifiedName)

Maybe better code

QualifiedRenameRule(const NamedDecl *ND, const std::string & NewQualifiedName)

llvmbot commented 6 years ago

Hmm, I don't see why this is an issue.

Because passing large parameters by const reference is a lot cheaper than passing them by copying.

Copying eight bytes is a lot cheaper than copying dozens or hundreds of bytes.

JoeLoser commented 6 years ago

Hmm, I don't see why this is an issue. The constructor argument NewQualifiedName gets immediately moved from into the private member NewQualifiedName.

nexon commented 2 years ago

I see that this is still an issue, if no one is working on this, I can take it

Martin-Vinci commented 2 years ago

Please do

On Sun, 12 Dec 2021 at 6:07 AM Alberto Lagos T. @.***> wrote:

I see that this is still an issue, if no one is working on this, I can take it

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/llvm/llvm-project/issues/34652#issuecomment-991826553, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASXEAEAMOGN5RRHWIF23BSLUQQGWTANCNFSM5J3XQLMA .

llvmbot commented 1 year ago

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1) Assign the issue to you. 2) Fix the issue locally. 3) Run the test suite locally. 3.1) Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests. 4) Create a git commit 5) Run git clang-format HEAD~1 to format your changes. 6) Submit the patch to Phabricator. 6.1) Detailed instructions can be found here

For more instructions on how to submit a patch to LLVM, see our documentation.

If you have any further questions about this issue, don't hesitate to ask via a comment on this Github issue.

@llvm/issue-subscribers-good-first-issue